diff --git a/pkg/debug/checkups/bboltdb.go b/pkg/debug/checkups/bboltdb.go index 30fdfad88..f3907f085 100644 --- a/pkg/debug/checkups/bboltdb.go +++ b/pkg/debug/checkups/bboltdb.go @@ -50,6 +50,6 @@ func (c *bboltdbCheckup) Summary() string { return "N/A" } -func (c *bboltdbCheckup) Data() map[string]any { +func (c *bboltdbCheckup) Data() any { return c.data } diff --git a/pkg/debug/checkups/binary_directory.go b/pkg/debug/checkups/binary_directory.go index 68b3bfe89..923e85a27 100644 --- a/pkg/debug/checkups/binary_directory.go +++ b/pkg/debug/checkups/binary_directory.go @@ -57,7 +57,7 @@ func (c *BinaryDirectory) Summary() string { return c.summary } -func (c *BinaryDirectory) Data() map[string]any { +func (c *BinaryDirectory) Data() any { return nil } diff --git a/pkg/debug/checkups/checkpoint.go b/pkg/debug/checkups/checkpoint.go index e6426e6f0..ec1a353aa 100644 --- a/pkg/debug/checkups/checkpoint.go +++ b/pkg/debug/checkups/checkpoint.go @@ -2,9 +2,7 @@ package checkups import ( "context" - "fmt" "io" - "strings" "time" "github.com/go-kit/kit/log" @@ -18,7 +16,7 @@ type ( Log(keyvals ...interface{}) error } - checkPointer struct { + logCheckPointer struct { logger logger knapsack types.Knapsack interrupt chan struct{} @@ -26,8 +24,8 @@ type ( } ) -func NewCheckupLogger(logger logger, k types.Knapsack) *checkPointer { - return &checkPointer{ +func NewCheckupLogger(logger logger, k types.Knapsack) *logCheckPointer { + return &logCheckPointer{ logger: log.With(logger, "component", "log checkpoint"), knapsack: k, interrupt: make(chan struct{}, 1), @@ -36,7 +34,7 @@ func NewCheckupLogger(logger logger, k types.Knapsack) *checkPointer { // Run starts a log checkpoint routine. The purpose of this is to // ensure we get good debugging information in the logs. -func (c *checkPointer) Run() error { +func (c *logCheckPointer) Run() error { ticker := time.NewTicker(time.Minute * 60) defer ticker.Stop() @@ -53,7 +51,7 @@ func (c *checkPointer) Run() error { } } -func (c *checkPointer) Interrupt(_ error) { +func (c *logCheckPointer) Interrupt(_ error) { // Only perform shutdown tasks on first call to interrupt -- no need to repeat on potential extra calls. if c.interrupted { return @@ -64,28 +62,17 @@ func (c *checkPointer) Interrupt(_ error) { c.interrupt <- struct{}{} } -func (c *checkPointer) Once(ctx context.Context) { +func (c *logCheckPointer) Once(ctx context.Context) { checkups := checkupsFor(c.knapsack, logSupported) for _, checkup := range checkups { checkup.Run(ctx, io.Discard) - logValues := []interface{}{"checkup", checkup.Name()} - for k, v := range checkup.Data() { - logValues = append(logValues, k, c.summarizeData(v)) - } - - c.logger.Log(logValues...) - } -} - -func (c *checkPointer) summarizeData(data any) any { - switch knownValue := data.(type) { - case []string: - return strings.Join(knownValue, ",") - case string, uint, uint64, int, int32, int64: - return knownValue - default: - return fmt.Sprintf("%v", data) + level.Debug(c.logger).Log( + "checkup", checkup.Name(), + "summary", checkup.Summary(), + "data", checkup.Data(), + "status", checkup.Status(), + ) } } diff --git a/pkg/debug/checkups/checkpoint_test.go b/pkg/debug/checkups/checkpoint_test.go index a7a043580..428c5d24a 100644 --- a/pkg/debug/checkups/checkpoint_test.go +++ b/pkg/debug/checkups/checkpoint_test.go @@ -29,6 +29,7 @@ func TestInterrupt_Multiple(t *testing.T) { mockKnapsack.On("Autoupdate").Return(true).Maybe() mockKnapsack.On("NotaryServerURL").Return("localhost").Maybe() mockKnapsack.On("LatestOsquerydPath").Return("").Maybe() + mockKnapsack.On("ServerProvidedDataStore").Return(nil).Maybe() checkupLogger := NewCheckupLogger(log.NewNopLogger(), mockKnapsack) mockKnapsack.AssertExpectations(t) diff --git a/pkg/debug/checkups/checkups.go b/pkg/debug/checkups/checkups.go index 7cd1c29b9..b64a2b221 100644 --- a/pkg/debug/checkups/checkups.go +++ b/pkg/debug/checkups/checkups.go @@ -72,7 +72,7 @@ type checkupInt interface { ExtraFileName() string // If this checkup will have extra data, what name should it use in flare Summary() string // Short summary string about the status Status() Status // State of this checkup - Data() map[string]any // What data objects exist, if any + Data() any // What data objects exist, if any } type targetBits uint8 diff --git a/pkg/debug/checkups/connectivity.go b/pkg/debug/checkups/connectivity.go index 6c94ce8ad..465cbec51 100644 --- a/pkg/debug/checkups/connectivity.go +++ b/pkg/debug/checkups/connectivity.go @@ -86,7 +86,7 @@ func (c *Connectivity) Summary() string { return c.summary } -func (c *Connectivity) Data() map[string]any { +func (c *Connectivity) Data() any { return c.data } diff --git a/pkg/debug/checkups/dns.go b/pkg/debug/checkups/dns.go index c4891722b..cbb985b3a 100644 --- a/pkg/debug/checkups/dns.go +++ b/pkg/debug/checkups/dns.go @@ -10,20 +10,30 @@ import ( "github.com/kolide/launcher/pkg/agent/types" ) -type dnsCheckup struct { - k types.Knapsack - status Status - summary string - data map[string]any -} +type ( + HostResolver interface { + LookupHost(ctx context.Context, host string) ([]string, error) + } + dnsCheckup struct { + k types.Knapsack + status Status + summary string + data map[string]any + resolver HostResolver + } +) -func (nc *dnsCheckup) Data() map[string]any { return nc.data } -func (nc *dnsCheckup) ExtraFileName() string { return "" } -func (nc *dnsCheckup) Name() string { return "DNS Resolution" } -func (nc *dnsCheckup) Status() Status { return nc.status } -func (nc *dnsCheckup) Summary() string { return nc.summary } +func (dc *dnsCheckup) Data() any { return dc.data } +func (dc *dnsCheckup) ExtraFileName() string { return "" } +func (dc *dnsCheckup) Name() string { return "DNS Resolution" } +func (dc *dnsCheckup) Status() Status { return dc.status } +func (dc *dnsCheckup) Summary() string { return dc.summary } func (dc *dnsCheckup) Run(ctx context.Context, extraFH io.Writer) error { + if dc.resolver == nil { + dc.resolver = &net.Resolver{} + } + hosts := []string{ dc.k.KolideServerURL(), dc.k.ControlServerURL(), @@ -34,7 +44,6 @@ func (dc *dnsCheckup) Run(ctx context.Context, extraFH io.Writer) error { dc.data = make(map[string]any) attemptedCount, successCount := 0, 0 - resolver := &net.Resolver{} for _, host := range hosts { if len(strings.TrimSpace(host)) == 0 { @@ -47,7 +56,7 @@ func (dc *dnsCheckup) Run(ctx context.Context, extraFH io.Writer) error { continue } - ips, err := resolveHost(resolver, hostUrl.Hostname()) + ips, err := dc.resolveHost(ctx, hostUrl.Hostname()) // keep attemptedCount as a separate variable to avoid indicating failures where we didn't even try attemptedCount++ @@ -76,11 +85,11 @@ func (dc *dnsCheckup) Run(ctx context.Context, extraFH io.Writer) error { return nil } -func resolveHost(resolver *net.Resolver, host string) (string, error) { - ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) +func (dc *dnsCheckup) resolveHost(ctx context.Context, host string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, requestTimeout) defer cancel() - ips, err := resolver.LookupHost(ctx, host) + ips, err := dc.resolver.LookupHost(ctx, host) if err != nil { return "", err diff --git a/pkg/debug/checkups/dns_test.go b/pkg/debug/checkups/dns_test.go new file mode 100644 index 000000000..186237f9b --- /dev/null +++ b/pkg/debug/checkups/dns_test.go @@ -0,0 +1,154 @@ +package checkups + +import ( + "context" + "fmt" + "io" + "strings" + "testing" + + typesMocks "github.com/kolide/launcher/pkg/agent/types/mocks" + "github.com/kolide/launcher/pkg/debug/checkups/mocks" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func Test_dnsCheckup_Run(t *testing.T) { + t.Parallel() + type fields struct { + k *typesMocks.Knapsack + status Status + summary string + data map[string]any + resolver *mocks.HostResolver + } + type resolution struct { + ips []string + err error + } + type args struct { + ctx context.Context + } + tests := []struct { + name string + fields fields + args args + knapsackReturns map[string]any + onLookupHostReturns map[string]resolution + expectedStatus Status + expectedSuccessCount int + expectedAttemptsCount int + }{ + { + name: "happy path", + fields: fields{ + k: typesMocks.NewKnapsack(t), + resolver: mocks.NewHostResolver(t), + }, + args: args{context.Background()}, + knapsackReturns: map[string]interface{}{ + "KolideServerURL": "https://kolide-server.example.com", + "ControlServerURL": "https://control-server.example.com", + "TufServerURL": "https://tuf-server.example.com", + "InsecureTransportTLS": false, + }, + onLookupHostReturns: map[string]resolution{ + "kolide-server.example.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil}, + "control-server.example.com": resolution{ips: []string{"111.2.3.4", "111.2.3.5"}, err: nil}, + "tuf-server.example.com": resolution{ips: []string{"34.149.84.181"}, err: nil}, + "google.com": resolution{ips: []string{"2620:149:af0::10", "17.253.144.10"}, err: nil}, + "apple.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil}, + }, + expectedStatus: Passing, + expectedSuccessCount: 5, + expectedAttemptsCount: 5, + }, + { + name: "unresolvable host errors are included in data", + fields: fields{ + k: typesMocks.NewKnapsack(t), + resolver: mocks.NewHostResolver(t), + }, + args: args{context.Background()}, + knapsackReturns: map[string]interface{}{ + "KolideServerURL": "https://kolide-server.example.com", + "ControlServerURL": "https://control-server.example.com", + "TufServerURL": "https://tuf-server.example.com", + "InsecureTransportTLS": false, + }, + onLookupHostReturns: map[string]resolution{ + "kolide-server.example.com": resolution{ips: []string{}, err: fmt.Errorf("Unable to resolve: No Such Host")}, + "control-server.example.com": resolution{ips: []string{"111.2.3.4", "111.2.3.5"}, err: nil}, + "tuf-server.example.com": resolution{ips: []string{"34.149.84.181"}, err: nil}, + "google.com": resolution{ips: []string{"2620:149:af0::10", "17.253.144.10"}, err: nil}, + "apple.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil}, + }, + expectedStatus: Warning, + expectedSuccessCount: 4, + expectedAttemptsCount: 5, + }, + { + name: "unset host values are not counted against checkup", + fields: fields{ + k: typesMocks.NewKnapsack(t), + resolver: mocks.NewHostResolver(t), + }, + args: args{context.Background()}, + knapsackReturns: map[string]interface{}{ + "KolideServerURL": "https://kolide-server.example.com", + "ControlServerURL": "https://control-server.example.com", + "TufServerURL": "", + "InsecureTransportTLS": false, + }, + onLookupHostReturns: map[string]resolution{ + "kolide-server.example.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil}, + "control-server.example.com": resolution{ips: []string{"111.2.3.4", "111.2.3.5"}, err: nil}, + "google.com": resolution{ips: []string{"2620:149:af0::10", "17.253.144.10"}, err: nil}, + "apple.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil}, + }, + expectedStatus: Passing, + expectedSuccessCount: 4, + expectedAttemptsCount: 4, + }, + } + + for _, tt := range tests { + tt := tt + for mockfunc, mockval := range tt.knapsackReturns { + tt.fields.k.On(mockfunc).Return(mockval) + } + + for mockhost, mockresolution := range tt.onLookupHostReturns { + tt.fields.resolver.On("LookupHost", mock.Anything, mockhost).Return(mockresolution.ips, mockresolution.err).Once() + } + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dc := &dnsCheckup{ + k: tt.fields.k, + status: tt.fields.status, + summary: tt.fields.summary, + data: tt.fields.data, + resolver: tt.fields.resolver, + } + if err := dc.Run(tt.args.ctx, io.Discard); err != nil { + t.Errorf("dnsCheckup.Run() error = %v", err) + return + } + + gotData, ok := dc.Data().(map[string]any) + require.True(t, ok, "expected to be able to typecast data into map[string]any for testing") + + for mockhost, mockresolution := range tt.onLookupHostReturns { + if mockresolution.err != nil { + require.Contains(t, gotData[mockhost], mockresolution.err.Error(), "expected data for %s to contain the resolution error message", mockhost) + } else { + require.Contains(t, gotData[mockhost], strings.Join(mockresolution.ips, ","), "expected data to contain the resulting IP addresses") + } + } + + require.Equal(t, tt.expectedAttemptsCount, gotData["lookup_attempts"], "expected lookup attempts to match") + require.Equal(t, tt.expectedSuccessCount, gotData["lookup_successes"], "expected lookup success count to match") + }) + } +} diff --git a/pkg/debug/checkups/enroll-secret.go b/pkg/debug/checkups/enroll-secret.go index 6de35662b..88a87d5ca 100644 --- a/pkg/debug/checkups/enroll-secret.go +++ b/pkg/debug/checkups/enroll-secret.go @@ -70,7 +70,7 @@ func (c *enrollSecretCheckup) Summary() string { return c.summary } -func (c *enrollSecretCheckup) Data() map[string]any { +func (c *enrollSecretCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/gnome-extensions.go b/pkg/debug/checkups/gnome-extensions.go index 67c37df41..17b9a9d11 100644 --- a/pkg/debug/checkups/gnome-extensions.go +++ b/pkg/debug/checkups/gnome-extensions.go @@ -82,7 +82,7 @@ func (c *gnomeExtensions) Summary() string { return c.summary } -func (c *gnomeExtensions) Data() map[string]any { +func (c *gnomeExtensions) Data() any { return nil } diff --git a/pkg/debug/checkups/host.go b/pkg/debug/checkups/host.go index baf6d300b..1ea3dc90e 100644 --- a/pkg/debug/checkups/host.go +++ b/pkg/debug/checkups/host.go @@ -26,7 +26,7 @@ type ( } ) -func (hc *hostInfoCheckup) Data() map[string]any { return hc.data } +func (hc *hostInfoCheckup) Data() any { return hc.data } func (hc *hostInfoCheckup) ExtraFileName() string { return "" } func (hc *hostInfoCheckup) Name() string { return "Host Info" } func (hc *hostInfoCheckup) Status() Status { return hc.status } @@ -45,10 +45,9 @@ func (hc *hostInfoCheckup) Run(ctx context.Context, extraFH io.Writer) error { hc.data["uptime_friendly"] = fmt.Sprintf("ERROR: %s", err.Error()) } else { hc.data["uptime_friendly"] = formatUptime(uptimeRaw) + hc.data["uptime"] = uptimeRaw } - hc.data["uptime"] = uptimeRaw - if runtime.GOOS == "windows" { hc.data["in_modern_standby"] = hc.k.InModernStandby() } diff --git a/pkg/debug/checkups/host_test.go b/pkg/debug/checkups/host_test.go index 6b18d1bb7..861e33b40 100644 --- a/pkg/debug/checkups/host_test.go +++ b/pkg/debug/checkups/host_test.go @@ -6,22 +6,22 @@ func Test_formatUptime(t *testing.T) { t.Parallel() tests := []struct { - name string - uptime uint64 - want string + name string + uptime uint64 + expected string }{ - {name: "1 day", uptime: 86400, want: "1 day"}, - {name: "just over 1 day", uptime: 86401, want: "1 day, 1 second"}, - {name: "less than a day", uptime: 82860, want: "23 hours, 1 minute"}, - {name: "just booted", uptime: 0, want: "0 seconds"}, - {name: "you should reboot", uptime: 34559999, want: "399 days, 23 hours, 59 minutes, 59 seconds"}, + {name: "1 day", uptime: 86400, expected: "1 day"}, + {name: "just over 1 day", uptime: 86401, expected: "1 day, 1 second"}, + {name: "less than a day", uptime: 82860, expected: "23 hours, 1 minute"}, + {name: "just booted", uptime: 0, expected: "0 seconds"}, + {name: "you should reboot", uptime: 34559999, expected: "399 days, 23 hours, 59 minutes, 59 seconds"}, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := formatUptime(tt.uptime); got != tt.want { - t.Errorf("formatUptime() = %v, want %v", got, tt.want) + if actual := formatUptime(tt.uptime); actual != tt.expected { + t.Errorf("formatUptime() = %v, want %v", actual, tt.expected) } }) } diff --git a/pkg/debug/checkups/install.go b/pkg/debug/checkups/install.go index b99c60f35..ba9bdb7ee 100644 --- a/pkg/debug/checkups/install.go +++ b/pkg/debug/checkups/install.go @@ -40,7 +40,7 @@ func (i *installCheckup) Summary() string { return "N/A" } -func (i *installCheckup) Data() map[string]any { +func (i *installCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/launchd.go b/pkg/debug/checkups/launchd.go index c58f6e3b8..4cd6c1b08 100644 --- a/pkg/debug/checkups/launchd.go +++ b/pkg/debug/checkups/launchd.go @@ -110,6 +110,6 @@ func (c *launchdCheckup) Summary() string { return c.summary } -func (c *launchdCheckup) Data() map[string]any { +func (c *launchdCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/launcher_flags.go b/pkg/debug/checkups/launcher_flags.go index 664d9d9f6..55fa77a7b 100644 --- a/pkg/debug/checkups/launcher_flags.go +++ b/pkg/debug/checkups/launcher_flags.go @@ -69,7 +69,7 @@ func (lf *launcherFlags) ExtraFileName() string { return "launcherFlags.log" } -func (lf *launcherFlags) Data() map[string]any { +func (lf *launcherFlags) Data() any { return nil } diff --git a/pkg/debug/checkups/logs.go b/pkg/debug/checkups/logs.go index e15c8fae7..f94d8be3d 100644 --- a/pkg/debug/checkups/logs.go +++ b/pkg/debug/checkups/logs.go @@ -77,6 +77,6 @@ func (c *Logs) ExtraFileName() string { return "logs.zip" } -func (c *Logs) Data() map[string]any { +func (c *Logs) Data() any { return nil } diff --git a/pkg/debug/checkups/mocks/HostResolver.go b/pkg/debug/checkups/mocks/HostResolver.go new file mode 100644 index 000000000..b4f02093f --- /dev/null +++ b/pkg/debug/checkups/mocks/HostResolver.go @@ -0,0 +1,54 @@ +// Code generated by mockery v2.34.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// HostResolver is an autogenerated mock type for the HostResolver type +type HostResolver struct { + mock.Mock +} + +// LookupHost provides a mock function with given fields: ctx, host +func (_m *HostResolver) LookupHost(ctx context.Context, host string) ([]string, error) { + ret := _m.Called(ctx, host) + + var r0 []string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]string, error)); ok { + return rf(ctx, host) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []string); ok { + r0 = rf(ctx, host) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, host) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewHostResolver creates a new instance of HostResolver. 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 NewHostResolver(t interface { + mock.TestingT + Cleanup(func()) +}) *HostResolver { + mock := &HostResolver{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/debug/checkups/network.go b/pkg/debug/checkups/network.go index 19f387b72..a0c546c1e 100644 --- a/pkg/debug/checkups/network.go +++ b/pkg/debug/checkups/network.go @@ -78,7 +78,7 @@ func (n *networkCheckup) Summary() string { return n.summary } -func (n *networkCheckup) Data() map[string]any { +func (n *networkCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/notary.go b/pkg/debug/checkups/notary.go index 5eecca4bf..5c34b9d32 100644 --- a/pkg/debug/checkups/notary.go +++ b/pkg/debug/checkups/notary.go @@ -27,7 +27,7 @@ type ( } ) -func (nc *notaryCheckup) Data() map[string]any { return nc.data } +func (nc *notaryCheckup) Data() any { return nc.data } func (nc *notaryCheckup) ExtraFileName() string { return "" } func (nc *notaryCheckup) Name() string { return "Notary Version" } func (nc *notaryCheckup) Status() Status { return nc.status } diff --git a/pkg/debug/checkups/osq_config_conflicts.go b/pkg/debug/checkups/osq_config_conflicts.go index a344c575b..494d40129 100644 --- a/pkg/debug/checkups/osq_config_conflicts.go +++ b/pkg/debug/checkups/osq_config_conflicts.go @@ -20,7 +20,7 @@ type osqConfigConflictCheckup struct { data map[string]any } -func (occ *osqConfigConflictCheckup) Data() map[string]any { return occ.data } +func (occ *osqConfigConflictCheckup) Data() any { return occ.data } func (occ *osqConfigConflictCheckup) ExtraFileName() string { return "" } func (occ *osqConfigConflictCheckup) Name() string { return "Osquery Conflicts" } func (occ *osqConfigConflictCheckup) Status() Status { return occ.status } diff --git a/pkg/debug/checkups/osq_data.go b/pkg/debug/checkups/osq_data.go index 2abe78a32..23d13eea5 100644 --- a/pkg/debug/checkups/osq_data.go +++ b/pkg/debug/checkups/osq_data.go @@ -36,7 +36,7 @@ type ( } ) -func (odc *osqDataCollector) Data() map[string]any { return odc.data } +func (odc *osqDataCollector) Data() any { return odc.data } func (odc *osqDataCollector) ExtraFileName() string { return "" } func (odc *osqDataCollector) Name() string { return "Osquery Data" } func (odc *osqDataCollector) Status() Status { return odc.status } @@ -45,7 +45,7 @@ func (odc *osqDataCollector) Summary() string { return odc.summary } func (odc *osqDataCollector) Run(ctx context.Context, extraFH io.Writer) error { odc.data = make(map[string]any) - result, err := odc.osqueryInteractive(ctx, osSqlQuery) + result, err := odc.queryData(ctx, osSqlQuery) if err != nil { odc.status = Erroring odc.data["error"] = err.Error() @@ -67,10 +67,10 @@ func (odc *osqDataCollector) Run(ctx context.Context, extraFH io.Writer) error { return nil } -// osqueryInteractive execs osquery and parses the output to gather some basic host info. +// queryData sets up a runsimple osq process and parses the output to gather some basic host info. // it was done this way to avoid bringing Querier into knapsack for a task that will only be run // during flare or doctor -func (odc *osqDataCollector) osqueryInteractive(ctx context.Context, query string) (map[string]string, error) { +func (odc *osqDataCollector) queryData(ctx context.Context, query string) (map[string]string, error) { osqPath := odc.k.LatestOsquerydPath(ctx) var resultBuffer bytes.Buffer osqCtx, cmdCancel := context.WithTimeout(ctx, 5*time.Second) diff --git a/pkg/debug/checkups/osquery.go b/pkg/debug/checkups/osquery.go index 7e251fcc5..20a49330b 100644 --- a/pkg/debug/checkups/osquery.go +++ b/pkg/debug/checkups/osquery.go @@ -98,6 +98,6 @@ func (o *osqueryCheckup) Summary() string { return o.summary } -func (o *osqueryCheckup) Data() map[string]any { +func (o *osqueryCheckup) Data() any { return o.executionTimes } diff --git a/pkg/debug/checkups/platform.go b/pkg/debug/checkups/platform.go index 6a8fbaf36..af2851dac 100644 --- a/pkg/debug/checkups/platform.go +++ b/pkg/debug/checkups/platform.go @@ -52,7 +52,7 @@ func (c *Platform) Summary() string { return fmt.Sprintf("platform: %s, architecture: %s", runtime.GOOS, runtime.GOARCH) } -func (c *Platform) Data() map[string]any { +func (c *Platform) Data() any { return map[string]any{ "platform": runtime.GOOS, "architecture": runtime.GOARCH, diff --git a/pkg/debug/checkups/power_other.go b/pkg/debug/checkups/power_other.go index a75b21e57..f42972ef5 100644 --- a/pkg/debug/checkups/power_other.go +++ b/pkg/debug/checkups/power_other.go @@ -30,6 +30,6 @@ func (p *powerCheckup) Summary() string { return "" } -func (p *powerCheckup) Data() map[string]any { +func (p *powerCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/power_windows.go b/pkg/debug/checkups/power_windows.go index 12fbea61e..c63ab1ce7 100644 --- a/pkg/debug/checkups/power_windows.go +++ b/pkg/debug/checkups/power_windows.go @@ -56,6 +56,6 @@ func (p *powerCheckup) Summary() string { return "" } -func (p *powerCheckup) Data() map[string]any { +func (p *powerCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/processes.go b/pkg/debug/checkups/processes.go index 460489647..df5ec94cc 100644 --- a/pkg/debug/checkups/processes.go +++ b/pkg/debug/checkups/processes.go @@ -90,7 +90,7 @@ func (c *Processes) Summary() string { return fmt.Sprintf("found %d kolide processes, none running as root or system", c.kolideCount) } -func (c *Processes) Data() map[string]any { +func (c *Processes) Data() any { return c.data } diff --git a/pkg/debug/checkups/quarantine.go b/pkg/debug/checkups/quarantine.go index e26958cf6..c8150fe69 100644 --- a/pkg/debug/checkups/quarantine.go +++ b/pkg/debug/checkups/quarantine.go @@ -231,6 +231,6 @@ func (q *quarantine) ExtraFileName() string { return "quarantine.log" } -func (q *quarantine) Data() map[string]any { +func (q *quarantine) Data() any { return nil } diff --git a/pkg/debug/checkups/root_directory.go b/pkg/debug/checkups/root_directory.go index 04f5253ed..feedef5d1 100644 --- a/pkg/debug/checkups/root_directory.go +++ b/pkg/debug/checkups/root_directory.go @@ -53,6 +53,6 @@ func (c *RootDirectory) Summary() string { return c.summary } -func (c *RootDirectory) Data() map[string]any { +func (c *RootDirectory) Data() any { return nil } diff --git a/pkg/debug/checkups/runtime.go b/pkg/debug/checkups/runtime.go index 2b928f212..e998ce707 100644 --- a/pkg/debug/checkups/runtime.go +++ b/pkg/debug/checkups/runtime.go @@ -54,7 +54,7 @@ func (c *runtimeCheckup) Summary() string { return "N/A" } -func (c *runtimeCheckup) Data() map[string]any { +func (c *runtimeCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/server_data.go b/pkg/debug/checkups/server_data.go index 4acdaa861..f589176f4 100644 --- a/pkg/debug/checkups/server_data.go +++ b/pkg/debug/checkups/server_data.go @@ -2,13 +2,9 @@ package checkups import ( "context" - "fmt" "io" - "strings" - "github.com/kolide/launcher/pkg/agent/storage" "github.com/kolide/launcher/pkg/agent/types" - "go.etcd.io/bbolt" ) var serverProvidedDataKeys = []string{ @@ -26,57 +22,39 @@ type serverDataCheckup struct { data map[string]any } -func (sdc *serverDataCheckup) Data() map[string]any { return sdc.data } +func (sdc *serverDataCheckup) Data() any { return sdc.data } func (sdc *serverDataCheckup) ExtraFileName() string { return "" } func (sdc *serverDataCheckup) Name() string { return "Server Data" } func (sdc *serverDataCheckup) Status() Status { return sdc.status } func (sdc *serverDataCheckup) Summary() string { return sdc.summary } func (sdc *serverDataCheckup) Run(ctx context.Context, extraFH io.Writer) error { - db := sdc.k.BboltDB() + store := sdc.k.ServerProvidedDataStore() sdc.data = make(map[string]any) - if db == nil { + if store == nil { sdc.status = Warning - sdc.summary = "no bbolt DB connection in knapsack" + sdc.summary = "no server_data store in knapsack" return nil } - if err := db.View(func(tx *bbolt.Tx) error { - b := tx.Bucket([]byte(storage.ServerProvidedDataStore)) - if b == nil { - return fmt.Errorf("unable to access bbolt bucket (%s)", storage.ServerProvidedDataStore) + // set up the default failure states, we will overwrite when we get the required data + sdc.status = Failing + sdc.summary = "unable to collect server data" + for _, key := range serverProvidedDataKeys { + val, err := store.Get([]byte(key)) + if err != nil { + sdc.data[key] = err.Error() + continue } - for _, key := range serverProvidedDataKeys { - val := b.Get([]byte(key)) - if val == nil { - continue - } - - sdc.data[key] = string(val) + if key == "device_id" && string(val) != "" { + sdc.status = Passing + sdc.summary = "successfully collected server data" } - return nil - }); err != nil { - sdc.status = Erroring - sdc.data["error"] = err.Error() - sdc.summarize() - return nil + sdc.data[key] = string(val) } - sdc.status = Passing - sdc.summarize() - return nil } - -func (sdc *serverDataCheckup) summarize() { - summary := make([]string, 0) - - for k, v := range sdc.data { - summary = append(summary, fmt.Sprintf("%s: %s", k, v)) - } - - sdc.summary = fmt.Sprintf("collected server data: [%s]", strings.Join(summary, ", ")) -} diff --git a/pkg/debug/checkups/services_other.go b/pkg/debug/checkups/services_other.go index 28e8ab9c1..6de781e0f 100644 --- a/pkg/debug/checkups/services_other.go +++ b/pkg/debug/checkups/services_other.go @@ -31,6 +31,6 @@ func (s *servicesCheckup) Summary() string { return "" } -func (s *servicesCheckup) Data() map[string]any { +func (s *servicesCheckup) Data() any { return nil } diff --git a/pkg/debug/checkups/services_windows.go b/pkg/debug/checkups/services_windows.go index b2ca20095..ea5b8c7fc 100644 --- a/pkg/debug/checkups/services_windows.go +++ b/pkg/debug/checkups/services_windows.go @@ -286,6 +286,6 @@ func (s *servicesCheckup) Summary() string { return fmt.Sprintf("found Kolide service in state %d (%s)", s.serviceState, s.serviceStateHumanReadable) } -func (s *servicesCheckup) Data() map[string]any { +func (s *servicesCheckup) Data() any { return s.data } diff --git a/pkg/debug/checkups/system_time.go b/pkg/debug/checkups/system_time.go index 7ac7392f3..171b4294a 100644 --- a/pkg/debug/checkups/system_time.go +++ b/pkg/debug/checkups/system_time.go @@ -100,6 +100,6 @@ func (st *systemTime) Summary() string { return st.summary } -func (st *systemTime) Data() map[string]any { +func (st *systemTime) Data() any { return nil } diff --git a/pkg/debug/checkups/tuf.go b/pkg/debug/checkups/tuf.go index a0b95c977..41c144559 100644 --- a/pkg/debug/checkups/tuf.go +++ b/pkg/debug/checkups/tuf.go @@ -38,11 +38,12 @@ type ( } ) -func (tc *tufCheckup) Data() map[string]any { return tc.data } +func (tc *tufCheckup) Data() any { return tc.data } func (tc *tufCheckup) ExtraFileName() string { return "tuf.json" } func (tc *tufCheckup) Name() string { return "TUF" } -func (tc *tufCheckup) Status() Status { return tc.status } -func (tc *tufCheckup) Summary() string { return tc.summary } + +func (tc *tufCheckup) Status() Status { return tc.status } +func (tc *tufCheckup) Summary() string { return tc.summary } func (tc *tufCheckup) Run(ctx context.Context, extraFH io.Writer) error { tc.data = make(map[string]any) diff --git a/pkg/debug/checkups/version.go b/pkg/debug/checkups/version.go index f376467fd..4e6f91f0e 100644 --- a/pkg/debug/checkups/version.go +++ b/pkg/debug/checkups/version.go @@ -33,7 +33,7 @@ func (c *Version) Summary() string { return fmt.Sprintf("launcher_version %s", version.Version().Version) } -func (c *Version) Data() map[string]any { +func (c *Version) Data() any { return map[string]any{ "update_channel": c.k.UpdateChannel(), "tufServer": c.k.TufServerURL(), diff --git a/pkg/log/eventlog/eventlog_windows.go b/pkg/log/eventlog/eventlog_windows.go index dbe5c4a5b..95436d929 100644 --- a/pkg/log/eventlog/eventlog_windows.go +++ b/pkg/log/eventlog/eventlog_windows.go @@ -16,7 +16,7 @@ import ( func New(w *Writer) log.Logger { l := &eventLogger{ w: w, - newLogger: log.NewLogfmtLogger, + newLogger: log.NewJSONLogger, bufPool: sync.Pool{New: func() interface{} { return &loggerBuf{} }}, @@ -33,6 +33,7 @@ type eventLogger struct { func (l *eventLogger) Log(keyvals ...interface{}) error { lb := l.getLoggerBuf() defer l.putLoggerBuf(lb) + if err := lb.logger.Log(keyvals...); err != nil { return err }