From 7b46df569ce2070177238d26dbda3eef1c1a91b5 Mon Sep 17 00:00:00 2001 From: Tomas Touceda Date: Thu, 23 Dec 2021 16:57:43 -0300 Subject: [PATCH] Update return values to be null if the data is not available (#3490) * Update return values to be null if the data is not available * Return nil in the parent object if neither is available * Improve readability of the code --- server/fleet/hosts.go | 4 +- server/service/hosts.go | 37 +++++++---- server/service/integration_core_test.go | 87 ++++++++++++++++++++++--- 3 files changed, 102 insertions(+), 26 deletions(-) diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index 473925e80d..ed4c3cf8da 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -266,6 +266,6 @@ type HostMDM struct { } type MacadminsData struct { - Munki HostMunkiInfo `json:"munki"` - MDM HostMDM `json:"mobile_device_management"` + Munki *HostMunkiInfo `json:"munki"` + MDM *HostMDM `json:"mobile_device_management"` } diff --git a/server/service/hosts.go b/server/service/hosts.go index c5d3dc94cb..1d9804e73e 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -718,29 +718,38 @@ func (svc *Service) MacadminsData(ctx context.Context, id uint) (*fleet.Macadmin return nil, err } - version, err := svc.ds.GetMunkiVersion(ctx, id) - if err != nil && !fleet.IsNotFound(err) { + var munkiInfo *fleet.HostMunkiInfo + switch version, err := svc.ds.GetMunkiVersion(ctx, id); { + case err != nil && !fleet.IsNotFound(err): return nil, err + case err == nil: + munkiInfo = &fleet.HostMunkiInfo{Version: version} } - enrolled, serverURL, installedFromDep, err := svc.ds.GetMDM(ctx, id) - if err != nil && !fleet.IsNotFound(err) { + var mdm *fleet.HostMDM + switch enrolled, serverURL, installedFromDep, err := svc.ds.GetMDM(ctx, id); { + case err != nil && !fleet.IsNotFound(err): return nil, err + case err == nil: + enrollmentStatus := "Unenrolled" + if enrolled && !installedFromDep { + enrollmentStatus = "Enrolled (manual)" + } else if enrolled && installedFromDep { + enrollmentStatus = "Enrolled (automated)" + } + mdm = &fleet.HostMDM{ + EnrollmentStatus: enrollmentStatus, + ServerURL: serverURL, + } } - enrollmentStatus := "Unenrolled" - if enrolled && !installedFromDep { - enrollmentStatus = "Enrolled (manual)" - } else if enrolled && installedFromDep { - enrollmentStatus = "Enrolled (automated)" + if munkiInfo == nil && mdm == nil { + return nil, nil } data := &fleet.MacadminsData{ - Munki: fleet.HostMunkiInfo{Version: version}, - MDM: fleet.HostMDM{ - EnrollmentStatus: enrollmentStatus, - ServerURL: serverURL, - }, + Munki: munkiInfo, + MDM: mdm, } return data, nil diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index ed05a56d97..a6dd4f4812 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -1600,7 +1600,7 @@ func (s *integrationTestSuite) TestGetMacadminsData() { ctx := context.Background() - host, err := s.ds.NewHost(ctx, &fleet.Host{ + hostAll, err := s.ds.NewHost(ctx, &fleet.Host{ DetailUpdatedAt: time.Now(), LabelUpdatedAt: time.Now(), PolicyUpdatedAt: time.Now(), @@ -1610,36 +1610,103 @@ func (s *integrationTestSuite) TestGetMacadminsData() { Hostname: t.Name() + "foo.local", PrimaryIP: "192.168.1.1", PrimaryMac: "30-65-EC-6F-C4-58", + OsqueryHostID: "1", }) require.NoError(t, err) - require.NotNil(t, host) + require.NotNil(t, hostAll) - require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, host.ID, true, "url", false)) - require.NoError(t, s.ds.SetOrUpdateMunkiVersion(ctx, host.ID, "1.3.0")) + hostNothing, err := s.ds.NewHost(ctx, &fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: t.Name() + "2", + UUID: t.Name() + "2", + Hostname: t.Name() + "foo.local2", + PrimaryIP: "192.168.1.2", + PrimaryMac: "30-65-EC-6F-C4-59", + OsqueryHostID: "2", + }) + require.NoError(t, err) + require.NotNil(t, hostNothing) + + hostOnlyMunki, err := s.ds.NewHost(ctx, &fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: t.Name() + "3", + UUID: t.Name() + "3", + Hostname: t.Name() + "foo.local3", + PrimaryIP: "192.168.1.3", + PrimaryMac: "30-65-EC-6F-C4-5F", + OsqueryHostID: "3", + }) + require.NoError(t, err) + require.NotNil(t, hostOnlyMunki) + + hostOnlyMDM, err := s.ds.NewHost(ctx, &fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: t.Name() + "4", + UUID: t.Name() + "4", + Hostname: t.Name() + "foo.local4", + PrimaryIP: "192.168.1.4", + PrimaryMac: "30-65-EC-6F-C4-5A", + OsqueryHostID: "4", + }) + require.NoError(t, err) + require.NotNil(t, hostOnlyMDM) + + require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, hostAll.ID, true, "url", false)) + require.NoError(t, s.ds.SetOrUpdateMunkiVersion(ctx, hostAll.ID, "1.3.0")) macadminsData := getMacadminsDataResponse{} - s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", host.ID), nil, http.StatusOK, &macadminsData) + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostAll.ID), nil, http.StatusOK, &macadminsData) require.NotNil(t, macadminsData.Macadmins) assert.Equal(t, "url", macadminsData.Macadmins.MDM.ServerURL) assert.Equal(t, "Enrolled (manual)", macadminsData.Macadmins.MDM.EnrollmentStatus) assert.Equal(t, "1.3.0", macadminsData.Macadmins.Munki.Version) - require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, host.ID, true, "url2", true)) - require.NoError(t, s.ds.SetOrUpdateMunkiVersion(ctx, host.ID, "1.5.0")) + require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, hostAll.ID, true, "url2", true)) + require.NoError(t, s.ds.SetOrUpdateMunkiVersion(ctx, hostAll.ID, "1.5.0")) macadminsData = getMacadminsDataResponse{} - s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", host.ID), nil, http.StatusOK, &macadminsData) + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostAll.ID), nil, http.StatusOK, &macadminsData) require.NotNil(t, macadminsData.Macadmins) assert.Equal(t, "url2", macadminsData.Macadmins.MDM.ServerURL) assert.Equal(t, "Enrolled (automated)", macadminsData.Macadmins.MDM.EnrollmentStatus) assert.Equal(t, "1.5.0", macadminsData.Macadmins.Munki.Version) - require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, host.ID, false, "url2", false)) + require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, hostAll.ID, false, "url2", false)) macadminsData = getMacadminsDataResponse{} - s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", host.ID), nil, http.StatusOK, &macadminsData) + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostAll.ID), nil, http.StatusOK, &macadminsData) require.NotNil(t, macadminsData.Macadmins) assert.Equal(t, "Unenrolled", macadminsData.Macadmins.MDM.EnrollmentStatus) + + // nothing returns null + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostNothing.ID), nil, http.StatusOK, &macadminsData) + require.Nil(t, macadminsData.Macadmins) + + // only munki info returns null on mdm + require.NoError(t, s.ds.SetOrUpdateMunkiVersion(ctx, hostOnlyMunki.ID, "3.2.0")) + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostOnlyMunki.ID), nil, http.StatusOK, &macadminsData) + require.NotNil(t, macadminsData.Macadmins) + require.Nil(t, macadminsData.Macadmins.MDM) + require.NotNil(t, macadminsData.Macadmins.Munki) + assert.Equal(t, "3.2.0", macadminsData.Macadmins.Munki.Version) + + // only mdm returns null on munki info + require.NoError(t, s.ds.SetOrUpdateMDMData(ctx, hostOnlyMDM.ID, true, "AAA", true)) + s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d/macadmins", hostOnlyMDM.ID), nil, http.StatusOK, &macadminsData) + require.NotNil(t, macadminsData.Macadmins) + require.NotNil(t, macadminsData.Macadmins.MDM) + require.Nil(t, macadminsData.Macadmins.Munki) + assert.Equal(t, "AAA", macadminsData.Macadmins.MDM.ServerURL) + assert.Equal(t, "Enrolled (automated)", macadminsData.Macadmins.MDM.EnrollmentStatus) } func (s *integrationTestSuite) TestLabels() {