From 6d9085da71ae186043e3e44b9aae0bce97f273f6 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Thu, 30 Mar 2017 08:31:05 -0700 Subject: [PATCH] Remove extraneous calls to MarkHostSeen (#1441) The seen time should only be updated once per request from the osquery agent to the Kolide server. We now do that only in AuthenticateHost (which every request besides enrollment must go through). --- server/service/service_osquery.go | 32 ++--------- server/service/service_osquery_test.go | 79 +++++++++++--------------- 2 files changed, 38 insertions(+), 73 deletions(-) diff --git a/server/service/service_osquery.go b/server/service/service_osquery.go index 4c303828d0..47f67a7728 100644 --- a/server/service/service_osquery.go +++ b/server/service/service_osquery.go @@ -35,6 +35,7 @@ func (svc service) AuthenticateHost(ctx context.Context, nodeKey string) (*kolid nodeInvalid: true, } } + host, err := svc.ds.AuthenticateHost(nodeKey) if err != nil { return nil, osqueryError{ @@ -42,10 +43,13 @@ func (svc service) AuthenticateHost(ctx context.Context, nodeKey string) (*kolid nodeInvalid: true, } } + + // Update the "seen" time used to calculate online status err = svc.ds.MarkHostSeen(host, svc.clock.Now()) if err != nil { - return nil, osqueryError{message: "failed to make host seen: " + err.Error()} + return nil, osqueryError{message: "failed to mark host seen: " + err.Error()} } + return host, nil } @@ -148,11 +152,6 @@ func (svc service) GetClientConfig(ctx context.Context) (*kolide.OsqueryConfig, } func (svc service) SubmitStatusLogs(ctx context.Context, logs []kolide.OsqueryStatusLog) error { - host, ok := hostctx.FromContext(ctx) - if !ok { - return osqueryError{message: "internal error: missing host from request context"} - } - for _, log := range logs { err := json.NewEncoder(svc.osqueryStatusLogWriter).Encode(log) if err != nil { @@ -160,20 +159,10 @@ func (svc service) SubmitStatusLogs(ctx context.Context, logs []kolide.OsquerySt } } - err := svc.ds.MarkHostSeen(&host, svc.clock.Now()) - if err != nil { - return osqueryError{message: "failed to update host seen: " + err.Error()} - } - return nil } func (svc service) SubmitResultLogs(ctx context.Context, logs []kolide.OsqueryResultLog) error { - host, ok := hostctx.FromContext(ctx) - if !ok { - return osqueryError{message: "internal error: missing host from request context"} - } - for _, log := range logs { err := json.NewEncoder(svc.osqueryResultLogWriter).Encode(log) if err != nil { @@ -181,11 +170,6 @@ func (svc service) SubmitResultLogs(ctx context.Context, logs []kolide.OsqueryRe } } - err := svc.ds.MarkHostSeen(&host, svc.clock.Now()) - if err != nil { - return osqueryError{message: "failed to update host seen: " + err.Error()} - } - return nil } @@ -536,11 +520,7 @@ func (svc service) SubmitDistributedQueryResults(ctx context.Context, results ko return osqueryError{message: "internal error: missing host from request context"} } - err := svc.ds.MarkHostSeen(&host, svc.clock.Now()) - if err != nil { - return osqueryError{message: "failed to update host seen: " + err.Error()} - } - + var err error detailUpdated := false labelResults := map[uint]bool{} for query, rows := range results { diff --git a/server/service/service_osquery_test.go b/server/service/service_osquery_test.go index 309c7f94fc..80615e1daa 100644 --- a/server/service/service_osquery_test.go +++ b/server/service/service_osquery_test.go @@ -56,10 +56,38 @@ func TestEnrollAgentIncorrectEnrollSecret(t *testing.T) { assert.Len(t, hosts, 0) } -func TestSubmitStatusLogs(t *testing.T) { +func TestAuthenticateHost(t *testing.T) { ds, svc, mockClock := setupOsqueryTests(t) ctx := context.Background() + nodeKey, err := svc.EnrollAgent(ctx, "", "host123") + require.Nil(t, err) + + mockClock.AddTime(1 * time.Minute) + + host, err := svc.AuthenticateHost(ctx, nodeKey) + require.Nil(t, err) + + // Verify that the update time is set appropriately + checkHost, err := ds.Host(host.ID) + require.Nil(t, err) + assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) + + // Advance clock time and check that seen time is updated + mockClock.AddTime(1*time.Minute + 27*time.Second) + + _, err = svc.AuthenticateHost(ctx, nodeKey) + require.Nil(t, err) + + checkHost, err = ds.Host(host.ID) + require.Nil(t, err) + assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) +} + +func TestSubmitStatusLogs(t *testing.T) { + ds, svc, _ := setupOsqueryTests(t) + ctx := context.Background() + _, err := svc.EnrollAgent(ctx, "", "host123") require.Nil(t, err) @@ -67,18 +95,11 @@ func TestSubmitStatusLogs(t *testing.T) { require.Nil(t, err) require.Len(t, hosts, 1) host := hosts[0] + ctx = hostctx.NewContext(ctx, *host) // Hack to get at the service internals and modify the writer serv := ((svc.(validationMiddleware)).Service).(service) - // Error due to missing host - err = serv.SubmitResultLogs(ctx, []kolide.OsqueryResultLog{}) - assert.NotNil(t, err) - assert.Contains(t, err.Error(), "missing host") - - // Add that host - ctx = hostctx.NewContext(ctx, *host) - var statusBuf bytes.Buffer serv.osqueryStatusLogWriter = &statusBuf @@ -104,25 +125,10 @@ func TestSubmitStatusLogs(t *testing.T) { assert.JSONEq(t, logs[i], line) } } - - // Verify that the update time is set appropriately - checkHost, err := ds.Host(host.ID) - assert.Nil(t, err) - assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) - - // Advance clock time and check that time is updated on new logs - mockClock.AddTime(1 * time.Minute) - - err = serv.SubmitStatusLogs(ctx, []kolide.OsqueryStatusLog{}) - assert.Nil(t, err) - - checkHost, err = ds.Host(host.ID) - assert.Nil(t, err) - assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) } func TestSubmitResultLogs(t *testing.T) { - ds, svc, mockClock := setupOsqueryTests(t) + ds, svc, _ := setupOsqueryTests(t) ctx := context.Background() _, err := svc.EnrollAgent(ctx, "", "host123") @@ -132,17 +138,11 @@ func TestSubmitResultLogs(t *testing.T) { require.Nil(t, err) require.Len(t, hosts, 1) host := hosts[0] + ctx = hostctx.NewContext(ctx, *host) // Hack to get at the service internals and modify the writer serv := ((svc.(validationMiddleware)).Service).(service) - // Error due to missing host - err = serv.SubmitResultLogs(ctx, []kolide.OsqueryResultLog{}) - assert.NotNil(t, err) - assert.Contains(t, err.Error(), "missing host") - - ctx = hostctx.NewContext(ctx, *host) - var resultBuf bytes.Buffer serv.osqueryResultLogWriter = &resultBuf @@ -169,21 +169,6 @@ func TestSubmitResultLogs(t *testing.T) { assert.JSONEq(t, logs[i], line) } } - - // Verify that the update time is set appropriately - checkHost, err := ds.Host(host.ID) - assert.Nil(t, err) - assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) - - // Advance clock time and check that time is updated on new logs - mockClock.AddTime(1 * time.Minute) - - err = serv.SubmitResultLogs(ctx, []kolide.OsqueryResultLog{}) - assert.Nil(t, err) - - checkHost, err = ds.Host(host.ID) - assert.Nil(t, err) - assert.Equal(t, mockClock.Now(), checkHost.UpdatedAt) } func TestHostDetailQueries(t *testing.T) {