diff --git a/changes/10880-api-fleet-hosts-cleanup b/changes/10880-api-fleet-hosts-cleanup new file mode 100644 index 0000000000..66cef5ce8a --- /dev/null +++ b/changes/10880-api-fleet-hosts-cleanup @@ -0,0 +1 @@ +* Fix a HTTP 500 on `GET /api/_version_/fleet/hosts` returned when `mdm_enrollment_status` is invalid. diff --git a/server/fleet/hostresponse.go b/server/fleet/hostresponse.go index cc5aa163dd..8042190bb7 100644 --- a/server/fleet/hostresponse.go +++ b/server/fleet/hostresponse.go @@ -19,10 +19,10 @@ type HostResponse struct { } // HostResponseForHost returns a HostResponse from Host with Geolocation. -func HostResponseForHost(ctx context.Context, svc Service, host *Host) (*HostResponse, error) { +func HostResponseForHost(ctx context.Context, svc Service, host *Host) *HostResponse { hr := HostResponseForHostCheap(host) hr.Geolocation = svc.LookupGeoIP(ctx, host.PublicIP) - return hr, nil + return hr } // HostResponseForHostCheap returns a new HostResponse from a Host without computing Geolocation. diff --git a/server/service/hosts.go b/server/service/hosts.go index 97a5f49f89..b2ca3afa79 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -106,11 +106,7 @@ func listHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Servi hostResponses := make([]fleet.HostResponse, len(hosts)) for i, host := range hosts { - h, err := fleet.HostResponseForHost(ctx, svc, host) - if err != nil { - return listHostsResponse{Err: err}, nil - } - + h := fleet.HostResponseForHost(ctx, svc, host) hostResponses[i] = *h } return listHostsResponse{ @@ -1337,10 +1333,7 @@ func hostsReportEndpoint(ctx context.Context, request interface{}, svc fleet.Ser hostResps := make([]*fleet.HostResponse, len(hosts)) for i, h := range hosts { - hr, err := fleet.HostResponseForHost(ctx, svc, h) - if err != nil { - return hostsReportResponse{Err: err}, nil - } + hr := fleet.HostResponseForHost(ctx, svc, h) hostResps[i] = hr } return hostsReportResponse{Columns: cols, Hosts: hostResps}, nil diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 7fdfbf3ae6..ea36e7a3b5 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -1244,9 +1244,6 @@ func (s *integrationTestSuite) TestListHosts() { }) s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &resp) - for _, h := range resp.Hosts { - fmt.Println("host", fmt.Sprintf("%+v", h.Host.HardwareSerial)) - } // set MDM information for another host installed from DEP and pending enrollment to Fleet MDM pendingMDMHost, err := s.ds.NewHost(context.Background(), &fleet.Host{ @@ -1312,7 +1309,11 @@ func (s *integrationTestSuite) TestListHosts() { assert.Equal(t, mdmID, resp.MDMSolution.ID) resp = listHostsResponse{} - s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusInternalServerError, &resp, "mdm_enrollment_status", "invalid-status") // TODO: to be addressed by #4406 + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusBadRequest, &resp, "mdm_enrollment_status", "invalid-status") + + // Filter by inexistent software. + resp = listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusNotFound, &resp, "software_id", fmt.Sprint(9999)) // set munki information on a host require.NoError(t, s.ds.SetOrUpdateMunkiInfo(context.Background(), host.ID, "1.2.3", []string{"err"}, []string{"warn"})) diff --git a/server/service/labels.go b/server/service/labels.go index f39aadbedd..799b17fae5 100644 --- a/server/service/labels.go +++ b/server/service/labels.go @@ -272,11 +272,7 @@ func listHostsInLabelEndpoint(ctx context.Context, request interface{}, svc flee hostResponses := make([]fleet.HostResponse, len(hosts)) for i, host := range hosts { - h, err := fleet.HostResponseForHost(ctx, svc, host) - if err != nil { - return listHostsResponse{Err: err}, nil - } - + h := fleet.HostResponseForHost(ctx, svc, host) hostResponses[i] = *h } return listHostsResponse{Hosts: hostResponses, MDMSolution: mdmSolution}, nil diff --git a/server/service/transport.go b/server/service/transport.go index 09d90d4af5..5d7296ddbf 100644 --- a/server/service/transport.go +++ b/server/service/transport.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "net/url" @@ -313,7 +314,7 @@ func hostListOptionsFromRequest(r *http.Request) (fleet.HostListOptions, error) case "": // No error when unset default: - return hopt, ctxerr.Errorf(r.Context(), "invalid mdm enrollment status %s", enrollmentStatus) + return hopt, ctxerr.Wrap(r.Context(), badRequest(fmt.Sprintf("invalid mdm enrollment status %s", enrollmentStatus))) } macOSSettingsStatus := r.URL.Query().Get("macos_settings")