From d70071a0a8f68f2ffd7b7a631f82fb6faa38d3ba Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 22 Nov 2023 10:38:18 -0500 Subject: [PATCH 01/12] Fall back to finding DISPLAY from xorg process --- ee/desktop/runner/runner.go | 12 +++--- ee/desktop/runner/runner_linux.go | 67 ++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index cf7fa1c65..99860fb0f 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -485,11 +485,13 @@ func (r *DesktopUsersProcessesRunner) writeDefaultMenuTemplateFile() { } func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { - if !r.processSpawningEnabled { - // Desktop is disabled, kill any existing desktop user processes - r.killDesktopProcesses() - return nil - } + /* + if !r.processSpawningEnabled { + // Desktop is disabled, kill any existing desktop user processes + r.killDesktopProcesses() + return nil + } + */ executablePath, err := r.determineExecutablePath() if err != nil { diff --git a/ee/desktop/runner/runner_linux.go b/ee/desktop/runner/runner_linux.go index 188d78ae7..c24d43511 100644 --- a/ee/desktop/runner/runner_linux.go +++ b/ee/desktop/runner/runner_linux.go @@ -139,7 +139,7 @@ func (r *DesktopUsersProcessesRunner) userEnvVars(ctx context.Context, uid strin sessionType := strings.Trim(string(typeOutput), "\n") if sessionType == "x11" { - envVars["DISPLAY"] = r.displayFromX11(ctx, session) + envVars["DISPLAY"] = r.displayFromX11(ctx, session, int32(uidInt)) break } else if sessionType == "wayland" { envVars["DISPLAY"] = r.displayFromXwayland(ctx, int32(uidInt)) @@ -164,7 +164,7 @@ func (r *DesktopUsersProcessesRunner) userEnvVars(ctx context.Context, uid strin return envVars } -func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, session string) string { +func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, session string, uid int32) string { // We can read $DISPLAY from the session properties cmd, err := allowedcmd.Loginctl(ctx, "show-session", session, "--value", "--property=Display") if err != nil { @@ -172,7 +172,7 @@ func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, sessio "msg", "could not create command to get Display from user session", "err", err, ) - return defaultDisplay + return r.displayFromXorgProcess(ctx, uid) } xDisplayOutput, err := cmd.Output() if err != nil { @@ -180,17 +180,74 @@ func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, sessio "msg", "could not get Display from user session", "err", err, ) - return defaultDisplay + return r.displayFromXorgProcess(ctx, uid) } display := strings.Trim(string(xDisplayOutput), "\n") if display == "" { - return defaultDisplay + return r.displayFromXorgProcess(ctx, uid) } return display } +func (r *DesktopUsersProcessesRunner) displayFromXorgProcess(ctx context.Context, uid int32) string { + processes, err := process.ProcessesWithContext(ctx) + if err != nil { + level.Debug(r.logger).Log( + "msg", "could not query processes to find Xorg process", + "err", err, + ) + return defaultDisplay + } + + for _, p := range processes { + cmdline, err := p.CmdlineWithContext(ctx) + if err != nil { + level.Debug(r.logger).Log( + "msg", "could not get cmdline slice for process", + "err", err, + ) + continue + } + + if !strings.Contains(cmdline, "Xorg") { + continue + } + + // We have an Xorg process -- check to make sure it's for our running user + uids, err := p.UidsWithContext(ctx) + if err != nil { + level.Debug(r.logger).Log( + "msg", "could not get uids for process", + "err", err, + ) + continue + } + uidMatch := false + for _, procUid := range uids { + if procUid == uid { + uidMatch = true + break + } + } + + if uidMatch { + // We have a match! Grab the display value. The Xorg process looks like: + // /usr/lib/xorg/Xorg :20 -auth /home//.Xauthority -nolisten tcp -noreset -logfile /dev/null -verbose 3 -config /tmp/chrome_remote_desktop_j5rldjlk.conf + cmdlineArgs := strings.Split(cmdline, " ") + if len(cmdlineArgs) < 2 { + // Process is somehow malformed or not what we're looking for -- continue so we can evaluate the following process + continue + } + + return cmdlineArgs[1] + } + } + + return defaultDisplay +} + func (r *DesktopUsersProcessesRunner) displayFromXwayland(ctx context.Context, uid int32) string { //For wayland, DISPLAY is not included in loginctl show-session output -- in GNOME, // Mutter spawns Xwayland and sets $DISPLAY at the same time. Find $DISPLAY by finding From fdea31fa69c6d631b827f1c988c7d3308266ec76 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 22 Nov 2023 11:06:18 -0500 Subject: [PATCH 02/12] Remove debugging change --- ee/desktop/runner/runner.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index 99860fb0f..cf7fa1c65 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -485,13 +485,11 @@ func (r *DesktopUsersProcessesRunner) writeDefaultMenuTemplateFile() { } func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { - /* - if !r.processSpawningEnabled { - // Desktop is disabled, kill any existing desktop user processes - r.killDesktopProcesses() - return nil - } - */ + if !r.processSpawningEnabled { + // Desktop is disabled, kill any existing desktop user processes + r.killDesktopProcesses() + return nil + } executablePath, err := r.determineExecutablePath() if err != nil { From 9748457356886e33ac67152a5a259febebb78c98 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 22 Nov 2023 11:52:34 -0500 Subject: [PATCH 03/12] Try looking for Xvfb process too --- ee/desktop/runner/runner.go | 12 +++++++----- ee/desktop/runner/runner_linux.go | 19 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index cf7fa1c65..99860fb0f 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -485,11 +485,13 @@ func (r *DesktopUsersProcessesRunner) writeDefaultMenuTemplateFile() { } func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { - if !r.processSpawningEnabled { - // Desktop is disabled, kill any existing desktop user processes - r.killDesktopProcesses() - return nil - } + /* + if !r.processSpawningEnabled { + // Desktop is disabled, kill any existing desktop user processes + r.killDesktopProcesses() + return nil + } + */ executablePath, err := r.determineExecutablePath() if err != nil { diff --git a/ee/desktop/runner/runner_linux.go b/ee/desktop/runner/runner_linux.go index c24d43511..ce168f542 100644 --- a/ee/desktop/runner/runner_linux.go +++ b/ee/desktop/runner/runner_linux.go @@ -172,7 +172,7 @@ func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, sessio "msg", "could not create command to get Display from user session", "err", err, ) - return r.displayFromXorgProcess(ctx, uid) + return r.displayFromXDisplayServerProcess(ctx, uid) } xDisplayOutput, err := cmd.Output() if err != nil { @@ -180,22 +180,22 @@ func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, sessio "msg", "could not get Display from user session", "err", err, ) - return r.displayFromXorgProcess(ctx, uid) + return r.displayFromXDisplayServerProcess(ctx, uid) } display := strings.Trim(string(xDisplayOutput), "\n") if display == "" { - return r.displayFromXorgProcess(ctx, uid) + return r.displayFromXDisplayServerProcess(ctx, uid) } return display } -func (r *DesktopUsersProcessesRunner) displayFromXorgProcess(ctx context.Context, uid int32) string { +func (r *DesktopUsersProcessesRunner) displayFromXDisplayServerProcess(ctx context.Context, uid int32) string { processes, err := process.ProcessesWithContext(ctx) if err != nil { level.Debug(r.logger).Log( - "msg", "could not query processes to find Xorg process", + "msg", "could not query processes to find display server process", "err", err, ) return defaultDisplay @@ -211,11 +211,11 @@ func (r *DesktopUsersProcessesRunner) displayFromXorgProcess(ctx context.Context continue } - if !strings.Contains(cmdline, "Xorg") { + if !strings.Contains(cmdline, "Xorg") && !strings.Contains(cmdline, "Xvfb") { continue } - // We have an Xorg process -- check to make sure it's for our running user + // We have an Xorg or Xvfb process -- check to make sure it's for our running user uids, err := p.UidsWithContext(ctx) if err != nil { level.Debug(r.logger).Log( @@ -233,8 +233,11 @@ func (r *DesktopUsersProcessesRunner) displayFromXorgProcess(ctx context.Context } if uidMatch { - // We have a match! Grab the display value. The Xorg process looks like: + // We have a match! Grab the display value. + // The Xorg process looks like: // /usr/lib/xorg/Xorg :20 -auth /home//.Xauthority -nolisten tcp -noreset -logfile /dev/null -verbose 3 -config /tmp/chrome_remote_desktop_j5rldjlk.conf + // The Xvfb process looks like: + // Xvfb :20 -auth /home//.Xauthority -nolisten tcp -noreset -screen 0 3840x2560x24 cmdlineArgs := strings.Split(cmdline, " ") if len(cmdlineArgs) < 2 { // Process is somehow malformed or not what we're looking for -- continue so we can evaluate the following process From 1fa39d0d1249931ab22a31fcab76dee32af204ca Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 22 Nov 2023 12:11:42 -0500 Subject: [PATCH 04/12] Remove debugging change --- ee/desktop/runner/runner.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index 99860fb0f..cf7fa1c65 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -485,13 +485,11 @@ func (r *DesktopUsersProcessesRunner) writeDefaultMenuTemplateFile() { } func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { - /* - if !r.processSpawningEnabled { - // Desktop is disabled, kill any existing desktop user processes - r.killDesktopProcesses() - return nil - } - */ + if !r.processSpawningEnabled { + // Desktop is disabled, kill any existing desktop user processes + r.killDesktopProcesses() + return nil + } executablePath, err := r.determineExecutablePath() if err != nil { From c2834219cdd0fa7cd07047f1b3df62dc8784ebd0 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 11:48:41 -0500 Subject: [PATCH 05/12] Add osq instance history to osq checkup --- pkg/debug/checkups/osquery.go | 51 ++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/pkg/debug/checkups/osquery.go b/pkg/debug/checkups/osquery.go index 981d843d6..b27d1d910 100644 --- a/pkg/debug/checkups/osquery.go +++ b/pkg/debug/checkups/osquery.go @@ -10,13 +10,14 @@ import ( "time" "github.com/kolide/launcher/pkg/agent/types" + "github.com/kolide/launcher/pkg/osquery/runtime/history" ) type osqueryCheckup struct { - k types.Knapsack - status Status - executionTimes map[string]any // maps command to how long it took to run, in ms - summary string + k types.Knapsack + status Status + data map[string]any + summary string } func (o *osqueryCheckup) Name() string { @@ -24,8 +25,9 @@ func (o *osqueryCheckup) Name() string { } func (o *osqueryCheckup) Run(ctx context.Context, extraWriter io.Writer) error { + o.data = make(map[string]any) + // Determine passing status by running osqueryd --version - o.executionTimes = make(map[string]any) if osqueryVersion, err := o.version(ctx); err != nil { o.status = Failing return fmt.Errorf("running osqueryd version: %w", err) @@ -39,11 +41,15 @@ func (o *osqueryCheckup) Run(ctx context.Context, extraWriter io.Writer) error { return fmt.Errorf("running launcher interactive: %w", err) } + // Retrieve osquery instance history to see if we have an abnormal number of restarts + o.instanceHistory(ctx) + return nil } func (o *osqueryCheckup) version(ctx context.Context) (string, error) { osquerydPath := o.k.LatestOsquerydPath(ctx) + o.data["osqueryd_path"] = osquerydPath cmdCtx, cmdCancel := context.WithTimeout(ctx, 10*time.Second) defer cmdCancel() @@ -52,12 +58,15 @@ func (o *osqueryCheckup) version(ctx context.Context) (string, error) { hideWindow(cmd) startTime := time.Now().UnixMilli() out, err := cmd.CombinedOutput() - o.executionTimes[cmd.String()] = fmt.Sprintf("%d ms", time.Now().UnixMilli()-startTime) + o.data["execution_time_osq_version"] = fmt.Sprintf("%d ms", time.Now().UnixMilli()-startTime) if err != nil { return "", fmt.Errorf("running %s version: err %w, output %s", osquerydPath, err, string(out)) } - return strings.TrimSpace(string(out)), nil + osqVersion := strings.TrimSpace(string(out)) + o.data["osqueryd_version"] = osqVersion + + return osqVersion, nil } func (o *osqueryCheckup) interactive(ctx context.Context) error { @@ -79,7 +88,7 @@ func (o *osqueryCheckup) interactive(ctx context.Context) error { startTime := time.Now().UnixMilli() out, err := cmd.CombinedOutput() - o.executionTimes[cmd.String()] = fmt.Sprintf("%d ms", time.Now().UnixMilli()-startTime) + o.data["execution_time_launcher_interactive"] = fmt.Sprintf("%d ms", time.Now().UnixMilli()-startTime) if err != nil { return fmt.Errorf("running %s interactive: err %w, output %s", launcherPath, err, string(out)) } @@ -87,6 +96,30 @@ func (o *osqueryCheckup) interactive(ctx context.Context) error { return nil } +func (o *osqueryCheckup) instanceHistory(_ context.Context) { + mostRecentInstances, err := history.GetHistory() + if err != nil { + o.data["osquery_instance_history"] = fmt.Errorf("could not get instance history: %+v", err) + return + } + + mostRecentInstancesFormatted := make([]map[string]string, len(mostRecentInstances)) + for i, instance := range mostRecentInstances { + mostRecentInstancesFormatted[i] = map[string]string{ + "start_time": instance.StartTime, + "connect_time": instance.ConnectTime, + "exit_time": instance.ExitTime, + "hostname": instance.Hostname, + "instance_id": instance.InstanceId, + "version": instance.Version, + "error": instance.Error, + } + + } + + o.data["osquery_instance_history"] = mostRecentInstancesFormatted +} + func (o *osqueryCheckup) ExtraFileName() string { return "" } @@ -100,5 +133,5 @@ func (o *osqueryCheckup) Summary() string { } func (o *osqueryCheckup) Data() any { - return o.executionTimes + return o.data } From 2b0bf4ee869c1406a6af202f06a8bba8d7a4ee9e Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 12:30:46 -0500 Subject: [PATCH 06/12] Add querier to knapsack --- cmd/launcher/launcher.go | 5 +-- ee/localserver/mocks/querier.go | 53 ------------------------ ee/localserver/request-id.go | 6 +-- ee/localserver/request-query.go | 6 +-- ee/localserver/request-query_test.go | 16 ++----- ee/localserver/server.go | 5 --- ee/tuf/autoupdate.go | 10 +---- ee/tuf/autoupdate_test.go | 62 ++++++++++++++-------------- ee/tuf/mock_querier_test.go | 51 ----------------------- pkg/agent/knapsack/knapsack.go | 22 +++++++++- pkg/agent/types/knapsack.go | 2 + pkg/agent/types/mocks/knapsack.go | 37 ++++++++++++++--- pkg/traces/exporter/exporter.go | 7 +--- pkg/traces/exporter/exporter_test.go | 36 ++++------------ pkg/traces/exporter/mocks/querier.go | 51 ----------------------- 15 files changed, 106 insertions(+), 263 deletions(-) delete mode 100644 ee/localserver/mocks/querier.go delete mode 100644 ee/tuf/mock_querier_test.go delete mode 100644 pkg/traces/exporter/mocks/querier.go diff --git a/cmd/launcher/launcher.go b/cmd/launcher/launcher.go index 48534c496..21fcf42b4 100644 --- a/cmd/launcher/launcher.go +++ b/cmd/launcher/launcher.go @@ -275,6 +275,7 @@ func runLauncher(ctx context.Context, cancel func(), slogger, systemSlogger *mul return fmt.Errorf("create extension with runtime: %w", err) } runGroup.Add("osqueryExtension", extension.Execute, extension.Interrupt) + k.SetQuerier(extension) versionInfo := version.Version() k.SystemSlogger().Info("started kolide launcher", @@ -346,7 +347,7 @@ func runLauncher(ctx context.Context, cancel func(), slogger, systemSlogger *mul return fmt.Errorf("failed to register auth token consumer: %w", err) } - if exp, err := exporter.NewTraceExporter(ctx, k, extension, logger); err != nil { + if exp, err := exporter.NewTraceExporter(ctx, k, logger); err != nil { level.Debug(logger).Log( "msg", "could not set up trace exporter", "err", err, @@ -391,7 +392,6 @@ func runLauncher(ctx context.Context, cancel func(), slogger, systemSlogger *mul level.Error(logger).Log("msg", "Failed to setup localserver", "error", err) } - ls.SetQuerier(extension) runGroup.Add("localserver", ls.Start, ls.Interrupt) } @@ -406,7 +406,6 @@ func runLauncher(ctx context.Context, cancel func(), slogger, systemSlogger *mul k, metadataClient, mirrorClient, - extension, tuf.WithLogger(logger), tuf.WithOsqueryRestart(runnerRestart), ) diff --git a/ee/localserver/mocks/querier.go b/ee/localserver/mocks/querier.go deleted file mode 100644 index 17bc7d802..000000000 --- a/ee/localserver/mocks/querier.go +++ /dev/null @@ -1,53 +0,0 @@ -// Code generated by mockery v2.20.0. DO NOT EDIT. -// Generated via: mockery --name Querier from local server folder -// https://github.com/vektra/mockery - -package mocks - -import mock "github.com/stretchr/testify/mock" - -// Querier is an autogenerated mock type for the Querier type -type Querier struct { - mock.Mock -} - -// Query provides a mock function with given fields: query -func (_m *Querier) Query(query string) ([]map[string]string, error) { - ret := _m.Called(query) - - var r0 []map[string]string - var r1 error - if rf, ok := ret.Get(0).(func(string) ([]map[string]string, error)); ok { - return rf(query) - } - if rf, ok := ret.Get(0).(func(string) []map[string]string); ok { - r0 = rf(query) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]map[string]string) - } - } - - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(query) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -type mockConstructorTestingTNewQuerier interface { - mock.TestingT - Cleanup(func()) -} - -// NewQuerier creates a new instance of Querier. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewQuerier(t mockConstructorTestingTNewQuerier) *Querier { - mock := &Querier{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/ee/localserver/request-id.go b/ee/localserver/request-id.go index c5ce8b292..a21f26a68 100644 --- a/ee/localserver/request-id.go +++ b/ee/localserver/request-id.go @@ -36,11 +36,7 @@ const ( ) func (ls *localServer) updateIdFields() error { - if ls.querier == nil { - return errors.New("no querier set") - } - - results, err := ls.querier.Query(idSQL) + results, err := ls.knapsack.Query(idSQL) if err != nil { return fmt.Errorf("id query failed: %w", err) } diff --git a/ee/localserver/request-query.go b/ee/localserver/request-query.go index 29932bf01..56bc894b6 100644 --- a/ee/localserver/request-query.go +++ b/ee/localserver/request-query.go @@ -39,7 +39,7 @@ func (ls *localServer) requestQueryHanlderFunc(w http.ResponseWriter, r *http.Re return } - results, err := queryWithRetries(ls.querier, query) + results, err := queryWithRetries(ls.knapsack, query) if err != nil { sendClientError(w, span, fmt.Errorf("error executing query: %s", err)) return @@ -92,7 +92,7 @@ func (ls *localServer) requestScheduledQueryHandlerFunc(w http.ResponseWriter, r scheduledQueryQuery := fmt.Sprintf("select name, query from osquery_schedule where name like '%s'", name) - scheduledQueriesQueryResults, err := queryWithRetries(ls.querier, scheduledQueryQuery) + scheduledQueriesQueryResults, err := queryWithRetries(ls.knapsack, scheduledQueryQuery) if err != nil { sendClientError(w, span, fmt.Errorf("error executing query for scheduled queries using \"%s\": %s", scheduledQueryQuery, err)) return @@ -105,7 +105,7 @@ func (ls *localServer) requestScheduledQueryHandlerFunc(w http.ResponseWriter, r QueryName: scheduledQuery["name"], } - scheduledQueryResult, err := queryWithRetries(ls.querier, scheduledQuery["query"]) + scheduledQueryResult, err := queryWithRetries(ls.knapsack, scheduledQuery["query"]) if err != nil { ls.slogger.Log(r.Context(), slog.LevelError, "running scheduled query on demand", diff --git a/ee/localserver/request-query_test.go b/ee/localserver/request-query_test.go index cc92084c1..3201e4f23 100644 --- a/ee/localserver/request-query_test.go +++ b/ee/localserver/request-query_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/go-kit/kit/log" - "github.com/kolide/launcher/ee/localserver/mocks" "github.com/kolide/launcher/pkg/agent/storage" storageci "github.com/kolide/launcher/pkg/agent/storage/ci" typesMocks "github.com/kolide/launcher/pkg/agent/types/mocks" @@ -60,16 +59,11 @@ func Test_localServer_requestQueryHandler(t *testing.T) { mockKnapsack.On("KolideServerURL").Return("localhost") mockKnapsack.On("Slogger").Return(multislogger.New().Logger) - //go:generate mockery --name Querier - // https://github.com/vektra/mockery <-- cli tool to generate mocks for usage with testify - mockQuerier := mocks.NewQuerier(t) - if tt.mockQueryResult != nil { - mockQuerier.On("Query", tt.query).Return(tt.mockQueryResult, nil).Once() + mockKnapsack.On("Query", tt.query).Return(tt.mockQueryResult, nil).Once() } server := testServer(t, mockKnapsack) - server.querier = mockQuerier jsonBytes, err := json.Marshal(map[string]string{ "query": tt.query, @@ -224,22 +218,18 @@ func Test_localServer_requestRunScheduledQueryHandler(t *testing.T) { mockKnapsack.On("ConfigStore").Return(storageci.NewStore(t, log.NewNopLogger(), storage.ConfigStore.String())) mockKnapsack.On("KolideServerURL").Return("localhost") mockKnapsack.On("Slogger").Return(multislogger.New().Logger) - - // set up mock querier - mockQuerier := mocks.NewQuerier(t) scheduledQueryQuery := fmt.Sprintf("select name, query from osquery_schedule where name like '%s'", tt.scheduledQueriesQueryNamePattern) // the query for the scheduled queries - mockQuerier.On("Query", scheduledQueryQuery).Return(tt.scheduledQueriesQueryResults, tt.scheduledQueriesQueryError) + mockKnapsack.On("Query", scheduledQueryQuery).Return(tt.scheduledQueriesQueryResults, tt.scheduledQueriesQueryError) // the results of each scheduled query for i, queryResult := range tt.queryReturns { - mockQuerier.On("Query", tt.scheduledQueriesQueryResults[i]["query"]).Return(queryResult.results, queryResult.err) + mockKnapsack.On("Query", tt.scheduledQueriesQueryResults[i]["query"]).Return(queryResult.results, queryResult.err) } // set up test server server := testServer(t, mockKnapsack) - server.querier = mockQuerier // make request body body := make(map[string]string) diff --git a/ee/localserver/server.go b/ee/localserver/server.go index f51b12c07..164415de9 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -45,7 +45,6 @@ type localServer struct { identifiers identifiers limiter *rate.Limiter tlsCerts []tls.Certificate - querier Querier kolideServer string myKey *rsa.PrivateKey @@ -131,10 +130,6 @@ func New(k types.Knapsack) (*localServer, error) { return ls, nil } -func (ls *localServer) SetQuerier(querier Querier) { - ls.querier = querier -} - func (ls *localServer) LoadDefaultKeyIfNotSet() error { if ls.serverKey != nil { return nil diff --git a/ee/tuf/autoupdate.go b/ee/tuf/autoupdate.go index 9bf34159f..6cde2717e 100644 --- a/ee/tuf/autoupdate.go +++ b/ee/tuf/autoupdate.go @@ -55,14 +55,9 @@ type librarian interface { TidyLibrary(binary autoupdatableBinary, currentVersion string) } -type querier interface { - Query(query string) ([]map[string]string, error) -} - type TufAutoupdater struct { metadataClient *client.Client libraryManager librarian - osquerier querier // used to query for current running osquery version osquerierRetryInterval time.Duration knapsack types.Knapsack store types.KVStore // stores autoupdater errors for kolide_tuf_autoupdater_errors table @@ -91,13 +86,12 @@ func WithOsqueryRestart(restart func() error) TufAutoupdaterOption { } func NewTufAutoupdater(k types.Knapsack, metadataHttpClient *http.Client, mirrorHttpClient *http.Client, - osquerier querier, opts ...TufAutoupdaterOption) (*TufAutoupdater, error) { + opts ...TufAutoupdaterOption) (*TufAutoupdater, error) { ta := &TufAutoupdater{ knapsack: k, interrupt: make(chan struct{}, 1), signalRestart: make(chan error, 1), store: k.AutoupdateErrorsStore(), - osquerier: osquerier, osquerierRetryInterval: 30 * time.Second, logger: log.NewNopLogger(), restartFuncs: make(map[autoupdatableBinary]func() error), @@ -257,7 +251,7 @@ func (ta *TufAutoupdater) currentRunningVersion(binary autoupdatableBinary) (str var err error for i := 0; i < osquerydVersionCheckRetries; i += 1 { var resp []map[string]string - resp, err = ta.osquerier.Query("SELECT version FROM osquery_info;") + resp, err = ta.knapsack.Query("SELECT version FROM osquery_info;") if err == nil && len(resp) > 0 { if osquerydVersion, ok := resp[0]["version"]; ok { return osquerydVersion, nil diff --git a/ee/tuf/autoupdate_test.go b/ee/tuf/autoupdate_test.go index 207754feb..9b616c962 100644 --- a/ee/tuf/autoupdate_test.go +++ b/ee/tuf/autoupdate_test.go @@ -37,7 +37,7 @@ func TestNewTufAutoupdater(t *testing.T) { mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - _, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, newMockQuerier(t)) + _, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient) require.NoError(t, err, "could not initialize new TUF autoupdater") // Confirm we pulled all config items as expected @@ -74,10 +74,9 @@ func TestExecute_launcherUpdate(t *testing.T) { mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") mockKnapsack.On("LocalDevelopmentPath").Return("") - mockQuerier := newMockQuerier(t) // Set up autoupdater - autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, mockQuerier) + autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient) require.NoError(t, err, "could not initialize new TUF autoupdater") // Update the metadata client with our test root JSON @@ -100,7 +99,7 @@ func TestExecute_launcherUpdate(t *testing.T) { autoupdater.libraryManager = mockLibraryManager currentLauncherVersion := "" // cannot determine using version package in test currentOsqueryVersion := "1.1.1" - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once() // Expect that we attempt to update the library @@ -163,10 +162,9 @@ func TestExecute_launcherUpdate_noRestartIfUsingLegacyAutoupdater(t *testing.T) mockKnapsack.On("TufServerURL").Return(tufServerUrl) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) // Set up autoupdater - autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, mockQuerier) + autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient) require.NoError(t, err, "could not initialize new TUF autoupdater") // Update the metadata client with our test root JSON @@ -189,7 +187,7 @@ func TestExecute_launcherUpdate_noRestartIfUsingLegacyAutoupdater(t *testing.T) autoupdater.libraryManager = mockLibraryManager currentLauncherVersion := "" // cannot determine using version package in test currentOsqueryVersion := "1.1.1" - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once() // Expect that we attempt to update the library @@ -235,10 +233,9 @@ func TestExecute_osquerydUpdate(t *testing.T) { mockKnapsack.On("TufServerURL").Return(tufServerUrl) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) // Set up autoupdater - autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, mockQuerier, WithOsqueryRestart(func() error { return nil })) + autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, WithOsqueryRestart(func() error { return nil })) require.NoError(t, err, "could not initialize new TUF autoupdater") // Update the metadata client with our test root JSON @@ -258,7 +255,7 @@ func TestExecute_osquerydUpdate(t *testing.T) { mockLibraryManager := NewMocklibrarian(t) autoupdater.libraryManager = mockLibraryManager currentOsqueryVersion := "1.1.1" - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once() // Expect that we attempt to update the library @@ -308,10 +305,9 @@ func TestExecute_downgrade(t *testing.T) { mockKnapsack.On("TufServerURL").Return(tufServerUrl) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) // Set up autoupdater - autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, mockQuerier, WithOsqueryRestart(func() error { return nil })) + autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, WithOsqueryRestart(func() error { return nil })) require.NoError(t, err, "could not initialize new TUF autoupdater") // Update the metadata client with our test root JSON @@ -329,7 +325,7 @@ func TestExecute_downgrade(t *testing.T) { mockLibraryManager := NewMocklibrarian(t) autoupdater.libraryManager = mockLibraryManager currentOsqueryVersion := "4.0.0" - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": currentOsqueryVersion}}, nil) mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once() // Expect that we do not attempt to update the library (i.e. the osquery update was previously downloaded) @@ -392,11 +388,10 @@ func TestExecute_withInitialDelay(t *testing.T) { mockKnapsack.On("TufServerURL").Return(tufServerUrl) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) // Set up autoupdater autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, - mockQuerier, WithOsqueryRestart(func() error { return nil })) + WithOsqueryRestart(func() error { return nil })) require.NoError(t, err, "could not initialize new TUF autoupdater") // Set logger so that we can capture output @@ -455,11 +450,10 @@ func TestInterrupt_Multiple(t *testing.T) { mockKnapsack.On("TufServerURL").Return(testMetadataServer.URL) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) // Set up autoupdater autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, - mockQuerier, WithOsqueryRestart(func() error { return nil })) + WithOsqueryRestart(func() error { return nil })) require.NoError(t, err, "could not initialize new TUF autoupdater") // Set logger so that we can capture output @@ -470,7 +464,7 @@ func TestInterrupt_Multiple(t *testing.T) { mockLibraryManager := NewMocklibrarian(t) autoupdater.libraryManager = mockLibraryManager mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once() - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": "1.1.1"}}, nil) + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": "1.1.1"}}, nil) // Let the autoupdater run for a bit go autoupdater.Execute() @@ -512,10 +506,10 @@ func TestInterrupt_Multiple(t *testing.T) { func Test_currentRunningVersion_launcher_errorWhenVersionIsNotSet(t *testing.T) { t.Parallel() - mockQuerier := newMockQuerier(t) + mockKnapsack := typesmocks.NewKnapsack(t) autoupdater := &TufAutoupdater{ - logger: log.NewNopLogger(), - osquerier: mockQuerier, + logger: log.NewNopLogger(), + knapsack: mockKnapsack, } // In test, version.Version() returns `unknown` for everything, which is not something @@ -523,43 +517,49 @@ func Test_currentRunningVersion_launcher_errorWhenVersionIsNotSet(t *testing.T) launcherVersion, err := autoupdater.currentRunningVersion("launcher") require.Error(t, err, "expected an error fetching current running version of launcher") require.Equal(t, "", launcherVersion) + + mockKnapsack.AssertExpectations(t) } func Test_currentRunningVersion_osqueryd(t *testing.T) { t.Parallel() - mockQuerier := newMockQuerier(t) + mockKnapsack := typesmocks.NewKnapsack(t) autoupdater := &TufAutoupdater{ - logger: log.NewNopLogger(), - osquerier: mockQuerier, + logger: log.NewNopLogger(), + knapsack: mockKnapsack, } // Expect to return one row containing the version expectedOsqueryVersion, err := semver.NewVersion("5.10.12") require.NoError(t, err) - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": expectedOsqueryVersion.Original()}}, nil).Once() + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": expectedOsqueryVersion.Original()}}, nil).Once() osqueryVersion, err := autoupdater.currentRunningVersion("osqueryd") require.NoError(t, err, "expected no error fetching current running version of osqueryd") require.Equal(t, expectedOsqueryVersion.Original(), osqueryVersion) + + mockKnapsack.AssertExpectations(t) } func Test_currentRunningVersion_osqueryd_handlesQueryError(t *testing.T) { t.Parallel() - mockQuerier := newMockQuerier(t) + mockKnapsack := typesmocks.NewKnapsack(t) autoupdater := &TufAutoupdater{ logger: log.NewNopLogger(), - osquerier: mockQuerier, + knapsack: mockKnapsack, osquerierRetryInterval: 1 * time.Millisecond, } // Expect to return an error (five times, since we perform retries) - mockQuerier.On("Query", mock.Anything).Return(make([]map[string]string, 0), errors.New("test osqueryd querying error")) + mockKnapsack.On("Query", mock.Anything).Return(make([]map[string]string, 0), errors.New("test osqueryd querying error")) osqueryVersion, err := autoupdater.currentRunningVersion("osqueryd") require.Error(t, err, "expected an error returning osquery version when querying osquery fails") require.Equal(t, "", osqueryVersion) + + mockKnapsack.AssertExpectations(t) } func Test_storeError(t *testing.T) { @@ -579,14 +579,13 @@ func Test_storeError(t *testing.T) { mockKnapsack.On("TufServerURL").Return(testTufServer.URL) mockKnapsack.On("UpdateDirectory").Return("") mockKnapsack.On("MirrorServerURL").Return("https://example.com") - mockQuerier := newMockQuerier(t) - autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient, mockQuerier) + autoupdater, err := NewTufAutoupdater(mockKnapsack, http.DefaultClient, http.DefaultClient) require.NoError(t, err, "could not initialize new TUF autoupdater") mockLibraryManager := NewMocklibrarian(t) autoupdater.libraryManager = mockLibraryManager - mockQuerier.On("Query", mock.Anything).Return([]map[string]string{{"version": "1.1.1"}}, nil).Once() + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{{"version": "1.1.1"}}, nil).Once() // We only expect TidyLibrary to run for osqueryd, since we can't get the current running version // for launcher in tests. @@ -616,7 +615,6 @@ func Test_storeError(t *testing.T) { require.Greater(t, errorCount, 0, "TUF autoupdater did not record error counts") mockLibraryManager.AssertExpectations(t) - mockQuerier.AssertExpectations(t) mockKnapsack.AssertExpectations(t) } diff --git a/ee/tuf/mock_querier_test.go b/ee/tuf/mock_querier_test.go deleted file mode 100644 index d316dd7ee..000000000 --- a/ee/tuf/mock_querier_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// Code generated by mockery v2.21.1. DO NOT EDIT. - -package tuf - -import mock "github.com/stretchr/testify/mock" - -// mockQuerier is an autogenerated mock type for the querier type -type mockQuerier struct { - mock.Mock -} - -// Query provides a mock function with given fields: query -func (_m *mockQuerier) Query(query string) ([]map[string]string, error) { - ret := _m.Called(query) - - var r0 []map[string]string - var r1 error - if rf, ok := ret.Get(0).(func(string) ([]map[string]string, error)); ok { - return rf(query) - } - if rf, ok := ret.Get(0).(func(string) []map[string]string); ok { - r0 = rf(query) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]map[string]string) - } - } - - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(query) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -type mockConstructorTestingTnewMockQuerier interface { - mock.TestingT - Cleanup(func()) -} - -// newMockQuerier creates a new instance of mockQuerier. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func newMockQuerier(t mockConstructorTestingTnewMockQuerier) *mockQuerier { - mock := &mockQuerier{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/pkg/agent/knapsack/knapsack.go b/pkg/agent/knapsack/knapsack.go index 9d448c3af..73d99093a 100644 --- a/pkg/agent/knapsack/knapsack.go +++ b/pkg/agent/knapsack/knapsack.go @@ -2,6 +2,7 @@ package knapsack import ( "context" + "errors" "time" "log/slog" @@ -33,9 +34,13 @@ type knapsack struct { slogger, systemSlogger *multislogger.MultiSlogger launcherRunId string + q querier + // This struct is a work in progress, and will be iteratively added to as needs arise. - // Some potential future additions include: - // Querier +} + +type querier interface { + Query(query string) ([]map[string]string, error) } func New(stores map[storage.Store]types.KVStore, flags types.Flags, db *bbolt.DB, slogger, systemSlogger *multislogger.MultiSlogger) *knapsack { @@ -68,6 +73,19 @@ func (k *knapsack) AddSlogHandler(handler ...slog.Handler) { k.systemSlogger.AddHandler(handler...) } +// Querier interface methods +func (k *knapsack) SetQuerier(q querier) { + k.q = q +} + +func (k *knapsack) Query(query string) ([]map[string]string, error) { + if k.q == nil { + return nil, errors.New("querier not set in knapsack") + } + + return k.q.Query(query) +} + // BboltDB interface methods func (k *knapsack) BboltDB() *bbolt.DB { return k.db diff --git a/pkg/agent/types/knapsack.go b/pkg/agent/types/knapsack.go index 47e93146e..ec671bcfc 100644 --- a/pkg/agent/types/knapsack.go +++ b/pkg/agent/types/knapsack.go @@ -11,4 +11,6 @@ type Knapsack interface { Slogger // LatestOsquerydPath finds the path to the latest osqueryd binary, after accounting for updates. LatestOsquerydPath(ctx context.Context) string + // Query allows for querying via the running osquery client + Query(query string) ([]map[string]string, error) } diff --git a/pkg/agent/types/mocks/knapsack.go b/pkg/agent/types/mocks/knapsack.go index 855b95adf..8daeab5ac 100644 --- a/pkg/agent/types/mocks/knapsack.go +++ b/pkg/agent/types/mocks/knapsack.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.33.3. DO NOT EDIT. +// Code generated by mockery v2.21.1. DO NOT EDIT. package mocks @@ -782,6 +782,32 @@ func (_m *Knapsack) OsquerydPath() string { return r0 } +// Query provides a mock function with given fields: query +func (_m *Knapsack) Query(query string) ([]map[string]string, error) { + ret := _m.Called(query) + + var r0 []map[string]string + var r1 error + if rf, ok := ret.Get(0).(func(string) ([]map[string]string, error)); ok { + return rf(query) + } + if rf, ok := ret.Get(0).(func(string) []map[string]string); ok { + r0 = rf(query) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]map[string]string) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(query) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // RegisterChangeObserver provides a mock function with given fields: observer, flagKeys func (_m *Knapsack) RegisterChangeObserver(observer types.FlagsChangeObserver, flagKeys ...keys.FlagKey) { _va := make([]interface{}, len(flagKeys)) @@ -1504,12 +1530,13 @@ func (_m *Knapsack) UpdateDirectory() string { return r0 } -// NewKnapsack creates a new instance of Knapsack. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewKnapsack(t interface { +type mockConstructorTestingTNewKnapsack interface { mock.TestingT Cleanup(func()) -}) *Knapsack { +} + +// NewKnapsack creates a new instance of Knapsack. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewKnapsack(t mockConstructorTestingTNewKnapsack) *Knapsack { mock := &Knapsack{} mock.Mock.Test(t) diff --git a/pkg/traces/exporter/exporter.go b/pkg/traces/exporter/exporter.go index 1548a6236..d3a04d4f0 100644 --- a/pkg/traces/exporter/exporter.go +++ b/pkg/traces/exporter/exporter.go @@ -13,7 +13,6 @@ import ( "github.com/kolide/launcher/pkg/agent/flags/keys" "github.com/kolide/launcher/pkg/agent/storage" "github.com/kolide/launcher/pkg/agent/types" - "github.com/kolide/launcher/pkg/osquery" osquerygotraces "github.com/osquery/osquery-go/traces" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -45,7 +44,6 @@ type TraceExporter struct { provider *sdktrace.TracerProvider providerLock sync.Mutex knapsack types.Knapsack - osqueryClient querier logger log.Logger attrs []attribute.KeyValue // resource attributes, identifying this device + installation attrLock sync.RWMutex @@ -63,7 +61,7 @@ type TraceExporter struct { // NewTraceExporter sets up our traces to be exported via OTLP over HTTP. // On interrupt, the provider will be shut down. -func NewTraceExporter(ctx context.Context, k types.Knapsack, client osquery.Querier, logger log.Logger) (*TraceExporter, error) { +func NewTraceExporter(ctx context.Context, k types.Knapsack, logger log.Logger) (*TraceExporter, error) { // Set all the attributes that we know we can get first attrs := []attribute.KeyValue{ semconv.ServiceName(applicationName), @@ -81,7 +79,6 @@ func NewTraceExporter(ctx context.Context, k types.Knapsack, client osquery.Quer t := &TraceExporter{ providerLock: sync.Mutex{}, knapsack: k, - osqueryClient: client, logger: log.With(logger, "component", "trace_exporter"), attrs: attrs, attrLock: sync.RWMutex{}, @@ -186,7 +183,7 @@ func (t *TraceExporter) addAttributesFromOsquery() { break } - resp, err = t.osqueryClient.Query(osqueryInfoQuery) + resp, err = t.knapsack.Query(osqueryInfoQuery) if err == nil && len(resp) > 0 { break } diff --git a/pkg/traces/exporter/exporter_test.go b/pkg/traces/exporter/exporter_test.go index 4ec5d1041..d3dadeaf1 100644 --- a/pkg/traces/exporter/exporter_test.go +++ b/pkg/traces/exporter/exporter_test.go @@ -10,7 +10,6 @@ import ( "github.com/go-kit/kit/log" "github.com/kolide/kit/version" - "github.com/kolide/launcher/ee/localserver/mocks" "github.com/kolide/launcher/pkg/agent/flags/keys" "github.com/kolide/launcher/pkg/agent/storage" storageci "github.com/kolide/launcher/pkg/agent/storage/ci" @@ -44,9 +43,7 @@ func TestNewTraceExporter(t *testing.T) { //nolint:paralleltest mockKnapsack.On("TraceSamplingRate").Return(1.0) mockKnapsack.On("TraceBatchTimeout").Return(1 * time.Minute) mockKnapsack.On("RegisterChangeObserver", mock.Anything, keys.ExportTraces, keys.TraceSamplingRate, keys.TraceIngestServerURL, keys.DisableTraceIngestTLS, keys.TraceBatchTimeout).Return(nil) - - osqueryClient := mocks.NewQuerier(t) - osqueryClient.On("Query", mock.Anything).Return([]map[string]string{ + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{ { "osquery_version": "5.8.0", "os_name": runtime.GOOS, @@ -55,7 +52,7 @@ func TestNewTraceExporter(t *testing.T) { //nolint:paralleltest }, }, nil) - traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, osqueryClient, log.NewNopLogger()) + traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, log.NewNopLogger()) require.NoError(t, err) // Wait a few seconds to allow the osquery queries to go through @@ -72,7 +69,6 @@ func TestNewTraceExporter(t *testing.T) { //nolint:paralleltest require.NotNil(t, traceExporter.provider, "expected provider to be created") mockKnapsack.AssertExpectations(t) - osqueryClient.AssertExpectations(t) } func TestNewTraceExporter_exportNotEnabled(t *testing.T) { @@ -89,7 +85,7 @@ func TestNewTraceExporter_exportNotEnabled(t *testing.T) { mockKnapsack.On("TraceBatchTimeout").Return(1 * time.Minute) mockKnapsack.On("RegisterChangeObserver", mock.Anything, keys.ExportTraces, keys.TraceSamplingRate, keys.TraceIngestServerURL, keys.DisableTraceIngestTLS, keys.TraceBatchTimeout).Return(nil) - traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, mocks.NewQuerier(t), log.NewNopLogger()) + traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, log.NewNopLogger()) require.NoError(t, err) // Confirm we didn't set a provider @@ -127,7 +123,7 @@ func TestInterrupt_Multiple(t *testing.T) { mockKnapsack.On("TraceBatchTimeout").Return(1 * time.Minute) mockKnapsack.On("RegisterChangeObserver", mock.Anything, keys.ExportTraces, keys.TraceSamplingRate, keys.TraceIngestServerURL, keys.DisableTraceIngestTLS, keys.TraceBatchTimeout).Return(nil) - traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, mocks.NewQuerier(t), log.NewNopLogger()) + traceExporter, err := NewTraceExporter(context.Background(), mockKnapsack, log.NewNopLogger()) require.NoError(t, err) mockKnapsack.AssertExpectations(t) @@ -186,7 +182,6 @@ func Test_addDeviceIdentifyingAttributes(t *testing.T) { traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: mocks.NewQuerier(t), logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -228,8 +223,8 @@ func Test_addAttributesFromOsquery(t *testing.T) { expectedOsVersion := "1.2.3" expectedHostname := "Test-Hostname" - osqueryClient := mocks.NewQuerier(t) - osqueryClient.On("Query", mock.Anything).Return([]map[string]string{ + k := typesmocks.NewKnapsack(t) + k.On("Query", mock.Anything).Return([]map[string]string{ { "osquery_version": expectedOsqueryVersion, "os_name": expectedOsName, @@ -239,8 +234,7 @@ func Test_addAttributesFromOsquery(t *testing.T) { }, nil) traceExporter := &TraceExporter{ - knapsack: typesmocks.NewKnapsack(t), - osqueryClient: osqueryClient, + knapsack: k, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -271,7 +265,7 @@ func Test_addAttributesFromOsquery(t *testing.T) { } } - osqueryClient.AssertExpectations(t) + k.AssertExpectations(t) } func TestPing(t *testing.T) { @@ -287,7 +281,6 @@ func TestPing(t *testing.T) { traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: mocks.NewQuerier(t), logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -354,11 +347,10 @@ func TestFlagsChanged_ExportTraces(t *testing.T) { //nolint:paralleltest s := testServerProvidedDataStore(t) mockKnapsack := typesmocks.NewKnapsack(t) mockKnapsack.On("ExportTraces").Return(tt.newEnableValue) - osqueryClient := mocks.NewQuerier(t) if tt.shouldReplaceProvider { mockKnapsack.On("ServerProvidedDataStore").Return(s) - osqueryClient.On("Query", mock.Anything).Return([]map[string]string{ + mockKnapsack.On("Query", mock.Anything).Return([]map[string]string{ { "osquery_version": "5.9.0", "os_name": "Windows", @@ -371,7 +363,6 @@ func TestFlagsChanged_ExportTraces(t *testing.T) { //nolint:paralleltest ctx, cancel := context.WithCancel(context.Background()) traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: osqueryClient, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -391,7 +382,6 @@ func TestFlagsChanged_ExportTraces(t *testing.T) { //nolint:paralleltest if tt.shouldReplaceProvider { mockKnapsack.AssertExpectations(t) - osqueryClient.AssertExpectations(t) require.Greater(t, len(traceExporter.attrs), 0) require.NotNil(t, traceExporter.provider) } @@ -435,12 +425,10 @@ func TestFlagsChanged_TraceSamplingRate(t *testing.T) { //nolint:paralleltest t.Run(tt.testName, func(t *testing.T) { mockKnapsack := typesmocks.NewKnapsack(t) mockKnapsack.On("TraceSamplingRate").Return(tt.newTraceSamplingRate) - osqueryClient := mocks.NewQuerier(t) ctx, cancel := context.WithCancel(context.Background()) traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: osqueryClient, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -503,12 +491,10 @@ func TestFlagsChanged_TraceIngestServerURL(t *testing.T) { //nolint:paralleltest t.Run(tt.testName, func(t *testing.T) { mockKnapsack := typesmocks.NewKnapsack(t) mockKnapsack.On("TraceIngestServerURL").Return(tt.newObservabilityIngestServerURL) - osqueryClient := mocks.NewQuerier(t) ctx, cancel := context.WithCancel(context.Background()) traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: osqueryClient, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -571,14 +557,12 @@ func TestFlagsChanged_DisableTraceIngestTLS(t *testing.T) { //nolint:paralleltes t.Run(tt.testName, func(t *testing.T) { mockKnapsack := typesmocks.NewKnapsack(t) mockKnapsack.On("DisableTraceIngestTLS").Return(tt.newDisableTraceIngestTLS) - osqueryClient := mocks.NewQuerier(t) clientAuthenticator := newClientAuthenticator("test token", tt.currentDisableTraceIngestTLS) ctx, cancel := context.WithCancel(context.Background()) traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: osqueryClient, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, @@ -642,12 +626,10 @@ func TestFlagsChanged_TraceBatchTimeout(t *testing.T) { //nolint:paralleltest t.Run(tt.testName, func(t *testing.T) { mockKnapsack := typesmocks.NewKnapsack(t) mockKnapsack.On("TraceBatchTimeout").Return(tt.newBatchTimeout) - osqueryClient := mocks.NewQuerier(t) ctx, cancel := context.WithCancel(context.Background()) traceExporter := &TraceExporter{ knapsack: mockKnapsack, - osqueryClient: osqueryClient, logger: log.NewNopLogger(), attrs: make([]attribute.KeyValue, 0), attrLock: sync.RWMutex{}, diff --git a/pkg/traces/exporter/mocks/querier.go b/pkg/traces/exporter/mocks/querier.go deleted file mode 100644 index 670aa2b4e..000000000 --- a/pkg/traces/exporter/mocks/querier.go +++ /dev/null @@ -1,51 +0,0 @@ -// Code generated by mockery v2.21.1. DO NOT EDIT. - -package mocks - -import mock "github.com/stretchr/testify/mock" - -// Querier is an autogenerated mock type for the querier type -type Querier struct { - mock.Mock -} - -// Query provides a mock function with given fields: query -func (_m *Querier) Query(query string) ([]map[string]string, error) { - ret := _m.Called(query) - - var r0 []map[string]string - var r1 error - if rf, ok := ret.Get(0).(func(string) ([]map[string]string, error)); ok { - return rf(query) - } - if rf, ok := ret.Get(0).(func(string) []map[string]string); ok { - r0 = rf(query) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]map[string]string) - } - } - - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(query) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -type mockConstructorTestingTNewQuerier interface { - mock.TestingT - Cleanup(func()) -} - -// NewQuerier creates a new instance of Querier. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewQuerier(t mockConstructorTestingTNewQuerier) *Querier { - mock := &Querier{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} From d1d263e341ac9b39ed20fc321933585556400b99 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 13:09:05 -0500 Subject: [PATCH 07/12] Add QuerierHealthy to knapsack interface --- cmd/launcher/extension.go | 10 ++++++++-- pkg/agent/knapsack/knapsack.go | 9 +++++++++ pkg/agent/types/knapsack.go | 3 +-- pkg/agent/types/mocks/knapsack.go | 14 ++++++++++++++ pkg/agent/types/querier.go | 6 ++++++ 5 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 pkg/agent/types/querier.go diff --git a/cmd/launcher/extension.go b/cmd/launcher/extension.go index 5f2bdea99..f9ab19994 100644 --- a/cmd/launcher/extension.go +++ b/cmd/launcher/extension.go @@ -29,13 +29,18 @@ import ( // going to be pretty extensive work. type actorQuerier struct { actor.Actor - querier func(query string) ([]map[string]string, error) + querier func(query string) ([]map[string]string, error) + healthchecker func() error } func (aq actorQuerier) Query(query string) ([]map[string]string, error) { return aq.querier(query) } +func (aq actorQuerier) Healthy() error { + return aq.healthchecker() +} + // TODO: the extension, runtime, and client are all kind of entangled // here. Untangle the underlying libraries and separate into units func createExtensionRuntime(ctx context.Context, k types.Knapsack, launcherClient service.KolideService) ( @@ -169,7 +174,8 @@ func createExtensionRuntime(ctx context.Context, k types.Knapsack, launcherClien } }, }, - querier: runner.Query, + querier: runner.Query, + healthchecker: runner.Healthy, }, restartFunc, runner.Shutdown, diff --git a/pkg/agent/knapsack/knapsack.go b/pkg/agent/knapsack/knapsack.go index 73d99093a..dcf75b139 100644 --- a/pkg/agent/knapsack/knapsack.go +++ b/pkg/agent/knapsack/knapsack.go @@ -41,6 +41,7 @@ type knapsack struct { type querier interface { Query(query string) ([]map[string]string, error) + Healthy() error } func New(stores map[storage.Store]types.KVStore, flags types.Flags, db *bbolt.DB, slogger, systemSlogger *multislogger.MultiSlogger) *knapsack { @@ -86,6 +87,14 @@ func (k *knapsack) Query(query string) ([]map[string]string, error) { return k.q.Query(query) } +func (k *knapsack) QuerierHealthy() error { + if k.q == nil { + return errors.New("querier not set in knapsack") + } + + return k.q.Healthy() +} + // BboltDB interface methods func (k *knapsack) BboltDB() *bbolt.DB { return k.db diff --git a/pkg/agent/types/knapsack.go b/pkg/agent/types/knapsack.go index ec671bcfc..cd059a071 100644 --- a/pkg/agent/types/knapsack.go +++ b/pkg/agent/types/knapsack.go @@ -9,8 +9,7 @@ type Knapsack interface { BboltDB Flags Slogger + Querier // LatestOsquerydPath finds the path to the latest osqueryd binary, after accounting for updates. LatestOsquerydPath(ctx context.Context) string - // Query allows for querying via the running osquery client - Query(query string) ([]map[string]string, error) } diff --git a/pkg/agent/types/mocks/knapsack.go b/pkg/agent/types/mocks/knapsack.go index 8daeab5ac..4b23df93d 100644 --- a/pkg/agent/types/mocks/knapsack.go +++ b/pkg/agent/types/mocks/knapsack.go @@ -782,6 +782,20 @@ func (_m *Knapsack) OsquerydPath() string { return r0 } +// QuerierHealthy provides a mock function with given fields: +func (_m *Knapsack) QuerierHealthy() error { + ret := _m.Called() + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Query provides a mock function with given fields: query func (_m *Knapsack) Query(query string) ([]map[string]string, error) { ret := _m.Called(query) diff --git a/pkg/agent/types/querier.go b/pkg/agent/types/querier.go new file mode 100644 index 000000000..60c2d593f --- /dev/null +++ b/pkg/agent/types/querier.go @@ -0,0 +1,6 @@ +package types + +type Querier interface { + Query(query string) ([]map[string]string, error) + QuerierHealthy() error +} From a3214d08a3c4cd6142d34d0babaec48ca32a14c9 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 13:30:30 -0500 Subject: [PATCH 08/12] Add osquery healthcheck to osquery checkup --- pkg/debug/checkups/osquery.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/debug/checkups/osquery.go b/pkg/debug/checkups/osquery.go index b27d1d910..aef8a1310 100644 --- a/pkg/debug/checkups/osquery.go +++ b/pkg/debug/checkups/osquery.go @@ -42,7 +42,10 @@ func (o *osqueryCheckup) Run(ctx context.Context, extraWriter io.Writer) error { } // Retrieve osquery instance history to see if we have an abnormal number of restarts - o.instanceHistory(ctx) + o.instanceHistory() + + // Check to see if current extension is healthy + o.extensionHealth() return nil } @@ -96,7 +99,7 @@ func (o *osqueryCheckup) interactive(ctx context.Context) error { return nil } -func (o *osqueryCheckup) instanceHistory(_ context.Context) { +func (o *osqueryCheckup) instanceHistory() { mostRecentInstances, err := history.GetHistory() if err != nil { o.data["osquery_instance_history"] = fmt.Errorf("could not get instance history: %+v", err) @@ -120,6 +123,15 @@ func (o *osqueryCheckup) instanceHistory(_ context.Context) { o.data["osquery_instance_history"] = mostRecentInstancesFormatted } +func (o *osqueryCheckup) extensionHealth() { + err := o.k.QuerierHealthy() + if err != nil { + o.data["osquery_instance_healthcheck_err"] = err.Error() + } else { + o.data["osquery_instance_healthcheck_err"] = nil + } +} + func (o *osqueryCheckup) ExtraFileName() string { return "" } From 867fdd2f2b52efab65bc69e2ce14900d50ca1bef Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 13:39:31 -0500 Subject: [PATCH 09/12] No need for launcher interactive to test launcher <=> osq communication anymore --- pkg/debug/checkups/osquery.go | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/pkg/debug/checkups/osquery.go b/pkg/debug/checkups/osquery.go index aef8a1310..27b97f3c0 100644 --- a/pkg/debug/checkups/osquery.go +++ b/pkg/debug/checkups/osquery.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os/exec" - "runtime" "strings" "time" @@ -36,11 +35,6 @@ func (o *osqueryCheckup) Run(ctx context.Context, extraWriter io.Writer) error { o.summary = osqueryVersion } - // Run launcher interactive to capture timing details - if err := o.interactive(ctx); err != nil { - return fmt.Errorf("running launcher interactive: %w", err) - } - // Retrieve osquery instance history to see if we have an abnormal number of restarts o.instanceHistory() @@ -72,33 +66,6 @@ func (o *osqueryCheckup) version(ctx context.Context) (string, error) { return osqVersion, nil } -func (o *osqueryCheckup) interactive(ctx context.Context) error { - var launcherPath string - switch runtime.GOOS { - case "linux", "darwin": - launcherPath = "/usr/local/kolide-k2/bin/launcher" - case "windows": - launcherPath = `C:\Program Files\Kolide\Launcher-kolide-k2\bin\launcher.exe` - } - - cmdCtx, cmdCancel := context.WithTimeout(ctx, 20*time.Second) - defer cmdCancel() - - // We trust the autoupdate library to find the correct path - cmd := exec.CommandContext(cmdCtx, launcherPath, "interactive") //nolint:forbidigo // We trust the autoupdate library to find the correct path - hideWindow(cmd) - cmd.Stdin = strings.NewReader(`select * from osquery_info;`) - - startTime := time.Now().UnixMilli() - out, err := cmd.CombinedOutput() - o.data["execution_time_launcher_interactive"] = fmt.Sprintf("%d ms", time.Now().UnixMilli()-startTime) - if err != nil { - return fmt.Errorf("running %s interactive: err %w, output %s", launcherPath, err, string(out)) - } - - return nil -} - func (o *osqueryCheckup) instanceHistory() { mostRecentInstances, err := history.GetHistory() if err != nil { From 1bc496c33486012b4a40a4296f7792a98df5d9f9 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 13:41:58 -0500 Subject: [PATCH 10/12] Regularly log osquery instance history and health --- pkg/debug/checkups/checkups.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/debug/checkups/checkups.go b/pkg/debug/checkups/checkups.go index b64a2b221..5c73f0f81 100644 --- a/pkg/debug/checkups/checkups.go +++ b/pkg/debug/checkups/checkups.go @@ -108,7 +108,7 @@ func checkupsFor(k types.Knapsack, target targetBits) []checkupInt { {&installCheckup{}, flareSupported}, {&servicesCheckup{}, doctorSupported | flareSupported}, {&powerCheckup{}, flareSupported}, - {&osqueryCheckup{k: k}, doctorSupported | flareSupported}, + {&osqueryCheckup{k: k}, doctorSupported | flareSupported | logSupported}, {&launcherFlags{}, doctorSupported | flareSupported}, {&gnomeExtensions{}, doctorSupported | flareSupported}, {&quarantine{}, doctorSupported | flareSupported}, From 67ce5cd8621c835fb00743b332d2cd276b4f37d2 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 13:51:52 -0500 Subject: [PATCH 11/12] Remove unused interface --- pkg/traces/exporter/exporter.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/traces/exporter/exporter.go b/pkg/traces/exporter/exporter.go index d3a04d4f0..9d5699de1 100644 --- a/pkg/traces/exporter/exporter.go +++ b/pkg/traces/exporter/exporter.go @@ -36,10 +36,6 @@ var archAttributeMap = map[string]attribute.KeyValue{ var osqueryClientRecheckInterval = 30 * time.Second -type querier interface { - Query(query string) ([]map[string]string, error) -} - type TraceExporter struct { provider *sdktrace.TracerProvider providerLock sync.Mutex From 32c7fcfe91b4ad4411c77ace73b137068266666f Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Tue, 28 Nov 2023 14:07:15 -0500 Subject: [PATCH 12/12] Fix method mock --- pkg/debug/checkups/checkpoint_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/debug/checkups/checkpoint_test.go b/pkg/debug/checkups/checkpoint_test.go index 428c5d24a..daccc3c23 100644 --- a/pkg/debug/checkups/checkpoint_test.go +++ b/pkg/debug/checkups/checkpoint_test.go @@ -8,6 +8,7 @@ import ( "github.com/go-kit/kit/log" storageci "github.com/kolide/launcher/pkg/agent/storage/ci" typesmocks "github.com/kolide/launcher/pkg/agent/types/mocks" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -28,7 +29,7 @@ func TestInterrupt_Multiple(t *testing.T) { mockKnapsack.On("RootDirectory").Return("").Maybe() mockKnapsack.On("Autoupdate").Return(true).Maybe() mockKnapsack.On("NotaryServerURL").Return("localhost").Maybe() - mockKnapsack.On("LatestOsquerydPath").Return("").Maybe() + mockKnapsack.On("LatestOsquerydPath", mock.Anything).Return("").Maybe() mockKnapsack.On("ServerProvidedDataStore").Return(nil).Maybe() checkupLogger := NewCheckupLogger(log.NewNopLogger(), mockKnapsack) mockKnapsack.AssertExpectations(t)