Skip to content

Commit d25ac41

Browse files
synackdsynackd
authored
fix: improve error handling for user-/meta-/vendor-data handlers (#93)
* fix(SMDClient): GroupMembership: returhn error if id is empty Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> * 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> * 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> * 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> * fix(vendordata_handlers): improve error messages Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --------- Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
1 parent 43a2187 commit d25ac41

5 files changed

Lines changed: 81 additions & 16 deletions

File tree

‎cmd/cloud-init-server/metadata_handlers.go‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"fmt"
45
"net/http"
56
"strings"
67

@@ -51,29 +52,54 @@ func MetaDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) http
5152
var err error
5253
// If this request includes an id, it can be interrpreted as an impersonation request
5354
if urlId == "" {
55+
log.Debug().Msg("no id specified in request, attempting to identify based on requesting IP")
5456
ip := getActualRequestIP(r)
57+
log.Debug().Msgf("requesting IP is: %s", ip)
5558
// Get the component information from the SMD client
5659
id, err = smd.IDfromIP(ip)
5760
if err != nil {
58-
log.Print(err)
59-
http.Error(w, "Failed to retrieve node ID from IP address", http.StatusUnprocessableEntity)
61+
log.Printf("did not find id from ip %s: %v", ip, err)
62+
w.WriteHeader(http.StatusUnprocessableEntity)
6063
return
6164
} else {
6265
log.Printf("xname %s with ip %s found\n", id, ip)
6366
}
67+
} else {
68+
id = urlId
6469
}
6570
log.Debug().Msgf("Getting metadata for id: %s", id)
6671
smdComponent, err := smd.ComponentInformation(id)
6772
if err != nil {
68-
log.Debug().Msgf("Failed to get component information for %s: %s", id, err)
69-
// If the component information is not available, return a 404
70-
http.Error(w, "Node not found in SMD. Instance-data not available", http.StatusNotFound)
73+
if esr, ok := err.(smdclient.ErrSMDResponse); ok {
74+
var msg string
75+
var status int
76+
switch esr.HTTPResponse.StatusCode {
77+
case http.StatusNotFound:
78+
msg = fmt.Sprintf("node %s not found in SMD", id)
79+
status = http.StatusNotFound
80+
default:
81+
msg = fmt.Sprintf("failed to get component information for node %s: %v", id, err)
82+
status = http.StatusInternalServerError
83+
}
84+
http.Error(w, msg, status)
85+
} else {
86+
log.Debug().Msgf("failed to get component information for node %s: %s", id, err)
87+
http.Error(w, fmt.Sprintf("internal error occurred fetching component information for node %s", id), http.StatusInternalServerError)
88+
}
7189
return
7290
}
7391
groups, err := smd.GroupMembership(id)
7492
if err != nil {
75-
// If the group information is not available, return an empty list
76-
groups = []string{}
93+
if esr, ok := err.(smdclient.ErrSMDResponse); ok {
94+
switch esr.HTTPResponse.StatusCode {
95+
case http.StatusBadRequest:
96+
http.Error(w, fmt.Sprintf("%s is not a valid xname for SMD", id), http.StatusBadRequest)
97+
return
98+
default:
99+
// If the group information is not available, return an empty list
100+
groups = []string{}
101+
}
102+
}
77103
}
78104
bootIP, err := smd.IPfromID(id)
79105
if err != nil {

‎cmd/cloud-init-server/userdata_handlers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func GroupUserDataHandler(smd smdclient.SMDClientInterface, store cistore.Store)
5757
}
5858

5959
if !isUserInGroup(id, group, smd) {
60-
http.Error(w, "Group not found", http.StatusNotFound)
60+
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)
6161
return
6262
}
6363

