From 671255b10592bd2a3f249d28b0bf293d06e6c52e Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Wed, 11 Jan 2017 10:48:24 -0800 Subject: [PATCH] Fix bug in saving host detail update time (#888) Saving a new detail update time when the host details were not actually updated caused detail updates to be missed. This PR fixes the existing test to catch the bug, and fixes the bug. --- server/service/service_osquery.go | 15 +++++++++++---- server/service/service_osquery_test.go | 11 ++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/server/service/service_osquery.go b/server/service/service_osquery.go index c3ac3cd6a6..6b773a123e 100644 --- a/server/service/service_osquery.go +++ b/server/service/service_osquery.go @@ -494,11 +494,13 @@ func (svc service) SubmitDistributedQueryResults(ctx context.Context, results ko return osqueryError{message: "failed to update host seen: " + err.Error()} } + detailUpdated := false labelResults := map[string]bool{} for query, rows := range results { switch { case strings.HasPrefix(query, hostDetailQueryPrefix): err = svc.ingestDetailQuery(&host, query, rows) + detailUpdated = true case strings.HasPrefix(query, hostLabelQueryPrefix): err = svc.ingestLabelQuery(host, query, rows, labelResults) case strings.HasPrefix(query, hostDistributedQueryPrefix): @@ -524,10 +526,15 @@ func (svc service) SubmitDistributedQueryResults(ctx context.Context, results ko } } - host.DetailUpdateTime = svc.clock.Now() - err = svc.ds.SaveHost(&host) - if err != nil { - return osqueryError{message: "failed to update host details: " + err.Error()} + if detailUpdated { + host.DetailUpdateTime = svc.clock.Now() + } + + if len(labelResults) > 0 || detailUpdated { + err = svc.ds.SaveHost(&host) + if err != nil { + return osqueryError{message: "failed to update host details: " + err.Error()} + } } return nil diff --git a/server/service/service_osquery_test.go b/server/service/service_osquery_test.go index ad1a1dfa7d..9094a7d9cd 100644 --- a/server/service/service_osquery_test.go +++ b/server/service/service_osquery_test.go @@ -590,9 +590,12 @@ func TestDetailQueries(t *testing.T) { // uptime assert.Equal(t, 1730893*time.Second, host.Uptime) - ctx = hostctx.NewContext(ctx, *host) + mockClock.AddTime(1 * time.Minute) // Now no detail queries should be required + host, err = ds.AuthenticateHost(nodeKey) + require.Nil(t, err) + ctx = hostctx.NewContext(ctx, *host) queries, err = svc.GetDistributedQueries(ctx) assert.Nil(t, err) assert.Len(t, queries, 0) @@ -600,6 +603,12 @@ func TestDetailQueries(t *testing.T) { // Advance clock and queries should exist again mockClock.AddTime(1*time.Hour + 1*time.Minute) + err = svc.SubmitDistributedQueryResults(ctx, kolide.OsqueryDistributedQueryResults{}, map[string]string{}) + require.Nil(t, err) + host, err = ds.AuthenticateHost(nodeKey) + require.Nil(t, err) + + ctx = hostctx.NewContext(ctx, *host) queries, err = svc.GetDistributedQueries(ctx) assert.Nil(t, err) assert.Len(t, queries, len(detailQueries))