From 73986ed65178241f8c27c1fd1a4fdff51a94752c Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Wed, 1 Oct 2025 13:04:44 -0600 Subject: [PATCH 1/5] fix(SMDClient): GroupMembership: returhn error if id is empty Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- internal/smdclient/SMDclient.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/smdclient/SMDclient.go b/internal/smdclient/SMDclient.go index 7c466677..99bf7b48 100644 --- a/internal/smdclient/SMDclient.go +++ b/internal/smdclient/SMDclient.go @@ -339,7 +339,9 @@ func (s *SMDClient) MACfromID(id string) (string, error) { // GroupMembership returns the group labels for the xname with the given ID func (s *SMDClient) GroupMembership(id string) ([]string, error) { if id == "" { - log.Err(errors.New("ID is empty")).Msg("ID is empty") + err := errors.New("ID is empty") + log.Err(err).Msg("failed to get group membership") + return []string{}, err } ml := new(sm.Membership) ep := "/hsm/v2/memberships/" + id From 6389d84751b4a284f1a9cfa9986b310a92614f61 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Wed, 1 Oct 2025 18:28:25 -0600 Subject: [PATCH 2/5] feat(SMDClient): add ErrEmptyID and ErrSMDResponse errors 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> --- internal/smdclient/SMDclient.go | 11 +++++++---- internal/smdclient/errors.go | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 internal/smdclient/errors.go diff --git a/internal/smdclient/SMDclient.go b/internal/smdclient/SMDclient.go index 99bf7b48..3e0e65ff 100644 --- a/internal/smdclient/SMDclient.go +++ b/internal/smdclient/SMDclient.go @@ -38,10 +38,6 @@ type SMDClientInterface interface { // golang lint // Expand this client to handle more of the SMD API and work more directly with the resources it manages -var ( - ErrUnmarshal = errors.New("cannot unmarshal JSON") -) - // SMDClient is a client for SMD type SMDClient struct { clusterName string @@ -206,6 +202,10 @@ func (s *SMDClient) getSMD(ep string, smd interface{}) error { defer func() { _ = resp.Body.Close() // ignoring error on deferred Close }() + if resp.StatusCode >= 400 { + log.Error().Msgf("SMD GET request went through, but returned unsuccessful HTTP response (HTTP %d)", resp.StatusCode) + return ErrSMDResponse{HTTPResponse: resp} + } body, err := io.ReadAll(resp.Body) if err != nil { log.Error().Err(err).Msg("failed to read response body") @@ -354,6 +354,9 @@ func (s *SMDClient) GroupMembership(id string) ([]string, error) { func (s *SMDClient) ComponentInformation(id string) (base.Component, error) { var node base.Component + if strings.Trim(id, " \t") == "" { + return node, ErrEmptyID + } ep := "/hsm/v2/State/Components/" + id err := s.getSMD(ep, &node) if err != nil { diff --git a/internal/smdclient/errors.go b/internal/smdclient/errors.go new file mode 100644 index 00000000..0c52e054 --- /dev/null +++ b/internal/smdclient/errors.go @@ -0,0 +1,21 @@ +package smdclient + +import ( + "errors" + "fmt" + "net/http" +) + +var ( + ErrUnmarshal = errors.New("cannot unmarshal JSON") + ErrEmptyID = errors.New("empty id") +) + +// ErrSMDResponse contains the HTTP response of a REST API request to SMD. +type ErrSMDResponse struct { + HTTPResponse *http.Response +} + +func (esr ErrSMDResponse) Error() string { + return fmt.Sprintf("SMD response returned %s", esr.HTTPResponse.Status) +} From 1c835580728cc2db0703541a6cf3af151dc44a47 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Wed, 1 Oct 2025 18:32:32 -0600 Subject: [PATCH 3/5] fix(metadata_handlers): distinguish HTTP errors from control flow errors 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> --- cmd/cloud-init-server/metadata_handlers.go | 40 ++++++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/cmd/cloud-init-server/metadata_handlers.go b/cmd/cloud-init-server/metadata_handlers.go index 90f3086d..559ae698 100644 --- a/cmd/cloud-init-server/metadata_handlers.go +++ b/cmd/cloud-init-server/metadata_handlers.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "net/http" "strings" @@ -51,29 +52,54 @@ func MetaDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) http var err error // If this request includes an id, it can be interrpreted as an impersonation request if urlId == "" { + log.Debug().Msg("no id specified in request, attempting to identify based on requesting IP") ip := getActualRequestIP(r) + log.Debug().Msgf("requesting IP is: %s", ip) // Get the component information from the SMD client id, err = smd.IDfromIP(ip) if err != nil { - log.Print(err) - http.Error(w, "Failed to retrieve node ID from IP address", http.StatusUnprocessableEntity) + log.Printf("did not find id from ip %s: %v", ip, err) + w.WriteHeader(http.StatusUnprocessableEntity) return } else { log.Printf("xname %s with ip %s found\n", id, ip) } + } else { + id = urlId } log.Debug().Msgf("Getting metadata for id: %s", id) smdComponent, err := smd.ComponentInformation(id) if err != nil { - log.Debug().Msgf("Failed to get component information for %s: %s", id, err) - // If the component information is not available, return a 404 - http.Error(w, "Node not found in SMD. Instance-data not available", http.StatusNotFound) + if esr, ok := err.(smdclient.ErrSMDResponse); ok { + var msg string + var status int + switch esr.HTTPResponse.StatusCode { + case http.StatusNotFound: + msg = fmt.Sprintf("node %s not found in SMD", id) + status = http.StatusNotFound + default: + msg = fmt.Sprintf("failed to get component information for node %s: %v", id, err) + status = http.StatusInternalServerError + } + http.Error(w, msg, status) + } else { + log.Debug().Msgf("failed to get component information for node %s: %s", id, err) + http.Error(w, fmt.Sprintf("internal error occurred fetching component information for node %s", id), http.StatusInternalServerError) + } return } groups, err := smd.GroupMembership(id) if err != nil { - // If the group information is not available, return an empty list - groups = []string{} + if esr, ok := err.(smdclient.ErrSMDResponse); ok { + switch esr.HTTPResponse.StatusCode { + case http.StatusBadRequest: + http.Error(w, fmt.Sprintf("%s is not a valid xname for SMD", id), http.StatusBadRequest) + return + default: + // If the group information is not available, return an empty list + groups = []string{} + } + } } bootIP, err := smd.IPfromID(id) if err != nil { From f91c3393de0d62912693571640471d924a486b0a Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Thu, 2 Oct 2025 10:14:18 -0600 Subject: [PATCH 4/5] fix(userdata_handlers): clarify not found error message 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> --- cmd/cloud-init-server/userdata_handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud-init-server/userdata_handlers.go b/cmd/cloud-init-server/userdata_handlers.go index 354b4a0a..9549c042 100644 --- a/cmd/cloud-init-server/userdata_handlers.go +++ b/cmd/cloud-init-server/userdata_handlers.go @@ -57,7 +57,7 @@ func GroupUserDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) } if !isUserInGroup(id, group, smd) { - http.Error(w, "Group not found", http.StatusNotFound) + http.Error(w, fmt.Sprintf("node %s is not a member of group %s (node and/or group may not exist in SMD)", id, group), http.StatusNotFound) return } From 1ca0633dbe35690bd1bb041e8a81cb0f94731b91 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Thu, 2 Oct 2025 10:54:14 -0600 Subject: [PATCH 5/5] fix(vendordata_handlers): improve error messages Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- cmd/cloud-init-server/vendordata_handlers.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/cmd/cloud-init-server/vendordata_handlers.go b/cmd/cloud-init-server/vendordata_handlers.go index 9b9e0f3b..4aa4aa69 100644 --- a/cmd/cloud-init-server/vendordata_handlers.go +++ b/cmd/cloud-init-server/vendordata_handlers.go @@ -32,11 +32,13 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht var err error // If this request includes an id, it can be interrpreted as an impersonation request if urlId == "" { + log.Debug().Msg("no id specified in request, attempting to identify based on requesting IP") ip := getActualRequestIP(r) + log.Debug().Msgf("requesting IP is: %s", ip) // Get the component information from the SMD client id, err = smd.IDfromIP(ip) if err != nil { - log.Print(err) + log.Printf("did not find id from ip %s: %v", ip, err) w.WriteHeader(http.StatusUnprocessableEntity) return } else { @@ -45,7 +47,18 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht } groups, err := smd.GroupMembership(id) if err != nil { - log.Debug().Msgf("Error getting group membership: %s", err) + if esr, ok := err.(smdclient.ErrSMDResponse); ok { + switch esr.HTTPResponse.StatusCode { + case http.StatusNotFound: + log.Warn().Msgf("node %s not found in SMD, include list will be empty", id) + case http.StatusBadRequest: + log.Warn().Msgf("node %s is an invalid xname in SMD, include list will be empty", id) + default: + log.Error().Err(err).Msg("unhandled HTTP response from SMD, include list will be empty") + } + } else { + log.Error().Err(err).Msgf("failed to get group membership for id %s, include list will be empty", id) + } } clusterDefaults, err := store.GetClusterDefaults() @@ -57,7 +70,7 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht } extendedInstanceData, err := store.GetInstanceInfo(id) if err != nil { - log.Err(err).Msg("Error getting instance info") + log.Err(err).Msgf("Error getting instance info for id %s", id) } if extendedInstanceData.CloudInitBaseURL != "" { baseUrl = extendedInstanceData.CloudInitBaseURL