‎cmd/cloud-init-server/vendordata_handlers.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,13 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht
3232
var err error
3333
// If this request includes an id, it can be interrpreted as an impersonation request
3434
if urlId == "" {
35+
log.Debug().Msg("no id specified in request, attempting to identify based on requesting IP")
3536
ip := getActualRequestIP(r)
37+
log.Debug().Msgf("requesting IP is: %s", ip)
3638
// Get the component information from the SMD client
3739
id, err = smd.IDfromIP(ip)
3840
if err != nil {
39-
log.Print(err)
41+
log.Printf("did not find id from ip %s: %v", ip, err)
4042
w.WriteHeader(http.StatusUnprocessableEntity)
4143
return
4244
} else {
@@ -45,7 +47,18 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht
4547
}
4648
groups, err := smd.GroupMembership(id)
4749
if err != nil {
48-
log.Debug().Msgf("Error getting group membership: %s", err)
50+
if esr, ok := err.(smdclient.ErrSMDResponse); ok {
51+
switch esr.HTTPResponse.StatusCode {
52+
case http.StatusNotFound:
53+
log.Warn().Msgf("node %s not found in SMD, include list will be empty", id)
54+
case http.StatusBadRequest:
55+
log.Warn().Msgf("node %s is an invalid xname in SMD, include list will be empty", id)
56+
default:
57+
log.Error().Err(err).Msg("unhandled HTTP response from SMD, include list will be empty")
58+
}
59+
} else {
60+
log.Error().Err(err).Msgf("failed to get group membership for id %s, include list will be empty", id)
61+
}
4962
}
5063

5164
clusterDefaults, err := store.GetClusterDefaults()
@@ -57,7 +70,7 @@ func VendorDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) ht
5770
}
5871
extendedInstanceData, err := store.GetInstanceInfo(id)
5972
if err != nil {
60-
log.Err(err).Msg("Error getting instance info")
73+
log.Err(err).Msgf("Error getting instance info for id %s", id)
6174
}
6275
if extendedInstanceData.CloudInitBaseURL != "" {
6376
baseUrl = extendedInstanceData.CloudInitBaseURL

‎internal/smdclient/SMDclient.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,6 @@ type SMDClientInterface interface {
3838
// golang lint
3939
// Expand this client to handle more of the SMD API and work more directly with the resources it manages
4040

41-
var (
42-
ErrUnmarshal = errors.New("cannot unmarshal JSON")
43-
)
44-
4541
// SMDClient is a client for SMD
4642
type SMDClient struct {
4743
clusterName string
@@ -206,6 +202,10 @@ func (s *SMDClient) getSMD(ep string, smd interface{}) error {
206202
defer func() {
207203
_ = resp.Body.Close() // ignoring error on deferred Close
208204
}()
205+
if resp.StatusCode >= 400 {
206+
log.Error().Msgf("SMD GET request went through, but returned unsuccessful HTTP response (HTTP %d)", resp.StatusCode)
207+
return ErrSMDResponse{HTTPResponse: resp}
208+
}
209209
body, err := io.ReadAll(resp.Body)
210210
if err != nil {
211211
log.Error().Err(err).Msg("failed to read response body")
@@ -339,7 +339,9 @@ func (s *SMDClient) MACfromID(id string) (string, error) {
339339
// GroupMembership returns the group labels for the xname with the given ID
340340
func (s *SMDClient) GroupMembership(id string) ([]string, error) {
341341
if id == "" {
342-
log.Err(errors.New("ID is empty")).Msg("ID is empty")
342+
err := errors.New("ID is empty")
343+
log.Err(err).Msg("failed to get group membership")
344+
return []string{}, err
343345
}
344346
ml := new(sm.Membership)
345347
ep := "/hsm/v2/memberships/" + id
@@ -352,6 +354,9 @@ func (s *SMDClient) GroupMembership(id string) ([]string, error) {
352354

353355
func (s *SMDClient) ComponentInformation(id string) (base.Component, error) {
354356
var node base.Component
357+
if strings.Trim(id, " \t") == "" {
358+
return node, ErrEmptyID
359+
}
355360
ep := "/hsm/v2/State/Components/" + id
356361
err := s.getSMD(ep, &node)
357362
if err != nil {

‎internal/smdclient/errors.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package smdclient
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"net/http"
7+
)
8+
9+
var (
10+
ErrUnmarshal = errors.New("cannot unmarshal JSON")
11+
ErrEmptyID = errors.New("empty id")
12+
)
13+
14+
// ErrSMDResponse contains the HTTP response of a REST API request to SMD.
15+
type ErrSMDResponse struct {
16+
HTTPResponse *http.Response
17+
}
18+
19+
func (esr ErrSMDResponse) Error() string {
20+
return fmt.Sprintf("SMD response returned %s", esr.HTTPResponse.Status)
21+
}

0 commit comments

Comments
 (0)