fix: improve error handling for user-/meta-/vendor-data handlers#93
Merged
Merged
Conversation
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
ErrEmptyID is returned if an ID (xname) passed is empty. Functions can check for this error to warn the caller not to pass an empty ID. ErrSMDResponse is an error that wraps an *http.Response, specifically one that is returned from calls to SMD (e.g. via getSMD()). This is so callers of getSMD() can distinguish between control flow errors and HTTP errors (response >= 400) from SMD. This commit also separates error definitions for the smdclient package into their own errors.go file. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Contributor
Author
|
Ugh, forgot to lint before pushing... Will push changes shortly. |
In MetaDataHandler(), issue HTTP response based on if the errors returned by smd.ComponentInformation() and smd.GroupMembership() are control flow errors or unsuccessful HTTP return codes (>= 400). With the addition of ErrSMDResponse, the error can be checked if it is an HTTP error from an API call to SMD or a control flow error and perform the proper actions based on this. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
When asking for the user data of either: - a non-existent group - a non-existent node - a node that is not in a group the previous error message simply said "Group not found" which can be misleading. This commit clarifies that the error could be any of the above possibilities. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
synackd
force-pushed
the
synackd/fix/nonexistent-node-impersonation
branch
from
f765488 to
1ca0633
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #67
Changes
Refactor the error handling in the data handlers. Specifically:
SMDClient:
errors.goErrEmptyIDerror type to distinguish errors that occur because the passed ID is emptyErrSMDResponseerror type (returned bygetSMD()) to capture SMD HTTP return codesHandlers:
ErrSMDResponseerror to determine cloud-init's response to different SMD response codesTesting
Node Booting
When booting a node with debugging enabled, you should see messages akin to:
Invalid xnames
vendor-data:
Try to get vendor-data for invalid xname:
cloud-init logs (200 returned, notice 400 returned by SMD):
SMD logs (400 returned by SMD):
meta-data:
Try to get meta-data for invalid xname:
cloud-init logs (404 returned, notice 404 returned by SMD):
SMD for some reason doesn't check the xname format, but simply searches for the passed ID. Thus, it returns 404 when it can't find it:
user-data:
Try to get user-data for invalid xname:
cloud-init unconditionally returns
#cloud-init(200) for user-data since it is not used:Unknown (but valid) xname
vendor-data:
cloud-init should return 200 (include list is empty) and SMD should return 404.
cloud-init logs:
meta-data:
Both SMD and cloud-init return 404.
cloud-init logs:
user-data:
user-data always returns 200.
Groups
Try to get data for any of:
And the message should be the same: