From 08cd206de79d191a6f21eba171785f5ca0979a9a Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Thu, 14 Jul 2022 08:04:29 -0300 Subject: [PATCH] only send vuln configs to goroutine (#6650) In https://github.com/fleetdm/fleet/pull/6630 I added a new config for packaging, but when I started the server I got: ``` ~/fleet $ ./build/fleet serve --dev --dev_license level=info ts=2022-07-13T21:36:06.055998Z component=redis mode=standalone fatal error: newproc: function arguments too large for new goroutine runtime stack: runtime.throw({0x103d85259, 0x37}) /Users/roperzh/.gvm/gos/go1.17/src/runtime/panic.go:1198 +0x54 runtime.newproc1(0x104569a30, 0x14000ffda28, 0x7f8, 0x140000001a0, 0x103bc85a4) /Users/roperzh/.gvm/gos/go1.17/src/runtime/proc.go:4299 +0x4d0 runtime.newproc.func1() /Users/roperzh/.gvm/gos/go1.17/src/runtime/proc.go:4255 +0x4c runtime.systemstack() /Users/roperzh/.gvm/gos/go1.17/src/runtime/asm_arm64.s:230 +0x6c goroutine 1 [running]: runtime.systemstack_switch() /Users/roperzh/.gvm/gos/go1.17/src/runtime/asm_arm64.s:187 +0x8 fp=0x14000ffd9c0 sp=0x14000ffd9b0 pc=0x102b60958 runtime.newproc(0x7f8, 0x104569a30) /Users/roperzh/.gvm/gos/go1.17/src/runtime/proc.go:4254 +0x54 fp=0x14000ffda10 sp=0x14000ffd9c0 pc=0x102b38034 main.runCrons({0x1045cd500, 0x14000262990}, {0x104611a38, 0x14000262930}, 0x14000298190, {0x1045a0480, 0x140009c9f20}, {{{0x103d1adc4, 0x3}, {0x103d2b188, ...}, ...}, ...}, ...) /Users/roperzh/fleet/cmd/fleet/serve.go:694 +0x2c4 fp=0x14000ffe260 sp=0x14000ffda10 pc=0x103bc85a4 main.createServeCmd.func1(0x1400027ca00, {0x140000bcb40, 0x0, 0x2}) ``` With my local changes, `serve.go:694` is this line: https://github.com/fleetdm/fleet/blob/7559988000efc3d7408d9914edd5a3f2f1052138/cmd/fleet/serve.go#L685-L686 After passing only a subset of the config the issue was solved. --- cmd/fleet/cron.go | 32 +++++++++++++-------------- cmd/fleet/serve.go | 2 +- cmd/fleet/serve_test.go | 48 +++++++++++++++++------------------------ 3 files changed, 37 insertions(+), 45 deletions(-) diff --git a/cmd/fleet/cron.go b/cmd/fleet/cron.go index 50c9653ab3..932b8d022e 100644 --- a/cmd/fleet/cron.go +++ b/cmd/fleet/cron.go @@ -109,11 +109,11 @@ func cronVulnerabilities( ds fleet.Datastore, logger kitlog.Logger, identifier string, - config config.FleetConfig, + config config.VulnerabilitiesConfig, ) { logger = kitlog.With(logger, "cron", lockKeyVulnerabilities) - if config.Vulnerabilities.CurrentInstanceChecks == "no" || config.Vulnerabilities.CurrentInstanceChecks == "0" { + if config.CurrentInstanceChecks == "no" || config.CurrentInstanceChecks == "0" { level.Info(logger).Log("vulnerability scanning", "host not configured to check for vulnerabilities") return } @@ -126,7 +126,7 @@ func cronVulnerabilities( vulnDisabled := false if appConfig.VulnerabilitySettings.DatabasesPath == "" && - config.Vulnerabilities.DatabasesPath == "" { + config.DatabasesPath == "" { level.Info(logger).Log("vulnerability scanning", "not configured") vulnDisabled = true } @@ -137,10 +137,10 @@ func cronVulnerabilities( vulnPath := appConfig.VulnerabilitySettings.DatabasesPath if vulnPath == "" { - vulnPath = config.Vulnerabilities.DatabasesPath + vulnPath = config.DatabasesPath } - if config.Vulnerabilities.DatabasesPath != "" && config.Vulnerabilities.DatabasesPath != vulnPath { - vulnPath = config.Vulnerabilities.DatabasesPath + if config.DatabasesPath != "" && config.DatabasesPath != vulnPath { + vulnPath = config.DatabasesPath level.Info(logger).Log( "databases_path", "fleet config takes precedence over app config when both are configured", "result", vulnPath) @@ -149,10 +149,10 @@ func cronVulnerabilities( if !vulnDisabled { level.Info(logger).Log("databases-path", vulnPath) } - level.Info(logger).Log("periodicity", config.Vulnerabilities.Periodicity) + level.Info(logger).Log("periodicity", config.Periodicity) if !vulnDisabled { - if config.Vulnerabilities.CurrentInstanceChecks == "auto" { + if config.CurrentInstanceChecks == "auto" { level.Debug(logger).Log("current instance checks", "auto", "trying to create databases-path", vulnPath) err := os.MkdirAll(vulnPath, 0o755) if err != nil { @@ -168,12 +168,12 @@ func cronVulnerabilities( select { case <-ticker.C: level.Debug(logger).Log("waiting", "done") - ticker.Reset(config.Vulnerabilities.Periodicity) + ticker.Reset(config.Periodicity) case <-ctx.Done(): level.Debug(logger).Log("exit", "done with cron.") return } - if config.Vulnerabilities.CurrentInstanceChecks == "auto" { + if config.CurrentInstanceChecks == "auto" { if locked, err := ds.Lock(ctx, lockKeyVulnerabilities, identifier, 1*time.Hour); err != nil { errHandler(ctx, logger, "error acquiring lock", err) continue @@ -227,7 +227,7 @@ func cronVulnerabilities( collectVulns := vulnAutomationEnabled != "" nvdVulns := checkNVDVulnerabilities(ctx, ds, logger, vulnPath, config, collectVulns) ovalVulns := checkOvalVulnerabilities(ctx, ds, logger, vulnPath, config, collectVulns) - recentVulns := filterRecentVulns(ctx, ds, logger, nvdVulns, ovalVulns, config.Vulnerabilities.RecentVulnerabilityMaxAge) + recentVulns := filterRecentVulns(ctx, ds, logger, nvdVulns, ovalVulns, config.RecentVulnerabilityMaxAge) if len(recentVulns) > 0 { switch vulnAutomationEnabled { @@ -327,10 +327,10 @@ func checkOvalVulnerabilities( ds fleet.Datastore, logger kitlog.Logger, vulnPath string, - config config.FleetConfig, + config config.VulnerabilitiesConfig, collectVulns bool, ) []fleet.SoftwareVulnerability { - if config.Vulnerabilities.DisableDataSync { + if config.DisableDataSync { return nil } @@ -377,11 +377,11 @@ func checkNVDVulnerabilities( ds fleet.Datastore, logger kitlog.Logger, vulnPath string, - config config.FleetConfig, + config config.VulnerabilitiesConfig, collectVulns bool, ) []fleet.SoftwareVulnerability { - if !config.Vulnerabilities.DisableDataSync { - err := vulnerabilities.Sync(vulnPath, config.Vulnerabilities.CPEDatabaseURL) + if !config.DisableDataSync { + err := vulnerabilities.Sync(vulnPath, config.CPEDatabaseURL) if err != nil { errHandler(ctx, logger, "syncing vulnerability database", err) return nil diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index 1fe73c01d3..725b4e6bd0 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -683,7 +683,7 @@ func runCrons( go cronDB(ctx, ds, kitlog.With(logger, "cron", "cleanups"), ourIdentifier, license, enrollHostLimiter) go cronVulnerabilities( - ctx, ds, kitlog.With(logger, "cron", "vulnerabilities"), ourIdentifier, config) + ctx, ds, kitlog.With(logger, "cron", "vulnerabilities"), ourIdentifier, config.Vulnerabilities) go cronWebhooks(ctx, ds, kitlog.With(logger, "cron", "webhooks"), ourIdentifier, failingPoliciesSet, 1*time.Hour) go cronWorker(ctx, ds, kitlog.With(logger, "cron", "worker"), ourIdentifier) } diff --git a/cmd/fleet/serve_test.go b/cmd/fleet/serve_test.go index 6399b85926..18ca08d4a7 100644 --- a/cmd/fleet/serve_test.go +++ b/cmd/fleet/serve_test.go @@ -202,17 +202,15 @@ func TestCronVulnerabilitiesCreatesDatabasesPath(t *testing.T) { vulnPath := path.Join(t.TempDir(), "something") require.NoDirExists(t, vulnPath) - fleetConfig := config.FleetConfig{ - Vulnerabilities: config.VulnerabilitiesConfig{ - DatabasesPath: vulnPath, - Periodicity: 10 * time.Second, - CurrentInstanceChecks: "auto", - }, + config := config.VulnerabilitiesConfig{ + DatabasesPath: vulnPath, + Periodicity: 10 * time.Second, + CurrentInstanceChecks: "auto", } // We cancel right away so cronsVulnerailities finishes. The logic we are testing happens before the loop starts cancelFunc() - cronVulnerabilities(ctx, ds, kitlog.NewNopLogger(), "AAA", fleetConfig) + cronVulnerabilities(ctx, ds, kitlog.NewNopLogger(), "AAA", config) require.DirExists(t, vulnPath) } @@ -237,17 +235,15 @@ func TestCronVulnerabilitiesAcceptsExistingDbPath(t *testing.T) { return nil } - fleetConfig := config.FleetConfig{ - Vulnerabilities: config.VulnerabilitiesConfig{ - DatabasesPath: t.TempDir(), - Periodicity: 10 * time.Second, - CurrentInstanceChecks: "auto", - }, + config := config.VulnerabilitiesConfig{ + DatabasesPath: t.TempDir(), + Periodicity: 10 * time.Second, + CurrentInstanceChecks: "auto", } // We cancel right away so cronsVulnerailities finishes. The logic we are testing happens before the loop starts cancelFunc() - cronVulnerabilities(ctx, ds, logger, "AAA", fleetConfig) + cronVulnerabilities(ctx, ds, logger, "AAA", config) require.Contains(t, buf.String(), `"waiting":"on ticker"`) } @@ -276,17 +272,15 @@ func TestCronVulnerabilitiesQuitsIfErrorVulnPath(t *testing.T) { _, err := os.Create(fileVulnPath) require.NoError(t, err) - fleetConfig := config.FleetConfig{ - Vulnerabilities: config.VulnerabilitiesConfig{ - DatabasesPath: fileVulnPath, - Periodicity: 10 * time.Second, - CurrentInstanceChecks: "auto", - }, + config := config.VulnerabilitiesConfig{ + DatabasesPath: fileVulnPath, + Periodicity: 10 * time.Second, + CurrentInstanceChecks: "auto", } // We cancel right away so cronsVulnerailities finishes. The logic we are testing happens before the loop starts cancelFunc() - cronVulnerabilities(ctx, ds, logger, "AAA", fleetConfig) + cronVulnerabilities(ctx, ds, logger, "AAA", config) require.Contains(t, buf.String(), `"databases-path":"creation failed, returning"`) } @@ -312,17 +306,15 @@ func TestCronVulnerabilitiesSkipCreationIfStatic(t *testing.T) { vulnPath := path.Join(t.TempDir(), "something") require.NoDirExists(t, vulnPath) - fleetConfig := config.FleetConfig{ - Vulnerabilities: config.VulnerabilitiesConfig{ - DatabasesPath: vulnPath, - Periodicity: 10 * time.Second, - CurrentInstanceChecks: "1", - }, + config := config.VulnerabilitiesConfig{ + DatabasesPath: vulnPath, + Periodicity: 10 * time.Second, + CurrentInstanceChecks: "1", } // We cancel right away so cronsVulnerailities finishes. The logic we are testing happens before the loop starts cancelFunc() - cronVulnerabilities(ctx, ds, logger, "AAA", fleetConfig) + cronVulnerabilities(ctx, ds, logger, "AAA", config) require.NoDirExists(t, vulnPath) }