From 38f425510c0719144d8dfc5edf8cbbacfa5de5ad Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Thu, 16 Jul 2026 23:49:49 +0200 Subject: [PATCH] complete engine architecture contracts --- CHANGELOG.md | 13 +- cache_test.go | 30 +- engine_test.go | 3 +- gating_contract_test.go | 92 ++++++ internal/app/app.go | 51 ++-- internal/app/app_test.go | 278 +++++++++--------- internal/app/contract_test.go | 32 +- internal/app/disable_test.go | 10 +- internal/engine/az_test.go | 1 + internal/engine/cache.go | 16 +- internal/engine/cache_test.go | 47 +-- internal/engine/config.go | 27 +- internal/engine/config_benchmark_test.go | 2 +- internal/engine/config_test.go | 94 +++--- internal/engine/core.go | 13 +- internal/engine/core_gating_test.go | 260 ++++++++++++++-- internal/engine/defaults.go | 35 +++ internal/engine/defaults_test.go | 186 ++++++++++++ internal/engine/descriptors.go | 154 +++++----- internal/engine/descriptors_test.go | 88 +++++- internal/engine/discovery_plan.go | 25 +- internal/engine/disks.go | 4 - internal/engine/disks_test.go | 32 +- internal/engine/ec2_test.go | 1 + internal/engine/engine.go | 13 +- internal/engine/engine_test.go | 49 ++- internal/engine/external.go | 8 +- internal/engine/external_test.go | 63 ++-- internal/engine/gce_test.go | 1 + internal/engine/networking.go | 3 - internal/engine/networking_test.go | 28 +- internal/engine/os.go | 34 --- internal/engine/os_test.go | 48 +-- internal/engine/plan9.go | 6 +- internal/engine/plan9_parser_test.go | 30 +- internal/engine/seam_gate_test.go | 88 ++++++ .../.openspec.yaml | 2 + .../design.md | 96 ++++++ .../proposal.md | 28 ++ .../specs/fact-disable-controls/spec.md | 66 +++++ .../specs/facts-library-api/spec.md | 48 +++ .../tasks.md | 32 ++ 42 files changed, 1477 insertions(+), 660 deletions(-) create mode 100644 gating_contract_test.go create mode 100644 internal/engine/defaults.go create mode 100644 internal/engine/defaults_test.go create mode 100644 openspec/changes/complete-engine-architecture-contracts/.openspec.yaml create mode 100644 openspec/changes/complete-engine-architecture-contracts/design.md create mode 100644 openspec/changes/complete-engine-architecture-contracts/proposal.md create mode 100644 openspec/changes/complete-engine-architecture-contracts/specs/fact-disable-controls/spec.md create mode 100644 openspec/changes/complete-engine-architecture-contracts/specs/facts-library-api/spec.md create mode 100644 openspec/changes/complete-engine-architecture-contracts/tasks.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c1cd2f28..0aa2adfcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,11 @@ or fact group with `--disable a,b`, the `FACTS_DISABLE=a,b` environment variable, or the `facts.conf` `disable` key; the Facter `blocklist` key keeps working as its compatibility alias, and `--no-block` clears every disable - source. Disabling is resolution-gated — a disabled standalone-resolver fact - (`networking`, `processors`, `memory`, `ssh`, `timezone`, `fips`, `augeas`, - `xen`) is not resolved at all, not merely filtered from output; multi-output - categories resolve then prune. An explicitly queried fact disabled by the - environment or config emits a one-line stderr diagnostic. + source. Disabling is resolution-gated — a gateable resolver is skipped when + every top-level root it can emit is disabled; when any sibling root remains + enabled, the resolver runs and only disabled output is pruned. An explicitly + queried fact disabled by the environment or config emits a one-line stderr + diagnostic. - Release artifacts and cross-compile CI now include `arm` and `arm64` for FreeBSD, OpenBSD, and NetBSD. - Added native-gated Plan 9 (`plan9/amd64`) fact support for canonical @@ -53,6 +53,9 @@ ### Fixed +- Fully disabled multi-output and shared-output core resolvers now skip their + probe work, including demand-driven DMI/GCE sharing; enabled sibling roots, + output pruning, queries, cache behavior, and disable diagnostics are unchanged. - The DragonFly `disks`/`partitions` probe no longer reports empty memory-disk (`md`) phantoms and no longer fans out `disklabel` across non-existent slice targets. It skips `md`/`cd` pseudo-devices and enumerates only the slice nodes diff --git a/cache_test.go b/cache_test.go index 8ffe06097..a07194d69 100644 --- a/cache_test.go +++ b/cache_test.go @@ -11,16 +11,15 @@ import ( "github.com/ncode/facts/internal/engine" ) -// redirectCacheDir points the engine's persistent-cache location at a temp dir -// for the duration of the test, so WithCache exercises a real round trip -// without reading or writing the host's actual fact cache. -func redirectCacheDir(t *testing.T) string { +// cacheDefaults points one Engine at a temp persistent-cache location so +// WithCache exercises a real round trip without process-global mutation. +func cacheDefaults(t *testing.T) (string, Option) { t.Helper() dir := t.TempDir() - original := engine.DefaultCachePath - engine.DefaultCachePath = func() string { return dir } - t.Cleanup(func() { engine.DefaultCachePath = original }) - return dir + return dir, func(cfg *engine.EngineConfig) error { + cfg.Defaults = &engine.DiscoveryDefaults{CachePath: dir} + return nil + } } // writeTTLConfig writes a config file giving group a 30-day TTL, which is what @@ -68,10 +67,10 @@ func cachingResolver(value string) func(context.Context) (any, error) { // contract: opting in with WithCache and a TTL'd group causes Discover to // persist the freshly resolved fact to the cache directory. func TestWithCache_persistsResolvedFactToDisk(t *testing.T) { - dir := redirectCacheDir(t) + dir, defaults := cacheDefaults(t) conf := writeTTLConfig(t, "demo") - eng, err := New(WithCache(), WithConfigFile(conf), WithFact("demo", cachingResolver("fresh"))) + eng, err := New(WithCache(), WithConfigFile(conf), WithFact("demo", cachingResolver("fresh")), defaults) if err != nil { t.Fatal(err) } @@ -90,11 +89,11 @@ func TestWithCache_persistsResolvedFactToDisk(t *testing.T) { // freshly resolved one. A different resolver value makes the substitution // observable. func TestWithCache_servesFreshCachedValueOverResolver(t *testing.T) { - dir := redirectCacheDir(t) + dir, defaults := cacheDefaults(t) conf := writeTTLConfig(t, "demo") seedCacheFile(t, filepath.Join(dir, "demo"), map[string]any{"demo": "from-cache"}) - eng, err := New(WithCache(), WithConfigFile(conf), WithFact("demo", cachingResolver("fresh"))) + eng, err := New(WithCache(), WithConfigFile(conf), WithFact("demo", cachingResolver("fresh")), defaults) if err != nil { t.Fatal(err) } @@ -113,7 +112,7 @@ func TestWithCache_servesFreshCachedValueOverResolver(t *testing.T) { } func TestWithCache_selectsQueriedFactsThroughEngineCachePath(t *testing.T) { - dir := redirectCacheDir(t) + dir, defaults := cacheDefaults(t) conf := writeTTLConfig(t, "demo") seedCacheFile(t, filepath.Join(dir, "demo"), map[string]any{"demo": map[string]any{"child": "from-cache"}}) @@ -124,6 +123,7 @@ func TestWithCache_selectsQueriedFactsThroughEngineCachePath(t *testing.T) { return map[string]any{"child": "fresh"}, nil }), WithFact("other", cachingResolver("fresh")), + defaults, ) if err != nil { t.Fatal(err) @@ -144,11 +144,11 @@ func TestWithCache_selectsQueriedFactsThroughEngineCachePath(t *testing.T) { // cache that WithCache would serve is ignored when WithCache is absent, proving // the option — not some always-on path — is what enables caching. func TestWithoutCache_ignoresExistingCache(t *testing.T) { - dir := redirectCacheDir(t) + dir, defaults := cacheDefaults(t) conf := writeTTLConfig(t, "demo") seedCacheFile(t, filepath.Join(dir, "demo"), map[string]any{"demo": "from-cache"}) - eng, err := New(WithConfigFile(conf), WithFact("demo", cachingResolver("fresh"))) + eng, err := New(WithConfigFile(conf), WithFact("demo", cachingResolver("fresh")), defaults) if err != nil { t.Fatal(err) } diff --git a/engine_test.go b/engine_test.go index 545086e1c..8e9d013e7 100644 --- a/engine_test.go +++ b/engine_test.go @@ -142,7 +142,7 @@ func TestWithConfigFile_loadsConfiguredDirs(t *testing.T) { } func TestWithConfigFile_recomputesDiscoveryPolicyEachDiscover(t *testing.T) { - cacheDir := redirectCacheDir(t) + cacheDir, defaults := cacheDefaults(t) firstDir := t.TempDir() writeTestFile(t, firstDir, "site.txt", "site_location=first\n") writeTestFile(t, firstDir, "blocked.txt", "blocked_probe=blocked\n") @@ -162,6 +162,7 @@ facts : { WithCache(), WithConfigFile(configPath), WithFact("cache_probe", func(context.Context) (any, error) { return "cached", nil }), + defaults, ) if err != nil { t.Fatal(err) diff --git a/gating_contract_test.go b/gating_contract_test.go new file mode 100644 index 000000000..2386a08e9 --- /dev/null +++ b/gating_contract_test.go @@ -0,0 +1,92 @@ +package facts + +import ( + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "testing" +) + +func TestDiscover_gateableCoreFactsPreservePublicPruningAndQueries(t *testing.T) { + tests := []struct { + name string + disabled string + keptQuery string + pruned string + wantWarn string + }{ + { + name: "multi-output sibling remains resolved", + disabled: `"filesystems", "os", "system_profiler"`, + keptQuery: "kernel", + pruned: "os", + wantWarn: `fact "os" is disabled by the configuration file`, + }, + { + name: "disabled subfact is pruned after resolution", + disabled: `"os.release"`, + keptQuery: "os.name", + pruned: "os.release", + wantWarn: `fact "os.release" is disabled by the configuration file`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := writeTestFile(t, t.TempDir(), "facter.conf", "facts : {\n blocklist : [ "+tt.disabled+" ],\n}\n") + handler := &recordingHandler{} + eng, err := New(WithConfigFile(config), WithLogger(slog.New(handler))) + if err != nil { + t.Fatal(err) + } + + snap, err := eng.Discover(context.Background(), tt.keptQuery, tt.pruned) + if err != nil { + t.Fatalf("Discover() err = %v", err) + } + if _, err := snap.Value(tt.keptQuery); err != nil { + t.Fatalf("Value(%q) err = %v, want kept sibling resolved", tt.keptQuery, err) + } + if _, err := snap.Value(tt.pruned); !errors.Is(err, ErrFactNotFound) { + t.Fatalf("Value(%q) err = %v, want ErrFactNotFound", tt.pruned, err) + } + if !handler.hasWarn(tt.wantWarn) { + t.Fatalf("diagnostics = %#v, want WARN %q", handler.records, tt.wantWarn) + } + }) + } +} + +func TestDiscover_fullyDisabledCoreResolverSkipsCacheConsultation(t *testing.T) { + dir, defaults := cacheDefaults(t) + cachePath := filepath.Join(dir, "operating system") + if err := os.WriteFile(cachePath, []byte("{"), 0o600); err != nil { + t.Fatal(err) + } + config := writeTestFile(t, t.TempDir(), "facter.conf", `facts : { + blocklist : [ "filesystems", "kernel", "os", "system_profiler" ], + ttls : [ { "operating system" : "30 days" } ], +} +`) + handler := &recordingHandler{} + eng, err := New(WithCache(), WithConfigFile(config), WithLogger(slog.New(handler)), defaults) + if err != nil { + t.Fatal(err) + } + + snap, err := eng.Discover(context.Background(), "kernel") + if err != nil { + t.Fatalf("Discover() err = %v", err) + } + if _, err := snap.Value("kernel"); !errors.Is(err, ErrFactNotFound) { + t.Fatalf("Value(kernel) err = %v, want ErrFactNotFound", err) + } + if _, err := os.Stat(cachePath); err != nil { + t.Fatalf("malformed cache was consulted and removed: %v", err) + } + if want := `fact "kernel" is disabled by the configuration file`; !handler.hasWarn(want) { + t.Fatalf("diagnostics = %#v, want WARN %q", handler.records, want) + } +} diff --git a/internal/app/app.go b/internal/app/app.go index dd7797bf6..37e5c374e 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -14,10 +14,12 @@ import ( "github.com/ncode/facts/internal/engine" ) -var defaultExternalFactDirs = engine.CurrentDefaultExternalFactDirs - // Run executes the facts command with the provided arguments. func Run(stdout, stderr io.Writer, args []string) error { + return runWithDefaults(stdout, stderr, args, engine.CurrentDiscoveryDefaults()) +} + +func runWithDefaults(stdout, stderr io.Writer, args []string, defaults engine.DiscoveryDefaults) error { args = cli.PrepareArguments(args) if err := cli.ValidateOptions(args); err != nil { return optionError(stdout, err) @@ -37,30 +39,30 @@ func Run(stdout, stderr io.Writer, args []string) error { _, err := fmt.Fprintln(stdout, engine.Version) return err case "list_block_groups", "--list-block-groups": - groups, err := factGroups(args[1:]) + groups, err := factGroups(args[1:], defaults) if err != nil { return err } _, err = fmt.Fprintln(stdout, engine.FormatFactGroups(groups)) return err case "list_cache_groups", "--list-cache-groups": - groups, err := factGroups(args[1:]) + groups, err := factGroups(args[1:], defaults) if err != nil { return err } _, err = fmt.Fprintln(stdout, engine.FormatFactGroups(groups)) return err case "query": - return runQuery(stdout, stderr, args[1:]) + return runQuery(stdout, stderr, args[1:], defaults) default: return fmt.Errorf("unknown command %q", args[0]) } } -func factGroups(args []string) ([]engine.FactGroup, error) { +func factGroups(args []string, defaults engine.DiscoveryDefaults) ([]engine.FactGroup, error) { options := parseListOptions(args) groups := engine.BuiltinFactGroups() - config, err := engine.ParseConfig(options.ConfigPath, slog.New(slog.DiscardHandler)) + config, err := engine.ParseConfig(options.ConfigPath, slog.New(slog.DiscardHandler), defaults) if err != nil { return nil, err } @@ -69,7 +71,7 @@ func factGroups(args []string) ([]engine.FactGroup, error) { configuredExternalDirs := engine.DiscoveryExternalDirs(config, options.ExternalDirs, false, false, nil) var defaultExternalDirs []string if len(configuredExternalDirs) == 0 { - defaultExternalDirs = defaultExternalFactDirs() + defaultExternalDirs = defaults.ExternalFactDirs } externalDirs := engine.DiscoveryExternalDirs(config, options.ExternalDirs, false, true, defaultExternalDirs) external, err := engine.ExternalFactGroups(externalDirs) @@ -149,7 +151,7 @@ Format facts as JSON: return b.String() } -func runQuery(stdout, stderr io.Writer, args []string) error { +func runQuery(stdout, stderr io.Writer, args []string, defaults engine.DiscoveryDefaults) error { flags, options, err := parseOptions("query", stderr, args) if err != nil { return err @@ -163,7 +165,7 @@ func runQuery(stdout, stderr io.Writer, args []string) error { logHandler := &stderrLogHandler{stderr: stderr, color: colorDiagnostics} logger := slog.New(logHandler) configFile := options.ConfigPath - configOptions, configErr := engine.ParseConfig(configFile, logger) + configOptions, configErr := engine.ParseConfig(configFile, logger, defaults) if configErr != nil { return configErr } @@ -185,7 +187,7 @@ func runQuery(stdout, stderr io.Writer, args []string) error { } var defaultExternalDirs []string if !options.NoExternalFacts && len(conflictExternalDirs) == 0 { - defaultExternalDirs = defaultExternalFactDirs() + defaultExternalDirs = defaults.ExternalFactDirs } discoveryExternalDirs := engine.DiscoveryExternalDirs(configOptions, options.ExternalDirs, options.NoExternalFacts, true, defaultExternalDirs) disabledFactsForFastPath := map[string]bool{} @@ -227,20 +229,19 @@ func runQuery(stdout, stderr io.Writer, args []string) error { resolutionStart := time.Now() eng, err := engine.NewEngine(engine.EngineConfig{ - CLICompat: true, - SystemDefaults: true, - ConfigFile: configFile, - ConfigLoaded: true, - Config: configOptions, - ExternalDirs: cliExternalDirs, - UseCache: !options.NoCache, - NoExternalFacts: options.NoExternalFacts, - DisabledFacts: disabledFacts, - ExtraDisabled: options.DisableEntries, - DefaultExternalDirsSet: true, - DefaultExternalDirs: defaultExternalDirs, - IncludeTypedDotted: mergeDottedFacts, - Logger: logger, + CLICompat: true, + SystemDefaults: true, + ConfigFile: configFile, + ConfigLoaded: true, + Config: configOptions, + ExternalDirs: cliExternalDirs, + UseCache: !options.NoCache, + NoExternalFacts: options.NoExternalFacts, + DisabledFacts: disabledFacts, + ExtraDisabled: options.DisableEntries, + Defaults: &defaults, + IncludeTypedDotted: mergeDottedFacts, + Logger: logger, }) if err != nil { return err diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 77bc574b3..d22c7f92a 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -3,6 +3,7 @@ package app import ( "bytes" "encoding/json" + "io" "os" "path/filepath" "regexp" @@ -13,9 +14,8 @@ import ( "github.com/ncode/facts/internal/engine" ) -func TestMain(m *testing.M) { - defaultExternalFactDirs = func() []string { return nil } - os.Exit(m.Run()) +func runForTest(stdout, stderr io.Writer, args []string) error { + return runWithDefaults(stdout, stderr, args, engine.DiscoveryDefaults{}) } func seedAppCacheFile(t *testing.T, path string, data map[string]any) { @@ -47,7 +47,7 @@ func TestRun_version(t *testing.T) { func TestRun_shortVersion(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"-v"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"-v"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -64,7 +64,7 @@ func TestRun_shortVersion(t *testing.T) { func TestRun_facterversionJSONFastPathBytes(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--json", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--json", "facterversion"}); err != nil { t.Fatal(err) } want := "{\n \"facterversion\": \"" + engine.Version + "\"\n}\n" @@ -79,7 +79,7 @@ func TestRun_facterversionJSONFastPathBytes(t *testing.T) { func TestRun_facterversionHOCONFastPathBytes(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--hocon", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--hocon", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -103,7 +103,7 @@ func TestRun_facterversionFastPathIgnoresColorAndForceDotResolution(t *testing.T t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, tt.args); err != nil { + if err := runForTest(&stdout, &stderr, tt.args); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -123,7 +123,7 @@ func TestRun_facterversionQueryAllowsExternalOverride(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "facterversion"}); err != nil { t.Fatal(err) } if got, want := strings.TrimSpace(stdout.String()), "external"; got != want { @@ -147,11 +147,11 @@ func TestRun_acceptsCompatibilityFlagsAndConfigToggles(t *testing.T) { {"--log-level", "trace", "facterversion"}, } { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, args); err != nil { - t.Fatalf("Run(%v) err = %v, want compatibility options accepted", args, err) + if err := runForTest(&stdout, &stderr, args); err != nil { + t.Fatalf("runForTest(%v) err = %v, want compatibility options accepted", args, err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { - t.Fatalf("Run(%v) stdout = %q, want %q", args, got, want) + t.Fatalf("runForTest(%v) stdout = %q, want %q", args, got, want) } } } @@ -159,7 +159,7 @@ func TestRun_acceptsCompatibilityFlagsAndConfigToggles(t *testing.T) { func TestRun_helpListsSupportedCompatibilityOptions(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--help"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--help"}); err != nil { t.Fatal(err) } for _, want := range []string{ @@ -191,7 +191,7 @@ func TestRun_helpListsSupportedCompatibilityOptions(t *testing.T) { func TestRun_manPrintsManual(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--man"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--man"}); err != nil { t.Fatal(err) } for _, want := range []string{ @@ -213,7 +213,7 @@ func TestRun_manPrintsManual(t *testing.T) { func TestRun_queryJSON(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--json", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--json", "os.name"}); err != nil { t.Fatal(err) } var got map[string]string @@ -229,7 +229,7 @@ func TestRun_warnsAndIgnoresUnreadableConfig(t *testing.T) { configPath := filepath.Join(t.TempDir(), "missing.conf") var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { t.Fatal(err) } if strings.TrimSpace(stdout.String()) != engine.Version { @@ -244,7 +244,7 @@ func TestRun_warnsAndIgnoresUnreadableConfig(t *testing.T) { func TestRun_noQueryJSONReturnsFullFactMap(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -276,7 +276,7 @@ func TestRun_configBlocklistSkipsExternalFactFile(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "--json", "site_location", "site_owner"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "--json", "site_location", "site_owner"}); err != nil { t.Fatal(err) } var got map[string]any @@ -312,7 +312,7 @@ fact-groups : { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "--json", "site_role", "site_owner"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "--json", "site_role", "site_owner"}); err != nil { t.Fatal(err) } var got map[string]any @@ -345,12 +345,12 @@ func TestRun_rejectsInvalidOptionPairsFromConfig(t *testing.T) { } var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) + err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) if err == nil { - t.Fatal("Run() err = nil, want configured option conflict") + t.Fatal("runForTest() err = nil, want configured option conflict") } if got, want := err.Error(), "--no-external-facts and --external-dir options conflict: please specify only one"; got != want { - t.Fatalf("Run() err = %q, want %q", got, want) + t.Fatalf("runForTest() err = %q, want %q", got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -370,7 +370,7 @@ func TestRun_externalExecutableStderrIsWrittenAsWarning(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "script_one"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "script_one"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "two\n"; got != want { @@ -394,7 +394,7 @@ func TestRun_colorOutputsYellowWarnings(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--color", "--external-dir", dir, "script_one"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--color", "--external-dir", dir, "script_one"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "two\n"; got != want { @@ -416,12 +416,9 @@ func TestRun_loadsDefaultConfigWhenConfigFlagIsOmitted(t *testing.T) { if err := os.WriteFile(configPath, []byte(conf), 0o600); err != nil { t.Fatal(err) } - oldDefaultConfigPath := engine.DefaultConfigPath - engine.DefaultConfigPath = func() string { return configPath } - t.Cleanup(func() { engine.DefaultConfigPath = oldDefaultConfigPath }) var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"site_location"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"site_location"}, engine.DiscoveryDefaults{CompatibleConfigPath: configPath}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "lab\n"; got != want { @@ -440,7 +437,7 @@ func TestRun_configCliDebugEmitsDebugLogs(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { t.Fatal(err) } if !strings.Contains(stderr.String(), "DEBUG Facts - resolving facts") { @@ -456,7 +453,7 @@ func TestRun_configCliVerboseEmitsInfoLogs(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -478,7 +475,7 @@ func TestRun_configCliLogLevelDebugEmitsDebugLogs(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -497,12 +494,12 @@ func TestRun_rejectsUnsupportedConfiguredLogLevel(t *testing.T) { } var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) + err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) if err == nil { - t.Fatal("Run() err = nil, want unsupported configured log-level error") + t.Fatal("runForTest() err = nil, want unsupported configured log-level error") } if got, want := err.Error(), "unsupported log level loud"; got != want { - t.Fatalf("Run() err = %q, want %q", got, want) + t.Fatalf("runForTest() err = %q, want %q", got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -518,12 +515,12 @@ func TestRun_rejectsConflictingConfiguredLogOptions(t *testing.T) { } var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) + err := runForTest(&stdout, &stderr, []string{"--config", configPath, "facterversion"}) if err == nil { - t.Fatal("Run() err = nil, want configured log option conflict") + t.Fatal("runForTest() err = nil, want configured log option conflict") } if got, want := err.Error(), "debug, verbose, and log-level options conflict: please specify only one."; got != want { - t.Fatalf("Run() err = %q, want %q", got, want) + t.Fatalf("runForTest() err = %q, want %q", got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -547,7 +544,7 @@ func TestRun_cliExternalDirOverridesConfiguredExternalDir(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--external-dir", cliDir, "--json", "site_location", "config_only"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--external-dir", cliDir, "--json", "site_location", "config_only"}); err != nil { t.Fatal(err) } var got map[string]any @@ -577,12 +574,10 @@ func TestRun_configTTLsUseFreshCachedFact(t *testing.T) { t.Fatal(err) } cacheDir := t.TempDir() - oldDefaultCachePath := engine.DefaultCachePath - engine.DefaultCachePath = func() string { return cacheDir } - t.Cleanup(func() { engine.DefaultCachePath = oldDefaultCachePath }) + defaults := engine.DiscoveryDefaults{CachePath: cacheDir} var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "site_role"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "site_role"}, defaults); err != nil { t.Fatal(err) } if got, want := stdout.String(), "web\n"; got != want { @@ -597,7 +592,7 @@ func TestRun_configTTLsUseFreshCachedFact(t *testing.T) { stdout.Reset() stderr.Reset() - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "site_role"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"--config", configPath, "--external-dir", dir, "site_role"}, defaults); err != nil { t.Fatal(err) } if got, want := stdout.String(), "web\n"; got != want { @@ -620,13 +615,11 @@ func TestRun_noCacheIgnoresFreshCachedFact(t *testing.T) { t.Fatal(err) } cacheDir := t.TempDir() - oldDefaultCachePath := engine.DefaultCachePath - engine.DefaultCachePath = func() string { return cacheDir } - t.Cleanup(func() { engine.DefaultCachePath = oldDefaultCachePath }) + defaults := engine.DiscoveryDefaults{CachePath: cacheDir} seedAppCacheFile(t, filepath.Join(cacheDir, "site_role"), map[string]any{"site_role": "from-cache"}) var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--no-cache", "--config", configPath, "--external-dir", dir, "site_role"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"--no-cache", "--config", configPath, "--external-dir", dir, "site_role"}, defaults); err != nil { t.Fatal(err) } if got, want := stdout.String(), "fresh\n"; got != want { @@ -640,13 +633,13 @@ func TestRun_noCacheIgnoresFreshCachedFact(t *testing.T) { func TestRun_strictLogsMissingFactErrorWhenQueriedFactIsMissing(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--strict", "--json", "os.name", "missing_fact"}) + err := runForTest(&stdout, &stderr, []string{"--strict", "--json", "os.name", "missing_fact"}) status, ok := err.(ExitStatus) if !ok { - t.Fatalf("Run() err = %T %[1]v, want ExitStatus", err) + t.Fatalf("runForTest() err = %T %[1]v, want ExitStatus", err) } if status.Code() != 1 { - t.Fatalf("Run() status = %d, want 1", status.Code()) + t.Fatalf("runForTest() status = %d, want 1", status.Code()) } var got map[string]any if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { @@ -704,18 +697,18 @@ func TestRun_noCacheDisabledQueriedFactKeepsProjectionPlaceholder(t *testing.T) t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, tt.args) + err := runForTest(&stdout, &stderr, tt.args) if tt.wantStatus == 0 { if err != nil { - t.Fatalf("Run(%v) err = %v, want nil", tt.args, err) + t.Fatalf("runForTest(%v) err = %v, want nil", tt.args, err) } } else { status, ok := err.(ExitStatus) if !ok { - t.Fatalf("Run(%v) err = %T %[2]v, want ExitStatus", tt.args, err) + t.Fatalf("runForTest(%v) err = %T %[2]v, want ExitStatus", tt.args, err) } if status.Code() != tt.wantStatus { - t.Fatalf("Run(%v) status = %d, want %d", tt.args, status.Code(), tt.wantStatus) + t.Fatalf("runForTest(%v) status = %d, want %d", tt.args, status.Code(), tt.wantStatus) } } if got := stdout.String(); got != tt.wantStdout { @@ -731,7 +724,7 @@ func TestRun_noCacheDisabledQueriedFactKeepsProjectionPlaceholder(t *testing.T) func TestRun_queryNoJSONUsesLegacyOutput(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--no-json", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--no-json", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -747,7 +740,7 @@ func TestRun_noFormatCompatibilityFlagsAreAcceptedAndInert(t *testing.T) { t.Run(flag, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{flag, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{flag, "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -763,7 +756,7 @@ func TestRun_noFormatCompatibilityFlagsAreAcceptedAndInert(t *testing.T) { func TestRun_timingPrintsResolutionDuration(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--timing", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--timing", "facterversion"}); err != nil { t.Fatal(err) } lines := strings.Split(strings.TrimSpace(stdout.String()), "\n") @@ -785,13 +778,13 @@ func TestRun_timingPrintsResolutionDuration(t *testing.T) { func TestRun_sharedPresentationProjectionPreservesTimingOutputAndStrictOrder(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--timing", "--strict", "--json", "os.name", "missing_fact", "missing_fact"}) + err := runForTest(&stdout, &stderr, []string{"--timing", "--strict", "--json", "os.name", "missing_fact", "missing_fact"}) status, ok := err.(ExitStatus) if !ok { - t.Fatalf("Run() err = %T %[1]v, want ExitStatus", err) + t.Fatalf("runForTest() err = %T %[1]v, want ExitStatus", err) } if status.Code() != 1 { - t.Fatalf("Run() status = %d, want 1", status.Code()) + t.Fatalf("runForTest() status = %d, want 1", status.Code()) } lines := strings.Split(strings.TrimSpace(stdout.String()), "\n") @@ -840,12 +833,12 @@ func TestRun_rejectsRemovedCustomFactOptions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, tt.args) + err := runForTest(&stdout, &stderr, tt.args) if err == nil { - t.Fatalf("Run(%v) err = nil, want unknown option error", tt.args) + t.Fatalf("runForTest(%v) err = nil, want unknown option error", tt.args) } if got, want := err.Error(), "unrecognised option '"+tt.option+"'"; got != want { - t.Fatalf("Run(%v) err = %q, want %q", tt.args, got, want) + t.Fatalf("runForTest(%v) err = %q, want %q", tt.args, got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -869,12 +862,12 @@ func TestRun_rejectsRemovedPuppetOptions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, tt.args) + err := runForTest(&stdout, &stderr, tt.args) if err == nil { - t.Fatalf("Run(%v) err = nil, want unknown option error", tt.args) + t.Fatalf("runForTest(%v) err = nil, want unknown option error", tt.args) } if got, want := err.Error(), "unrecognised option '"+tt.option+"'"; got != want { - t.Fatalf("Run(%v) err = %q, want %q", tt.args, got, want) + t.Fatalf("runForTest(%v) err = %q, want %q", tt.args, got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -887,7 +880,7 @@ func TestRun_rejectsRemovedPuppetOptions(t *testing.T) { func TestRun_queryYAML(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--yaml", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--yaml", "os.name"}); err != nil { t.Fatal(err) } if got := stdout.String(); !strings.HasPrefix(got, "os.name: ") || strings.TrimSpace(strings.TrimPrefix(got, "os.name: ")) == "" { @@ -898,7 +891,7 @@ func TestRun_queryYAML(t *testing.T) { func TestRun_queryFacterversionYAMLHasSingleTrailingNewline(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--yaml", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--yaml", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "facterversion: "+engine.Version+"\n"; got != want { @@ -909,7 +902,7 @@ func TestRun_queryFacterversionYAMLHasSingleTrailingNewline(t *testing.T) { func TestRun_queryHOCON(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--hocon", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--hocon", "os.name"}); err != nil { t.Fatal(err) } if got := strings.TrimSpace(stdout.String()); got == "" { @@ -920,7 +913,7 @@ func TestRun_queryHOCON(t *testing.T) { func TestRun_queryLegacy(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -931,7 +924,7 @@ func TestRun_queryLegacy(t *testing.T) { func TestRun_acceptsLogLevelCompatibilityFlag(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--log-level", "none", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--log-level", "none", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -945,7 +938,7 @@ func TestRun_acceptsLogLevelCompatibilityFlag(t *testing.T) { func TestRun_acceptsLogLevelPlaceholderCompatibilityValue(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--log-level", "log_level", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--log-level", "log_level", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -971,7 +964,7 @@ func TestRun_logLevelDebugOutputsDebugLogs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, tt.args); err != nil { + if err := runForTest(&stdout, &stderr, tt.args); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -991,7 +984,7 @@ func TestRun_debugColorOutputsColorizedDebugLogs(t *testing.T) { t.Setenv("TERM", "xterm-256color") var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--debug", "--color", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--debug", "--color", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -1008,7 +1001,7 @@ func TestRun_debugColorOutputsColorizedDebugLogs(t *testing.T) { func TestRun_verboseOutputsInfoLogs(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--verbose", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--verbose", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -1025,7 +1018,7 @@ func TestRun_verboseOutputsInfoLogs(t *testing.T) { func TestRun_verboseColorOutputsGreenInfoLogs(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--verbose", "--color", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--verbose", "--color", "facterversion"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -1053,7 +1046,7 @@ func TestRun_acceptsNoOpCompatibilityFlags(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, tt.args); err != nil { + if err := runForTest(&stdout, &stderr, tt.args); err != nil { t.Fatal(err) } if got, want := stdout.String(), engine.Version+"\n"; got != want { @@ -1069,7 +1062,7 @@ func TestRun_acceptsNoOpCompatibilityFlags(t *testing.T) { func TestRun_queryStructuredRootFactJSON(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--json", "os"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--json", "os"}); err != nil { t.Fatal(err) } var got map[string]map[string]any @@ -1105,7 +1098,7 @@ func TestRun_colorColorsDefaultFormatKeysByDepth(t *testing.T) { dir := colorTreeExternalDir(t) var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--color", "--external-dir", dir, "colortree", "simple"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--color", "--external-dir", dir, "colortree", "simple"}); err != nil { t.Fatal(err) } want := "\x1b[36mcolortree\x1b[0m => {\n" + @@ -1130,11 +1123,11 @@ func TestRun_defaultFormatHasNoANSIWithoutColor(t *testing.T) { {"--no-color", "--external-dir", dir, "colortree", "simple"}, } { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, args); err != nil { - t.Fatalf("Run(%v) err = %v", args, err) + if err := runForTest(&stdout, &stderr, args); err != nil { + t.Fatalf("runForTest(%v) err = %v", args, err) } if strings.Contains(stdout.String(), "\x1b[") { - t.Fatalf("Run(%v) stdout = %q, want no ANSI escape sequences", args, stdout.String()) + t.Fatalf("runForTest(%v) stdout = %q, want no ANSI escape sequences", args, stdout.String()) } } } @@ -1146,10 +1139,10 @@ func TestRun_machineFormatsAreByteIdenticalWithAndWithoutColor(t *testing.T) { t.Run(format, func(t *testing.T) { var plain, colored bytes.Buffer var stderr bytes.Buffer - if err := Run(&plain, &stderr, []string{format, "--external-dir", dir, "colortree", "simple"}); err != nil { + if err := runForTest(&plain, &stderr, []string{format, "--external-dir", dir, "colortree", "simple"}); err != nil { t.Fatal(err) } - if err := Run(&colored, &stderr, []string{format, "--color", "--external-dir", dir, "colortree", "simple"}); err != nil { + if err := runForTest(&colored, &stderr, []string{format, "--color", "--external-dir", dir, "colortree", "simple"}); err != nil { t.Fatal(err) } if !bytes.Equal(plain.Bytes(), colored.Bytes()) { @@ -1170,7 +1163,7 @@ func TestRun_queryExternalTxtFact(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "owner"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "owner"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "platform=team\n"; got != want { @@ -1181,26 +1174,23 @@ func TestRun_queryExternalTxtFact(t *testing.T) { } } -func defaultExternalFactDirForTest(t *testing.T) string { +func defaultExternalFactDirForTest(t *testing.T) (string, engine.DiscoveryDefaults) { t.Helper() dir := filepath.Join(t.TempDir(), "facts.d") if err := os.MkdirAll(dir, 0o700); err != nil { t.Fatal(err) } - old := defaultExternalFactDirs - defaultExternalFactDirs = func() []string { return []string{dir} } - t.Cleanup(func() { defaultExternalFactDirs = old }) - return dir + return dir, engine.DiscoveryDefaults{ExternalFactDirs: []string{dir}} } func TestRun_queryDefaultExternalFactDirectory(t *testing.T) { - dir := defaultExternalFactDirForTest(t) + dir, defaults := defaultExternalFactDirForTest(t) if err := os.WriteFile(filepath.Join(dir, "site.txt"), []byte("site_location=default\n"), 0o600); err != nil { t.Fatal(err) } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"site_location"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"site_location"}, defaults); err != nil { t.Fatal(err) } if got, want := stdout.String(), "default\n"; got != want { @@ -1212,7 +1202,7 @@ func TestRun_queryDefaultExternalFactDirectory(t *testing.T) { } func TestRun_externalDirOverridesDefaultExternalFactDirectory(t *testing.T) { - defaultDir := defaultExternalFactDirForTest(t) + defaultDir, defaults := defaultExternalFactDirForTest(t) if err := os.WriteFile(filepath.Join(defaultDir, "site.txt"), []byte("site_location=default\n"), 0o600); err != nil { t.Fatal(err) } @@ -1222,7 +1212,7 @@ func TestRun_externalDirOverridesDefaultExternalFactDirectory(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", cliDir, "site_location"}); err != nil { + if err := runWithDefaults(&stdout, &stderr, []string{"--external-dir", cliDir, "site_location"}, defaults); err != nil { t.Fatal(err) } if got, want := stdout.String(), "cli\n"; got != want { @@ -1241,7 +1231,7 @@ func TestRun_externalFactOverridesCoreFactInFullJSON(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -1264,7 +1254,7 @@ func TestRun_queryExternalEnvironmentFact(t *testing.T) { t.Setenv("FACTER_site_location", "lab") var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"site_location"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"site_location"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "lab\n"; got != want { @@ -1286,7 +1276,7 @@ func TestRun_queryExecutableExternalFact(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "dynamic_owner"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "dynamic_owner"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "platform\n"; got != want { @@ -1305,7 +1295,7 @@ func TestRun_queryExternalYAMLArrayIndex(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "arr_ext_fact.0"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "arr_ext_fact.0"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "ex1\n"; got != want { @@ -1329,7 +1319,7 @@ func TestRun_warnsAndSkipsExecutableExternalFactsDuringRecursiveResolution(t *te } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -1355,12 +1345,12 @@ func TestRun_rejectsNoExternalFactsWithExternalDir(t *testing.T) { } var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--no-external-facts", "owner"}) + err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--no-external-facts", "owner"}) if err == nil { - t.Fatal("Run() err = nil, want conflicting option error") + t.Fatal("runForTest() err = nil, want conflicting option error") } if !strings.Contains(err.Error(), "--no-external-facts and --external-dir options conflict") { - t.Fatalf("Run() err = %q, want external-dir conflict", err) + t.Fatalf("runForTest() err = %q, want external-dir conflict", err) } assertUsageOutput(t, stdout.String()) } @@ -1369,12 +1359,12 @@ func TestRun_rejectsNoExternalFactsWithExternalDirEquals(t *testing.T) { dir := t.TempDir() var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--external-dir=" + dir, "--no-external-facts", "owner"}) + err := runForTest(&stdout, &stderr, []string{"--external-dir=" + dir, "--no-external-facts", "owner"}) if err == nil { - t.Fatal("Run() err = nil, want conflicting option error") + t.Fatal("runForTest() err = nil, want conflicting option error") } if !strings.Contains(err.Error(), "--no-external-facts and --external-dir options conflict") { - t.Fatalf("Run() err = %q, want external-dir conflict", err) + t.Fatalf("runForTest() err = %q, want external-dir conflict", err) } assertUsageOutput(t, stdout.String()) } @@ -1382,12 +1372,12 @@ func TestRun_rejectsNoExternalFactsWithExternalDirEquals(t *testing.T) { func TestRun_rejectsConflictingOutputFormats(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--json", "--yaml", "os.name"}) + err := runForTest(&stdout, &stderr, []string{"--json", "--yaml", "os.name"}) if err == nil { - t.Fatal("Run() err = nil, want conflicting option error") + t.Fatal("runForTest() err = nil, want conflicting option error") } if !strings.Contains(err.Error(), "--json and --yaml options conflict") { - t.Fatalf("Run() err = %q, want output format conflict", err) + t.Fatalf("runForTest() err = %q, want output format conflict", err) } assertUsageOutput(t, stdout.String()) } @@ -1395,12 +1385,12 @@ func TestRun_rejectsConflictingOutputFormats(t *testing.T) { func TestRun_rejectsConflictingHOCONOutputToggle(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--hocon", "--no-hocon", "os.name"}) + err := runForTest(&stdout, &stderr, []string{"--hocon", "--no-hocon", "os.name"}) if err == nil { - t.Fatal("Run() err = nil, want conflicting option error") + t.Fatal("runForTest() err = nil, want conflicting option error") } if !strings.Contains(err.Error(), "--hocon and --no-hocon options conflict") { - t.Fatalf("Run() err = %q, want hocon conflict", err) + t.Fatalf("runForTest() err = %q, want hocon conflict", err) } assertUsageOutput(t, stdout.String()) } @@ -1408,7 +1398,7 @@ func TestRun_rejectsConflictingHOCONOutputToggle(t *testing.T) { func TestRun_noQueryPrintsKnownCoreFacts(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, nil); err != nil { + if err := runForTest(&stdout, &stderr, nil); err != nil { t.Fatal(err) } for _, want := range []string{"facterversion => " + engine.Version, "os => {", "name => \"" + runtimeOSName() + "\""} { @@ -1442,12 +1432,12 @@ func TestRun_rejectsRemovedShowLegacyOptions(t *testing.T) { for _, option := range []string{"--show-legacy", "--no-show-legacy"} { t.Run(option, func(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{option}) + err := runForTest(&stdout, &stderr, []string{option}) if err == nil { - t.Fatalf("Run(%s) err = nil, want unknown option error", option) + t.Fatalf("runForTest(%s) err = nil, want unknown option error", option) } if got, want := err.Error(), "unrecognised option '"+option+"'"; got != want { - t.Fatalf("Run(%s) err = %q, want %q", option, got, want) + t.Fatalf("runForTest(%s) err = %q, want %q", option, got, want) } assertUsageOutput(t, stdout.String()) if stderr.Len() != 0 { @@ -1468,8 +1458,8 @@ func TestRun_configShowLegacyKeyIsInert(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--json"}); err != nil { - t.Fatalf("Run() err = %v, want retired show-legacy key ignored", err) + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--json"}); err != nil { + t.Fatalf("runForTest() err = %v, want retired show-legacy key ignored", err) } var got map[string]any if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { @@ -1497,7 +1487,7 @@ func TestRun_configBlocklistLegacyIsInertInDefaultOutput(t *testing.T) { } var withBlocklist, stderr bytes.Buffer - if err := Run(&withBlocklist, &stderr, []string{"-c", path, "--json"}); err != nil { + if err := runForTest(&withBlocklist, &stderr, []string{"-c", path, "--json"}); err != nil { t.Fatal(err) } if stderr.Len() != 0 { @@ -1523,7 +1513,7 @@ func TestRun_configAllowsNoExternalFactsWithEmptyExternalDir(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", path, "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", path, "facterversion"}); err != nil { t.Fatal(err) } if got, want := strings.TrimSpace(stdout.String()), engine.Version; got != want { @@ -1546,7 +1536,7 @@ func TestRun_configForceDotResolutionAllowsPartialDottedExternalFactQuery(t *tes } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--json", "a.b"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--json", "a.b"}); err != nil { t.Fatal(err) } var got map[string]map[string]string @@ -1568,7 +1558,7 @@ func TestRun_forceDotResolutionAllowsPartialDottedExternalFactQuery(t *testing.T } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", externalDir, "--force-dot-resolution", "--json", "a.b"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", externalDir, "--force-dot-resolution", "--json", "a.b"}); err != nil { t.Fatal(err) } var got map[string]map[string]string @@ -1590,7 +1580,7 @@ func TestRun_forceDotResolutionMergesDottedExternalFactWithoutQuery(t *testing.T } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", externalDir, "--force-dot-resolution", "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", externalDir, "--force-dot-resolution", "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -1621,7 +1611,7 @@ func TestRun_configBlocklistGroupSuppressesGroupFacts(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", path, "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", path, "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -1652,7 +1642,7 @@ func TestRun_noBlockIgnoresConfiguredBlocklist(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", path, "--no-block", "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", path, "--no-block", "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -1676,7 +1666,7 @@ func TestRun_noBlockIgnoresConfiguredBlocklist(t *testing.T) { func TestRun_concatenatedShortJSONAndDebugFlags(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"-jd", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"-jd", "os.name"}); err != nil { t.Fatal(err) } var got map[string]string @@ -1694,7 +1684,7 @@ func TestRun_concatenatedShortJSONAndDebugFlags(t *testing.T) { func TestRun_concatenatedShortTimingAndDebugFlags(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"-td", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"-td", "os.name"}); err != nil { t.Fatal(err) } if !strings.Contains(stdout.String(), "fact 'os.name', took: ") { @@ -1708,7 +1698,7 @@ func TestRun_concatenatedShortTimingAndDebugFlags(t *testing.T) { func TestRun_concatenatedShortJSONDebugAndTimingFlags(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"-jdt", "os.name"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"-jdt", "os.name"}); err != nil { t.Fatal(err) } if !strings.Contains(stdout.String(), "\"os.name\"") { @@ -1725,12 +1715,12 @@ func TestRun_concatenatedShortJSONDebugAndTimingFlags(t *testing.T) { func TestRun_concatenatedShortFlagsRejectUnknownOption(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"-jdtz"}) + err := runForTest(&stdout, &stderr, []string{"-jdtz"}) if err == nil { - t.Fatal("Run() err = nil, want unknown option") + t.Fatal("runForTest() err = nil, want unknown option") } if !strings.Contains(err.Error(), "unrecognised option '-z'") { - t.Fatalf("Run() err = %q, want unknown -z option", err) + t.Fatalf("runForTest() err = %q, want unknown -z option", err) } assertUsageOutput(t, stdout.String()) } @@ -1740,7 +1730,7 @@ func TestRun_helpPrintsUsage(t *testing.T) { t.Run(arg, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{arg}); err != nil { + if err := runForTest(&stdout, &stderr, []string{arg}); err != nil { t.Fatal(err) } for _, want := range []string{"Usage", "facts [options] [query]", "--list-block-groups"} { @@ -1767,8 +1757,8 @@ func assertUsageOutput(t *testing.T, got string) { func runAppOutput(t *testing.T, args ...string) (string, string) { t.Helper() var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, args); err != nil { - t.Fatalf("Run(%v) err = %v", args, err) + if err := runForTest(&stdout, &stderr, args); err != nil { + t.Fatalf("runForTest(%v) err = %v", args, err) } return stdout.String(), stderr.String() } @@ -1826,8 +1816,8 @@ func TestRun_listTasksIgnoreTrailingTaskFlags(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, tt.args); err != nil { - t.Fatalf("Run(%v) err = %v, want nil", tt.args, err) + if err := runForTest(&stdout, &stderr, tt.args); err != nil { + t.Fatalf("runForTest(%v) err = %v, want nil", tt.args, err) } if !strings.Contains(stdout.String(), tt.want) { t.Fatalf("stdout = %q, want substring %q", stdout.String(), tt.want) @@ -1842,7 +1832,7 @@ func TestRun_listTasksIgnoreTrailingTaskFlags(t *testing.T) { func TestRun_listBlockGroupsPrintsFactGroups(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--list-block-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--list-block-groups"}); err != nil { t.Fatal(err) } for _, want := range []string{"networking", "- networking", "operating system", "- os"} { @@ -1858,7 +1848,7 @@ func TestRun_listBlockGroupsPrintsFactGroups(t *testing.T) { func TestRun_listCacheGroupsPrintsFactGroups(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--list-cache-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--list-cache-groups"}); err != nil { t.Fatal(err) } for _, want := range []string{"networking", "- networking", "processor", "- processors"} { @@ -1958,8 +1948,8 @@ func TestRun_listTasksScanWholeTailForConfigAndExternalDirs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, tt.args); err != nil { - t.Fatalf("Run(%v) err = %v, want nil", tt.args, err) + if err := runForTest(&stdout, &stderr, tt.args); err != nil { + t.Fatalf("runForTest(%v) err = %v, want nil", tt.args, err) } if !strings.Contains(stdout.String(), tt.want) { t.Fatalf("stdout = %q, want substring %q", stdout.String(), tt.want) @@ -1981,7 +1971,7 @@ func TestRun_listCacheGroupsIncludesConfiguredFactGroups(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--list-cache-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--list-cache-groups"}); err != nil { t.Fatal(err) } for _, want := range []string{"cached-custom-facts", "- site_role", "- site_location"} { @@ -2004,7 +1994,7 @@ func TestRun_listCacheGroupsAcceptsShortConfigEquals(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"-c=" + configPath, "--list-cache-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"-c=" + configPath, "--list-cache-groups"}); err != nil { t.Fatal(err) } if !strings.Contains(stdout.String(), "cached-custom-facts\n- site_role") { @@ -2028,7 +2018,7 @@ func TestRun_listCacheGroupsIncludesExternalDirectoryEntries(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--list-cache-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--list-cache-groups"}); err != nil { t.Fatal(err) } for _, want := range []string{"site.yaml\n", ".ignored.yaml\n", "nested\n"} { @@ -2051,7 +2041,7 @@ func TestRun_listCacheGroupsConfiguredGroupOverridesDefault(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--list-cache-groups"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--list-cache-groups"}); err != nil { t.Fatal(err) } if !strings.Contains(stdout.String(), "memory\n- custom_memory_fact") { @@ -2069,7 +2059,7 @@ func BenchmarkRunJSONSingleFact(b *testing.B) { b.ReportAllocs() for b.Loop() { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--no-cache", "--json", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--no-cache", "--json", "facterversion"}); err != nil { b.Fatal(err) } } @@ -2079,7 +2069,7 @@ func BenchmarkRunLegacySingleFact(b *testing.B) { b.ReportAllocs() for b.Loop() { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--no-cache", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--no-cache", "facterversion"}); err != nil { b.Fatal(err) } } diff --git a/internal/app/contract_test.go b/internal/app/contract_test.go index 7c8bc0754..8e6367837 100644 --- a/internal/app/contract_test.go +++ b/internal/app/contract_test.go @@ -24,7 +24,7 @@ import ( func TestRun_queriedLegacyAliasIsAnOrdinaryMissingFact(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"operatingsystem"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"operatingsystem"}); err != nil { t.Fatalf("Run(operatingsystem) err = %v, want nil", err) } if got := stdout.String(); got != "" { @@ -38,7 +38,7 @@ func TestRun_queriedLegacyAliasIsAnOrdinaryMissingFact(t *testing.T) { func TestRun_strictQueriedLegacyAliasIsMissing(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--strict", "operatingsystem"}) + err := runForTest(&stdout, &stderr, []string{"--strict", "operatingsystem"}) status, ok := err.(ExitStatus) if !ok { t.Fatalf("Run() err = %T %[1]v, want ExitStatus", err) @@ -58,7 +58,7 @@ func TestRun_queriedNilExternalFactRendersJSONNull(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--json", "nil_fact"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--json", "nil_fact"}); err != nil { t.Fatal(err) } var got map[string]any @@ -82,7 +82,7 @@ func TestRun_strictQueriedNilExternalFactIsMissing(t *testing.T) { } var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--strict", "--external-dir", dir, "--json", "nil_fact"}) + err := runForTest(&stdout, &stderr, []string{"--strict", "--external-dir", dir, "--json", "nil_fact"}) status, ok := err.(ExitStatus) if !ok { t.Fatalf("Run() err = %T %[1]v, want ExitStatus", err) @@ -110,7 +110,7 @@ func TestRun_invalidExternalArrayIndexQueriesPrintNothing(t *testing.T) { for _, query := range []string{"arr_fact.3", "arr_fact.abc", "arr_fact.-1"} { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, query}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, query}); err != nil { t.Fatalf("Run(%s) err = %v", query, err) } if got := stdout.String(); got != "" { @@ -129,7 +129,7 @@ func TestRun_partialDottedExternalFactQueryRequiresForceDotResolution(t *testing } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "a.b"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "a.b"}); err != nil { t.Fatal(err) } if got := stdout.String(); got != "" { @@ -138,7 +138,7 @@ func TestRun_partialDottedExternalFactQueryRequiresForceDotResolution(t *testing stdout.Reset() stderr.Reset() - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--force-dot-resolution", "a.b"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--force-dot-resolution", "a.b"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "{\n c => \"custom\"\n}\n"; got != want { @@ -155,7 +155,7 @@ func TestRun_partialDottedExternalFactQueryRequiresForceDotResolution(t *testing func TestRun_trailingOptionTokensAreTreatedAsQueries(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--json", "os.name", "--timing", "kernel"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--json", "os.name", "--timing", "kernel"}); err != nil { t.Fatal(err) } var got map[string]any @@ -188,7 +188,7 @@ func TestRun_configExternalDirLoadsExternalFacts(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "site_location"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "site_location"}); err != nil { t.Fatal(err) } if got, want := stdout.String(), "lab\n"; got != want { @@ -214,7 +214,7 @@ func TestRun_retiredCustomFactConfigKeysAreInert(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "site_role"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "site_role"}); err != nil { t.Fatalf("Run() err = %v, want retired config keys ignored", err) } if got := stdout.String(); got != "" { @@ -229,7 +229,7 @@ func TestRun_noExternalFactsSkipsEnvironmentExternalFacts(t *testing.T) { t.Setenv("FACTER_site_location", "lab") var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--no-external-facts", "site_location"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--no-external-facts", "site_location"}); err != nil { t.Fatal(err) } if got := stdout.String(); got != "" { @@ -249,7 +249,7 @@ func TestRun_configNoExternalFactsSkipsEnvironmentExternalFacts(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "site_location"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "site_location"}); err != nil { t.Fatal(err) } if got := stdout.String(); got != "" { @@ -271,7 +271,7 @@ func TestRun_facterlibHasNoEffectOnDiscovery(t *testing.T) { t.Setenv("FACTERLIB", dir) var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"site_role"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"site_role"}); err != nil { t.Fatal(err) } if got := stdout.String(); got != "" { @@ -285,7 +285,7 @@ func TestRun_facterlibHasNoEffectOnDiscovery(t *testing.T) { func TestRun_rejectsUnknownLongOption(t *testing.T) { var stdout, stderr bytes.Buffer - err := Run(&stdout, &stderr, []string{"--unknown-option", "os.name"}) + err := runForTest(&stdout, &stderr, []string{"--unknown-option", "os.name"}) if err == nil { t.Fatal("Run() err = nil, want unknown option error") } @@ -307,7 +307,7 @@ func TestRun_warnsAndSkipsRubyExternalFact(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "rb_fact"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "rb_fact"}); err != nil { t.Fatal(err) } if got := stdout.String(); got != "" { @@ -331,7 +331,7 @@ func TestRun_configBlocklistLegacyIsInert(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--config", configPath, "--json", "os.name", "kernel"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--config", configPath, "--json", "os.name", "kernel"}); err != nil { t.Fatal(err) } var got map[string]any diff --git a/internal/app/disable_test.go b/internal/app/disable_test.go index 7f926ce97..2932a4c92 100644 --- a/internal/app/disable_test.go +++ b/internal/app/disable_test.go @@ -15,7 +15,7 @@ func TestRun_disableOptionDropsNamedFacts(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha,beta", "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha,beta", "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -40,7 +40,7 @@ func TestRun_disableOptionRepeatableAcrossFlags(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha", "--disable", "beta", "--json"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha", "--disable", "beta", "--json"}); err != nil { t.Fatal(err) } var got map[string]any @@ -65,7 +65,7 @@ func TestRun_noBlockClearsDisableOption(t *testing.T) { } var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha", "--no-block", "--json", "alpha"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--external-dir", dir, "--disable", "alpha", "--no-block", "--json", "alpha"}); err != nil { t.Fatal(err) } var got map[string]any @@ -86,7 +86,7 @@ func TestRun_facterversionDisabledByEnvFallsThrough(t *testing.T) { t.Setenv("FACTS_DISABLE", "facterversion") var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"facterversion"}); err != nil { t.Fatal(err) } if stdout.Len() != 0 { @@ -100,7 +100,7 @@ func TestRun_facterversionDisabledByEnvFallsThrough(t *testing.T) { func TestRun_facterversionDisabledByFlagFallsThrough(t *testing.T) { var stdout, stderr bytes.Buffer - if err := Run(&stdout, &stderr, []string{"--disable", "facterversion", "facterversion"}); err != nil { + if err := runForTest(&stdout, &stderr, []string{"--disable", "facterversion", "facterversion"}); err != nil { t.Fatal(err) } if stdout.Len() != 0 { diff --git a/internal/engine/az_test.go b/internal/engine/az_test.go index af9a0b7c1..f2d6dabb1 100644 --- a/internal/engine/az_test.go +++ b/internal/engine/az_test.go @@ -20,6 +20,7 @@ func TestAzureFactsFetchMetadataAndCloudProvider(t *testing.T) { t.Cleanup(server.Close) facts := azureFacts(context.Background(), newAzureClient(server.URL, server.Client()), virtualization{Name: "hyperv", IsVirtual: true}) + assertDescriptorDeclaresFacts(t, "az_metadata", facts) got := factValues(facts) metadata, ok := got["az_metadata"].(map[string]any) diff --git a/internal/engine/cache.go b/internal/engine/cache.go index 98a590d8c..e72dd0cc8 100644 --- a/internal/engine/cache.go +++ b/internal/engine/cache.go @@ -17,14 +17,6 @@ import ( const cacheFormatVersion = 1 -var ( - cacheRemove = os.Remove - cacheWriteFile = writeCacheFile -) - -// DefaultCachePath returns the platform default directory for cached fact groups. -var DefaultCachePath = platformDefaultCachePath - // FactCache reads and writes Facter-compatible cached fact groups. type FactCache struct { dir string @@ -223,7 +215,7 @@ func (fc *FactCache) CacheFacts(facts []ResolvedFact) error { if !ok { continue } - if err := cacheWriteFile(path, encoded, 0o600); err != nil { + if err := writeCacheFile(path, encoded, 0o600); err != nil { if warnCacheWriteFailure(err, fc.logger()) { return nil } @@ -425,7 +417,11 @@ func windowsReservedCacheName(name string) bool { } func deleteCacheFile(path string, log *slog.Logger) { - if err := cacheRemove(path); err != nil { + warnCacheDeleteFailure(os.Remove(path), log) +} + +func warnCacheDeleteFailure(err error, log *slog.Logger) { + if err != nil { log.Warn(fmt.Sprintf("Could not delete cache: %v", err)) } } diff --git a/internal/engine/cache_test.go b/internal/engine/cache_test.go index 34abe4aa8..3349ed40d 100644 --- a/internal/engine/cache_test.go +++ b/internal/engine/cache_test.go @@ -296,12 +296,11 @@ func TestDiscover_disabledFactIsNotServedFromFreshCache(t *testing.T) { "cache_format_version": float64(1), "networking.hostname": "cached-host", }) - oldDefaultCachePath := DefaultCachePath - DefaultCachePath = func() string { return cacheDir } - t.Cleanup(func() { DefaultCachePath = oldDefaultCachePath }) + defaults := DiscoveryDefaults{CachePath: cacheDir} eng, err := NewEngine(EngineConfig{ UseCache: true, + Defaults: &defaults, ConfigLoaded: true, Config: Config{TTLs: []FactTTL{{Fact: "networking", TTL: "1 hour"}}}, ExtraDisabled: []string{"networking.hostname"}, @@ -321,12 +320,11 @@ func TestDiscover_disabledFactIsNotServedFromFreshCache(t *testing.T) { func TestDiscover_prunedSubfactIsNotCached(t *testing.T) { cacheDir := t.TempDir() - oldDefaultCachePath := DefaultCachePath - DefaultCachePath = func() string { return cacheDir } - t.Cleanup(func() { DefaultCachePath = oldDefaultCachePath }) + defaults := DiscoveryDefaults{CachePath: cacheDir} eng, err := NewEngine(EngineConfig{ UseCache: true, + Defaults: &defaults, ConfigLoaded: true, Config: Config{ Disabled: []string{"os.release"}, @@ -534,19 +532,12 @@ func TestWarnCacheWriteFailureIgnoresNonPermissionErrors(t *testing.T) { } } -func TestFactCache_cacheFactsWarnsWhenCacheFileCannotBeWrittenLikeRubyCacheManager(t *testing.T) { - dir := t.TempDir() - originalWriteFile := cacheWriteFile - cacheWriteFile = func(string, []byte, os.FileMode) error { return os.ErrPermission } - t.Cleanup(func() { - cacheWriteFile = originalWriteFile - }) +func TestWarnCacheWriteFailureWarnsForPermissionErrors(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - cache := NewFactCache(dir, []FactTTL{{Fact: "operating system", TTL: "1 hour"}}, nil, logger) - if err := cache.CacheFacts([]ResolvedFact{{Name: "os", Value: "Ubuntu", Type: "core"}}); err != nil { - t.Fatalf("CacheFacts() err = %v, want nil", err) + if !warnCacheWriteFailure(os.ErrPermission, logger) { + t.Fatal("warnCacheWriteFailure(os.ErrPermission) = false, want true") } if len(warnings) != 1 { @@ -658,31 +649,11 @@ func TestFactCache_cacheFactsLogsNoKeysForNilFreshCacheLikeRubyCacheManager(t *t } } -func TestFactCache_resolveFactsWarnsWhenCorruptCacheCannotBeDeletedLikeRubyCacheManager(t *testing.T) { - dir := t.TempDir() - cachePath := filepath.Join(dir, "ext_file.txt") - if err := os.WriteFile(cachePath, []byte("{"), 0o600); err != nil { - t.Fatal(err) - } - originalRemove := cacheRemove - cacheRemove = func(string) error { return os.ErrPermission } - t.Cleanup(func() { - cacheRemove = originalRemove - }) +func TestWarnCacheDeleteFailureWarnsForPermissionErrors(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - cache := NewFactCache(dir, []FactTTL{{Fact: "ext_file.txt", TTL: "1 hour"}}, nil, logger) - searched := []ResolvedFact{{Name: "my_external_fact", Type: "file", File: "/tmp/ext_file.txt"}} - - remaining, cached := cache.ResolveFacts(searched) - - if !reflect.DeepEqual(remaining, searched) { - t.Fatalf("remaining = %#v, want searched fact", remaining) - } - if len(cached) != 0 { - t.Fatalf("cached = %#v, want none", cached) - } + warnCacheDeleteFailure(os.ErrPermission, logger) if len(warnings) != 1 { t.Fatalf("warnings = %#v, want one warning", warnings) } diff --git a/internal/engine/config.go b/internal/engine/config.go index a9b6805ef..7c985940d 100644 --- a/internal/engine/config.go +++ b/internal/engine/config.go @@ -27,14 +27,6 @@ var ttlEntryPattern = regexp.MustCompile(`(?is)\{\s*(?:"([^"]+)"|'([^']+)'|([A-Z var factGroupPattern = regexp.MustCompile(`(?is)(?:^|[\s{,])(?:"([^"]+)"|'([^']+)'|([A-Za-z0-9_-]+))\s*[:=]\s*(\[(.*?)\]|"([^"]*)"|'([^']*)'|([A-Za-z0-9_.-]+))`) var configArrayValuePattern = regexp.MustCompile(`"([^"]*)"|'([^']*)'|([^,\]\s]+)`) -// NativeDefaultConfigPath returns the platform default facts-native -// facts.conf path, consulted before the facter-compatible default. -var NativeDefaultConfigPath = platformNativeDefaultConfigPath - -// DefaultConfigPath returns the platform default facter-compatible -// facter.conf path, read when no facts-native config file exists. -var DefaultConfigPath = platformDefaultConfigPath - // Config contains the supported Facter config values loaded from a config file. type Config struct { Disabled []string @@ -103,19 +95,20 @@ type FactTTL struct { TTL string } -// ParseConfig returns every supported value from a Facter config file. -// Diagnostics (an unreadable or invalid config file) are emitted to log; pass a -// discard logger to ignore them. -func ParseConfig(path string, log *slog.Logger) (Config, error) { - return readConfigOptionsFile(path, log) +// ParseConfig returns every supported value from a Facter config file, +// consulting defaults when path is empty. Diagnostics (an unreadable or +// invalid config file) are emitted to log; pass a discard logger to ignore +// them. +func ParseConfig(path string, log *slog.Logger, defaults DiscoveryDefaults) (Config, error) { + return readConfigOptionsFile(path, log, defaults) } -func readConfigOptionsFile(path string, log *slog.Logger) (Config, error) { +func readConfigOptionsFile(path string, log *slog.Logger, defaults DiscoveryDefaults) (Config, error) { if log == nil { log = slog.New(slog.DiscardHandler) } if path == "" { - path = readableDefaultConfigPath() + path = readableDefaultConfigPath(defaults) if path == "" { return Config{}, nil } @@ -216,8 +209,8 @@ func skipConfigComment(content string, start int) int { // readableDefaultConfigPath returns the first existing default config file: // the facts-native facts.conf wins over the facter-compatible facter.conf. // Both are parsed with identical semantics; an explicit path overrides both. -func readableDefaultConfigPath() string { - for _, path := range []string{NativeDefaultConfigPath(), DefaultConfigPath()} { +func readableDefaultConfigPath(defaults DiscoveryDefaults) string { + for _, path := range []string{defaults.NativeConfigPath, defaults.CompatibleConfigPath} { if path == "" { continue } diff --git a/internal/engine/config_benchmark_test.go b/internal/engine/config_benchmark_test.go index b56a6d551..3c63845ca 100644 --- a/internal/engine/config_benchmark_test.go +++ b/internal/engine/config_benchmark_test.go @@ -31,7 +31,7 @@ fact-groups : { b.ReportAllocs() b.ResetTimer() for b.Loop() { - if _, err := ParseConfig(path, discardLog()); err != nil { + if _, err := parseConfigForTest(path, discardLog()); err != nil { b.Fatal(err) } } diff --git a/internal/engine/config_test.go b/internal/engine/config_test.go index 819718d54..132f5c131 100644 --- a/internal/engine/config_test.go +++ b/internal/engine/config_test.go @@ -1,6 +1,7 @@ package engine import ( + "log/slog" "os" "path/filepath" "reflect" @@ -9,6 +10,10 @@ import ( "testing" ) +func parseConfigForTest(path string, log *slog.Logger) (Config, error) { + return ParseConfig(path, log, CurrentDiscoveryDefaults()) +} + func TestParseConfig_returnsAllConfiguredSections(t *testing.T) { path := filepath.Join(t.TempDir(), "facter.conf") content := `facts : { @@ -35,7 +40,7 @@ fact-groups : { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -152,7 +157,7 @@ cli : { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -179,7 +184,7 @@ cli : { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -202,7 +207,7 @@ func TestParseConfig_ignoresRetiredCustomFactKeys(t *testing.T) { var warnings []string logger := captureLogger(nil, &warnings, nil) - got, err := ParseConfig(path, logger) + got, err := parseConfigForTest(path, logger) if err != nil { t.Fatal(err) } @@ -376,9 +381,6 @@ func TestPlatformNativeDefaultConfigPath(t *testing.T) { // facts-native facts.conf is consulted first, the facter-compatible // facter.conf second, and the first existing file wins. func TestParseConfig_nativeDefaultConfigDiscovery(t *testing.T) { - dir := t.TempDir() - nativePath := filepath.Join(dir, "facts.conf") - compatPath := filepath.Join(dir, "facter.conf") nativeContent := []byte(`global : { external-dir : "/native/external" }`) compatContent := []byte(`global : { external-dir : "/compat/external" }`) @@ -396,12 +398,10 @@ func TestParseConfig_nativeDefaultConfigDiscovery(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := os.RemoveAll(nativePath); err != nil { - t.Fatal(err) - } - if err := os.RemoveAll(compatPath); err != nil { - t.Fatal(err) - } + t.Parallel() + dir := t.TempDir() + nativePath := filepath.Join(dir, "facts.conf") + compatPath := filepath.Join(dir, "facter.conf") if tt.native { if err := os.WriteFile(nativePath, nativeContent, 0o600); err != nil { t.Fatal(err) @@ -412,16 +412,10 @@ func TestParseConfig_nativeDefaultConfigDiscovery(t *testing.T) { t.Fatal(err) } } - oldNative := NativeDefaultConfigPath - oldCompat := DefaultConfigPath - NativeDefaultConfigPath = func() string { return nativePath } - DefaultConfigPath = func() string { return compatPath } - t.Cleanup(func() { - NativeDefaultConfigPath = oldNative - DefaultConfigPath = oldCompat + got, err := ParseConfig("", discardLog(), DiscoveryDefaults{ + NativeConfigPath: nativePath, + CompatibleConfigPath: compatPath, }) - - got, err := ParseConfig("", discardLog()) if err != nil { t.Fatal(err) } @@ -441,7 +435,7 @@ func TestParseConfig_acceptsBareDirectoryPaths(t *testing.T) { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -455,7 +449,7 @@ func TestParseConfig_warnsAndIgnoresUnreadableConfig(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - got, err := ParseConfig(path, logger) + got, err := parseConfigForTest(path, logger) if err != nil { t.Fatal(err) } @@ -478,7 +472,7 @@ func TestParseConfig_warnsAndIgnoresInvalidConfig(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - got, err := ParseConfig(path, logger) + got, err := parseConfigForTest(path, logger) if err != nil { t.Fatal(err) } @@ -501,7 +495,7 @@ func TestParseConfig_emptyReadableConfigReturnsEmptySections(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - options, err := ParseConfig(path, logger) + options, err := parseConfigForTest(path, logger) if err != nil { t.Fatal(err) } @@ -509,7 +503,7 @@ func TestParseConfig_emptyReadableConfigReturnsEmptySections(t *testing.T) { t.Fatalf("ParseConfig() = %#v, want empty options", options) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -542,7 +536,7 @@ cli : { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -566,7 +560,7 @@ func TestParseConfig_retiredShowLegacyKeyIsInert(t *testing.T) { warnings := []string{} logger := captureLogger(nil, &warnings, nil) - got, err := ParseConfig(path, logger) + got, err := parseConfigForTest(path, logger) if err != nil { t.Fatalf("ParseConfig() error = %v, want nil for retired show-legacy key", err) } @@ -587,7 +581,7 @@ func TestParseConfig_readsConfiguredSequentialLikeRubyOptionStore(t *testing.T) t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -611,7 +605,7 @@ cli : { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -630,7 +624,7 @@ func TestParseConfig_nativeDisableKeyPopulatesDisabledSet(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -650,7 +644,7 @@ func TestParseConfig_nativeDisableKeySupersedesBlocklist(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -669,7 +663,7 @@ func TestParseConfig_acceptsBareEntries(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -696,7 +690,7 @@ facts : { t.Fatal(err) } - options, err := ParseConfig(path, discardLog()) + options, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -707,7 +701,7 @@ facts : { t.Fatal("NoExternalFacts = true, want commented true value ignored") } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -730,7 +724,7 @@ func TestParseConfig_returnsConfiguredFactTTLs(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -761,7 +755,7 @@ facts : { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -784,7 +778,7 @@ func TestParseConfig_acceptsBareFactNamesAndValues(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1165,7 +1159,7 @@ func TestParseConfig_returnsConfiguredFactGroups(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1188,7 +1182,7 @@ func TestParseConfig_acceptsQuotedGroupNamesWithSpaces(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1208,7 +1202,7 @@ func TestParseConfig_acceptsBareFactNames(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1228,7 +1222,7 @@ func TestParseConfig_acceptsScalarFactGroupValue(t *testing.T) { t.Fatal(err) } - config, err := ParseConfig(path, discardLog()) + config, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1257,7 +1251,7 @@ func TestConfigParser_pinnedSubsetBoundary(t *testing.T) { path := writeConfig(t, `global = { external-dir = [ "/json/external" ] }`) - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1270,7 +1264,7 @@ func TestConfigParser_pinnedSubsetBoundary(t *testing.T) { path := writeConfig(t, `cli : { log-level : 'trace', }`) - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1284,7 +1278,7 @@ func TestConfigParser_pinnedSubsetBoundary(t *testing.T) { global : { external-dir : [ "${base-dir}" ], }`) - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1307,7 +1301,7 @@ global : { t.Fatal(err) } - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1321,7 +1315,7 @@ global : { var warnings []string logger := captureLogger(nil, &warnings, nil) - got, err := ParseConfig(path, logger) + got, err := parseConfigForTest(path, logger) if err != nil { t.Fatal(err) } @@ -1338,7 +1332,7 @@ global : { external-dir : [ "/kept" ], # comment with , and : inside no-external-facts : true // trailing comment }`) - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } @@ -1355,7 +1349,7 @@ global : { external-dir : [ "/kept" ], } # no newline`) - got, err := ParseConfig(path, discardLog()) + got, err := parseConfigForTest(path, discardLog()) if err != nil { t.Fatal(err) } diff --git a/internal/engine/core.go b/internal/engine/core.go index 735ad8b9c..f575c93d6 100644 --- a/internal/engine/core.go +++ b/internal/engine/core.go @@ -15,8 +15,9 @@ import ( var Version = "dev" // ponytail: var, not const, so the build pipeline can inject the git tag // CoreFacts returns the small cross-platform fact set used by the initial Go -// CLI. disabled is the resolution-gating set: single-output categories whose -// top-level fact name it contains skip resolution entirely (ADR-0015). The +// CLI. disabled is the resolution-gating set: a gateable category skips +// resolution when it contains every top-level root the category can emit +// (ADR-0015). The // result is memoized per Session, keyed by a fingerprint of the disabled set, so // a repeat call with the same set reuses the build while a call with a different // set rebuilds (the gating depends on disabled, so a stale memo would misgate). @@ -58,9 +59,13 @@ func disabledFingerprint(disabled map[string]bool) string { // historical assembly order for reviewability. func buildCoreFacts(s *Session, disabled map[string]bool) []ResolvedFact { build := newCoreFactBuild(s) + return resolveCoreFactDescriptors(build, disabled, coreFactDescriptors) +} + +func resolveCoreFactDescriptors(build *coreFactBuild, disabled map[string]bool, descriptors []coreFactDescriptor) []ResolvedFact { facts := make([]ResolvedFact, 0) - for _, descriptor := range coreFactDescriptors { - if descriptor.class == coreFactStandalone && disabled[descriptor.root] { + for _, descriptor := range descriptors { + if !descriptor.shouldRun(disabled) { continue } facts = append(facts, descriptor.assemble(build)...) diff --git a/internal/engine/core_gating_test.go b/internal/engine/core_gating_test.go index 1b212a9aa..beae75c1c 100644 --- a/internal/engine/core_gating_test.go +++ b/internal/engine/core_gating_test.go @@ -22,41 +22,17 @@ func hasRoot(facts []ResolvedFact, root string) bool { return rootNames(facts)[root] } -func TestBuildCoreFacts_resolutionGatesSingleOutputCategories(t *testing.T) { - // buildCoreFacts performs no filtering, so a fact root that is absent here - // can only be absent because its resolver was skipped — exactly the - // resolution-gating contract. - baseline := buildCoreFacts(NewSession(), nil) - for _, fact := range standaloneCoreFactRoots() { - if !hasRoot(baseline, fact) { - // Not every category resolves on this host (e.g. augeas/xen). Only - // assert gating for the ones that do produce output by default. - continue - } - // Each gated build also disables packages: its probe is the most - // expensive (plutil over every .app; PowerShell spawns on Windows - // runners), and gates are independent, so disabling it alongside does - // not affect the fact under test — it just keeps ~9 full package - // probes out of every CI run. The packages iteration itself still - // asserts its own gating. - gated := buildCoreFacts(NewSession(), map[string]bool{fact: true, "packages": true}) - if hasRoot(gated, fact) { - t.Fatalf("buildCoreFacts(disabled=%q) still emitted %q root; resolver was not gated", fact, fact) - } - } -} - -func TestBuildCoreFacts_multiOutputCategoryStaysEager(t *testing.T) { - // Disabling the multi-output root `os` must NOT gate osCoreFacts: it stays - // eager (resolve-then-prune) so its sibling kernel/filesystems outputs are - // not collateral. buildCoreFacts still emits os.* because it never prunes. +func TestBuildCoreFacts_multiOutputResolverRunsForKeptSibling(t *testing.T) { + // Disabling only the multi-output root `os` must not gate osCoreFacts because + // its kernel/filesystems siblings remain enabled. buildCoreFacts still emits + // os.* because pruning happens later in discovery. facts := buildCoreFacts(NewSession(), map[string]bool{"os": true}) if !hasRoot(facts, "os") { - t.Fatal("buildCoreFacts(disabled=os) dropped os.*; multi-output category must stay eager") + t.Fatal("buildCoreFacts(disabled=os) dropped os.*; resolver must run for a kept sibling") } } -func TestBuildCoreFacts_multiOutputSubfactStaysEagerThenPruned(t *testing.T) { +func TestBuildCoreFacts_multiOutputSubfactDisableRunsResolverThenPrunes(t *testing.T) { disabled := map[string]bool{"os.release": true} facts := buildCoreFacts(NewSession(), disabled) // The resolver still runs (os.name present) and still emits os.release; @@ -135,6 +111,226 @@ func hostReadFileMatching(h *fakeHostOS, substr string) bool { return false } +func hostReadFileCount(h *fakeHostOS, substr string) int { + count := 0 + for _, path := range h.readFileCalls { + if strings.Contains(path, substr) { + count++ + } + } + return count +} + +func hostReadDirCount(h *fakeHostOS, substr string) int { + count := 0 + for _, path := range h.readDirCalls { + if strings.Contains(path, substr) { + count++ + } + } + return count +} + +func hostRunCount(h *fakeHostOS, name string) int { + count := 0 + for _, call := range h.runCalls { + if call.name == name { + count++ + } + } + return count +} + +func TestResolveCoreFactDescriptors_probeCountsEveryCatalogRow(t *testing.T) { + for i, descriptor := range coreFactDescriptors { + t.Run(descriptor.root, func(t *testing.T) { + calls := 0 + observed := descriptor + observed.assemble = func(*coreFactBuild) []ResolvedFact { + calls++ + return nil + } + + allDisabled := make(map[string]bool, len(descriptor.emittedRoots)) + for _, root := range descriptor.emittedRoots { + allDisabled[root] = true + } + resolveCoreFactDescriptors(&coreFactBuild{}, allDisabled, []coreFactDescriptor{observed}) + want := 0 + if descriptor.policy == coreFactAlwaysEager { + want = 1 + } + if calls != want { + t.Fatalf("catalog row %d all-disabled resolver calls = %d, want %d", i, calls, want) + } + + if descriptor.policy != coreFactGateable { + return + } + calls = 0 + delete(allDisabled, descriptor.emittedRoots[0]) + resolveCoreFactDescriptors(&coreFactBuild{}, allDisabled, []coreFactDescriptor{observed}) + if calls != 1 { + t.Fatalf("catalog row %d one-kept resolver calls = %d, want 1", i, calls) + } + }) + } +} + +func TestBuildCoreFacts_DMIProbeRunsOnlyForKeptDMIOrGCE(t *testing.T) { + tests := []struct { + name string + disabled map[string]bool + want int + }{ + { + name: "both consumers disabled", + disabled: map[string]bool{"cloud": true, "dmi": true, "gce": true, "packages": true}, + want: 0, + }, + { + name: "dmi kept", + disabled: map[string]bool{"cloud": true, "gce": true, "packages": true}, + want: 1, + }, + { + name: "gce kept", + disabled: map[string]bool{"dmi": true, "packages": true}, + want: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + host := &fakeHostOS{ + platform: "linux", + emptyRunDefault: true, + files: map[string][]byte{ + "/sys/class/dmi/id/board_vendor": []byte("Example"), + }, + } + buildCoreFacts(gatingProbeSession(host), tt.disabled) + if got := hostReadFileCount(host, "/sys/class/dmi/id/board_vendor"); got != tt.want { + t.Fatalf("DMI probe count = %d, want %d", got, tt.want) + } + }) + } +} + +func TestBuildCoreFacts_multiOutputProbesRunOnlyForKeptRoots(t *testing.T) { + tests := []struct { + name string + roots []string + probes func(*fakeHostOS) int + }{ + { + name: "operating system", + roots: []string{"filesystems", "kernel", "os", "system_profiler"}, + probes: func(host *fakeHostOS) int { + return hostReadFileCount(host, "/etc/os-release") + }, + }, + { + name: "disks", + roots: []string{"disks", "mountpoints", "partitions", "zfs", "zpool"}, + probes: func(host *fakeHostOS) int { + return hostReadDirCount(host, "/sys/block") + }, + }, + { + name: "uptime", + roots: []string{"load_averages", "system_uptime"}, + probes: func(host *fakeHostOS) int { + return hostReadFileCount(host, "/proc/uptime") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allDisabled := map[string]bool{"packages": true} + for _, root := range tt.roots { + allDisabled[root] = true + } + + host := &fakeHostOS{platform: "linux", emptyRunDefault: true} + buildCoreFacts(gatingProbeSession(host), allDisabled) + if got := tt.probes(host); got != 0 { + t.Fatalf("all emitted roots disabled: probe count = %d, want 0", got) + } + + oneKept := make(map[string]bool, len(allDisabled)-1) + for root := range allDisabled { + oneKept[root] = true + } + delete(oneKept, tt.roots[0]) + host = &fakeHostOS{platform: "linux", emptyRunDefault: true} + buildCoreFacts(gatingProbeSession(host), oneKept) + if got := tt.probes(host); got == 0 { + t.Fatal("one emitted root kept: probe count = 0, want resolver to run") + } + }) + } +} + +func TestBuildCoreFacts_identityProbeRunsOnlyForKeptIdentityOrSSH(t *testing.T) { + tests := []struct { + name string + disabled map[string]bool + want int + }{ + {name: "both consumers disabled", disabled: map[string]bool{"identity": true, "packages": true, "ssh": true}, want: 0}, + {name: "identity kept", disabled: map[string]bool{"packages": true, "ssh": true}, want: 1}, + {name: "ssh kept", disabled: map[string]bool{"identity": true, "packages": true}, want: 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + host := &fakeHostOS{platform: "windows", emptyRunDefault: true} + buildCoreFacts(gatingProbeSession(host), tt.disabled) + if got := hostRunCount(host, "whoami"); got != tt.want { + t.Fatalf("identity probe count = %d, want %d", got, tt.want) + } + }) + } +} + +func TestBuildCoreFacts_cloudProvidersRunForSharedCloudRoot(t *testing.T) { + tests := []struct { + name string + providerRoot string + disabled map[string]bool + }{ + {name: "azure", providerRoot: "az_metadata", disabled: map[string]bool{"az_metadata": true}}, + {name: "ec2", providerRoot: "ec2_metadata", disabled: map[string]bool{"ec2_metadata": true, "ec2_userdata": true}}, + {name: "gce", providerRoot: "gce", disabled: map[string]bool{"gce": true}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keptCloud := make(map[string]bool, len(tt.disabled)+1) + for root := range tt.disabled { + keptCloud[root] = true + } + keptCloud["packages"] = true + facts := buildCoreFacts(gatingProbeSession(&fakeHostOS{platform: "linux", emptyRunDefault: true}), keptCloud) + if !hasRoot(facts, tt.providerRoot) { + t.Fatalf("cloud kept: resolver did not emit %q", tt.providerRoot) + } + + allDisabled := make(map[string]bool, len(keptCloud)+1) + for root := range keptCloud { + allDisabled[root] = true + } + allDisabled["cloud"] = true + facts = buildCoreFacts(gatingProbeSession(&fakeHostOS{platform: "linux", emptyRunDefault: true}), allDisabled) + if hasRoot(facts, tt.providerRoot) { + t.Fatalf("all emitted roots disabled: resolver still emitted %q", tt.providerRoot) + } + }) + } +} + // TestBuildCoreFacts_resolutionGatingSkipsProbeWork proves the gate skips real // work, not just output: each gated category's distinctive host probe must run // when the category is enabled and must NOT run when it is disabled. A display @@ -178,6 +374,10 @@ func TestBuildCoreFacts_resolutionGatingSkipsProbeWork(t *testing.T) { category: "fips_enabled", probed: func(h *fakeHostOS) bool { return hostReadFileMatching(h, "/proc/sys/crypto/fips_enabled") }, }, + { + category: "packages", + probed: func(h *fakeHostOS) bool { return hostReadFileMatching(h, "/var/lib/dpkg/status") }, + }, // timezone is intentionally omitted: on every non-Windows host it derives // the zone from Go's time.Now().Format("MST") with no host probe at all, // so no recorded seam exists to observe. Its gating is covered by the diff --git a/internal/engine/defaults.go b/internal/engine/defaults.go new file mode 100644 index 000000000..13fc03563 --- /dev/null +++ b/internal/engine/defaults.go @@ -0,0 +1,35 @@ +package engine + +import "slices" + +// DiscoveryDefaults captures ambient paths and ordered external-fact +// directories for one discovery or CLI invocation. +type DiscoveryDefaults struct { + NativeConfigPath string + CompatibleConfigPath string + CachePath string + ExternalFactDirs []string +} + +// CurrentDiscoveryDefaults returns the defaults for the current process. +func CurrentDiscoveryDefaults() DiscoveryDefaults { + return DiscoveryDefaults{ + NativeConfigPath: platformNativeDefaultConfigPath(), + CompatibleConfigPath: platformDefaultConfigPath(), + CachePath: platformDefaultCachePath(), + ExternalFactDirs: CurrentDefaultExternalFactDirs(), + } +} + +func (d DiscoveryDefaults) clone() DiscoveryDefaults { + d.ExternalFactDirs = slices.Clone(d.ExternalFactDirs) + return d +} + +func cloneDiscoveryDefaults(defaults *DiscoveryDefaults) *DiscoveryDefaults { + if defaults == nil { + return nil + } + cloned := defaults.clone() + return &cloned +} diff --git a/internal/engine/defaults_test.go b/internal/engine/defaults_test.go new file mode 100644 index 000000000..73ada03d7 --- /dev/null +++ b/internal/engine/defaults_test.go @@ -0,0 +1,186 @@ +package engine + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "reflect" + "runtime" + "sync" + "testing" +) + +func TestEngineDiscover_isolatesInvocationDefaults(t *testing.T) { + t.Parallel() + + type fixture struct { + name string + factValue string + configPath string + cachePath string + external []string + engine *Engine + } + newFixture := func(name string) fixture { + t.Helper() + firstDir := t.TempDir() + secondDir := t.TempDir() + if err := os.WriteFile(filepath.Join(firstDir, "first.json"), []byte(`{"order_`+name+`_first":true}`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(secondDir, "second.json"), []byte(`{"order_`+name+`_second":true}`), 0o600); err != nil { + t.Fatal(err) + } + configPath := filepath.Join(t.TempDir(), "facts.conf") + config := `facts: { ttls: [ { "cache_probe": "30 days" } ] }` + if err := os.WriteFile(configPath, []byte(config), 0o600); err != nil { + t.Fatal(err) + } + cachePath := t.TempDir() + defaults := DiscoveryDefaults{ + NativeConfigPath: configPath, + CachePath: cachePath, + ExternalFactDirs: []string{firstDir, secondDir}, + } + eng, err := NewEngine(EngineConfig{ + SystemDefaults: true, + UseCache: true, + Defaults: &defaults, + Facts: []ProgrammaticFact{{ + Name: "cache_probe", + Resolve: func(context.Context) (any, error) { + return name, nil + }, + }}, + }) + if err != nil { + t.Fatal(err) + } + defaults.ExternalFactDirs[0] = t.TempDir() + return fixture{name: name, factValue: name, configPath: configPath, cachePath: cachePath, external: []string{firstDir, secondDir}, engine: eng} + } + + fixtures := []fixture{newFixture("alpha"), newFixture("beta")} + var wg sync.WaitGroup + for i := range fixtures { + f := &fixtures[i] + wg.Go(func() { + plan, failures := f.engine.planDiscovery(NewSession(), nil) + if len(failures) != 0 { + t.Errorf("%s plan failures = %v", f.name, failures) + return + } + if plan.cachePath != f.cachePath { + t.Errorf("%s cache path = %q, want %q", f.name, plan.cachePath, f.cachePath) + } + if !reflect.DeepEqual(plan.externalDirs, f.external) { + t.Errorf("%s external dirs = %#v, want ordered %#v", f.name, plan.externalDirs, f.external) + } + snapshot, err := f.engine.Discover(t.Context(), "cache_probe") + if err != nil { + t.Errorf("%s Discover() error = %v", f.name, err) + return + } + if got, err := snapshot.Value("cache_probe"); err != nil || got != f.factValue { + t.Errorf("%s cache_probe = %#v, %v, want %q", f.name, got, err, f.factValue) + } + }) + } + wg.Wait() + + for _, f := range fixtures { + data, err := os.ReadFile(filepath.Join(f.cachePath, "cache_probe")) + if err != nil { + t.Fatalf("read %s cache: %v", f.name, err) + } + var cached map[string]any + if err := json.Unmarshal(data, &cached); err != nil { + t.Fatalf("decode %s cache: %v", f.name, err) + } + if got := cached["cache_probe"]; got != f.factValue { + t.Fatalf("%s cached value = %#v, want %q", f.name, got, f.factValue) + } + } +} + +func TestPlanDiscovery_rereadsInvocationConfig(t *testing.T) { + t.Parallel() + + configPath := filepath.Join(t.TempDir(), "facts.conf") + writeConfig := func(disabled string) { + t.Helper() + if err := os.WriteFile(configPath, []byte(`global: { disable: ["`+disabled+`"] }`), 0o600); err != nil { + t.Fatal(err) + } + } + writeConfig("first") + defaults := DiscoveryDefaults{NativeConfigPath: configPath} + eng, err := NewEngine(EngineConfig{SystemDefaults: true, Defaults: &defaults}) + if err != nil { + t.Fatal(err) + } + + first, failures := eng.planDiscovery(NewSession(), nil) + if len(failures) != 0 { + t.Fatalf("first plan failures = %v", failures) + } + writeConfig("second") + second, failures := eng.planDiscovery(NewSession(), nil) + if len(failures) != 0 { + t.Fatalf("second plan failures = %v", failures) + } + if !first.disabledFacts["first"] || first.disabledFacts["second"] { + t.Fatalf("first disabled facts = %#v, want only first", first.disabledFacts) + } + if !second.disabledFacts["second"] || second.disabledFacts["first"] { + t.Fatalf("second disabled facts = %#v, want only second", second.disabledFacts) + } +} + +func TestEngineDiscover_nilDefaultsRederivesAmbientDefaults(t *testing.T) { + if runtime.GOOS != "windows" && os.Geteuid() == 0 { + t.Skip("root defaults do not depend on the process environment") + } + + roots := []string{t.TempDir(), t.TempDir()} + wants := []string{"first", "second"} + for i, root := range roots { + dir := filepath.Join(root, ".facts", "facts.d") + if runtime.GOOS == "windows" { + dir = filepath.Join(root, "facts", "facts.d") + } + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + content := []byte("ambient_defaults_probe: " + wants[i] + "\n") + if err := os.WriteFile(filepath.Join(dir, "ambient.yaml"), content, 0o600); err != nil { + t.Fatal(err) + } + } + + ambientName := "HOME" + if runtime.GOOS == "windows" { + ambientName = "ProgramData" + } + t.Setenv(ambientName, roots[0]) + eng, err := NewEngine(EngineConfig{SystemDefaults: true, ConfigLoaded: true}) + if err != nil { + t.Fatal(err) + } + + for i, root := range roots { + t.Setenv(ambientName, root) + snapshot, err := eng.Discover(t.Context(), "ambient_defaults_probe") + if err != nil { + t.Fatalf("Discover() with %s=%q: %v", ambientName, root, err) + } + got, err := snapshot.Value("ambient_defaults_probe") + if err != nil { + t.Fatalf("Value(ambient_defaults_probe) with %s=%q: %v", ambientName, root, err) + } + if got != wants[i] { + t.Fatalf("ambient_defaults_probe with %s=%q = %#v, want %q", ambientName, root, got, wants[i]) + } + } +} diff --git a/internal/engine/descriptors.go b/internal/engine/descriptors.go index d672874cc..54e20db1b 100644 --- a/internal/engine/descriptors.go +++ b/internal/engine/descriptors.go @@ -2,24 +2,20 @@ package engine import "slices" -type coreFactGatingClass string +type coreFactSchedulingPolicy string const ( - coreFactStandalone coreFactGatingClass = "standalone" - coreFactMultiOutput coreFactGatingClass = "multiOutput" - coreFactSharedProbe coreFactGatingClass = "sharedProbe" - coreFactInlineEager coreFactGatingClass = "inlineEager" + coreFactGateable coreFactSchedulingPolicy = "gateable" + coreFactAlwaysEager coreFactSchedulingPolicy = "alwaysEager" ) type coreFactDescriptor struct { - root string - group string - groupOrder int - class coreFactGatingClass - assemble func(*coreFactBuild) []ResolvedFact - emittedRoots []string - probeConsumers []string - emitsUnder string + root string + group string + groupOrder int + policy coreFactSchedulingPolicy + assemble func(*coreFactBuild) []ResolvedFact + emittedRoots []string } type coreFactBuild struct { @@ -28,19 +24,18 @@ type coreFactBuild struct { virtualization virtualization virtualFact any isVirtualFact any - dmi map[string]any } var coreFactDescriptors = []coreFactDescriptor{ { root: "facterversion", - class: coreFactInlineEager, + policy: coreFactAlwaysEager, assemble: func(*coreFactBuild) []ResolvedFact { return []ResolvedFact{{Name: "facterversion", Value: Version}} }, emittedRoots: []string{"facterversion"}, }, { - root: "is_virtual", - class: coreFactInlineEager, + root: "is_virtual", + policy: coreFactAlwaysEager, assemble: func(b *coreFactBuild) []ResolvedFact { return []ResolvedFact{{Name: "is_virtual", Value: b.isVirtualFact}} }, @@ -50,26 +45,25 @@ var coreFactDescriptors = []coreFactDescriptor{ root: "path", group: "path", groupOrder: 5, - class: coreFactInlineEager, + policy: coreFactAlwaysEager, assemble: func(b *coreFactBuild) []ResolvedFact { return []ResolvedFact{{Name: "path", Value: currentPathEntries(b.goos, b.s.getenv)}} }, emittedRoots: []string{"path"}, }, { - root: "virtual", - class: coreFactInlineEager, + root: "virtual", + policy: coreFactAlwaysEager, assemble: func(b *coreFactBuild) []ResolvedFact { return []ResolvedFact{{Name: "virtual", Value: b.virtualFact}} }, - emittedRoots: []string{"virtual"}, - probeConsumers: []string{"azure", "ec2", "gce", "hypervisors"}, + emittedRoots: []string{"virtual"}, }, { root: "networking", group: "networking", groupOrder: 2, - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return networkingCoreFacts(b.s) }, emittedRoots: []string{"networking"}, }, @@ -77,7 +71,7 @@ var coreFactDescriptors = []coreFactDescriptor{ root: "processors", group: "processor", groupOrder: 6, - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return processorsCoreFacts(b.s) }, emittedRoots: []string{"processors"}, }, @@ -85,7 +79,7 @@ var coreFactDescriptors = []coreFactDescriptor{ root: "memory", group: "memory", groupOrder: 1, - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return memoryCoreFacts(b.s) }, emittedRoots: []string{"memory"}, }, @@ -93,71 +87,67 @@ var coreFactDescriptors = []coreFactDescriptor{ root: "os", group: "operating system", groupOrder: 3, - class: coreFactMultiOutput, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return osCoreFacts(b.s) }, emittedRoots: []string{"filesystems", "kernel", "os", "system_profiler"}, }, { - root: "dmi", - class: coreFactSharedProbe, - assemble: func(b *coreFactBuild) []ResolvedFact { return dmiCoreFacts(b.s) }, - emittedRoots: []string{"dmi"}, - probeConsumers: []string{"gce"}, + root: "dmi", + policy: coreFactGateable, + assemble: func(b *coreFactBuild) []ResolvedFact { return dmiCoreFacts(b.s) }, + emittedRoots: []string{"dmi"}, }, { root: "disks", - class: coreFactMultiOutput, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return disksCoreFacts(b.s) }, emittedRoots: []string{"disks", "mountpoints", "partitions", "zfs", "zpool"}, }, { - root: "ssh", - class: coreFactStandalone, - assemble: func(b *coreFactBuild) []ResolvedFact { return sshCoreFacts(b.s) }, - emittedRoots: []string{"ssh"}, - probeConsumers: []string{"identity"}, + root: "ssh", + policy: coreFactGateable, + assemble: func(b *coreFactBuild) []ResolvedFact { return sshCoreFacts(b.s) }, + emittedRoots: []string{"ssh"}, }, { - root: "identity", - class: coreFactSharedProbe, - assemble: func(b *coreFactBuild) []ResolvedFact { return identityCoreFacts(b.s) }, - emittedRoots: []string{"identity"}, - probeConsumers: []string{"ssh"}, + root: "identity", + policy: coreFactGateable, + assemble: func(b *coreFactBuild) []ResolvedFact { return identityCoreFacts(b.s) }, + emittedRoots: []string{"identity"}, }, { root: "system_uptime", - class: coreFactMultiOutput, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return uptimeCoreFacts(b.s) }, emittedRoots: []string{"load_averages", "system_uptime"}, }, { root: "selinux", - class: coreFactInlineEager, + policy: coreFactAlwaysEager, assemble: func(b *coreFactBuild) []ResolvedFact { return selinuxCoreFacts(b.s) }, emittedRoots: []string{"os"}, - emitsUnder: "os.selinux", }, { root: "fips_enabled", - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return fipsCoreFacts(b.s) }, emittedRoots: []string{"fips_enabled"}, }, { root: "timezone", - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return timezoneCoreFacts(b.s) }, emittedRoots: []string{"timezone"}, }, { root: "augeas", - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return augeasCoreFacts(b.s) }, emittedRoots: []string{"augeas"}, }, { root: "xen", - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return xenCoreFacts(b.s) }, emittedRoots: []string{"xen"}, }, @@ -165,53 +155,60 @@ var coreFactDescriptors = []coreFactDescriptor{ root: "packages", group: "packages", groupOrder: 4, - class: coreFactStandalone, + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return packagesCoreFacts(b.s) }, emittedRoots: []string{"packages"}, }, { - root: "hypervisors", - class: coreFactSharedProbe, - assemble: func(b *coreFactBuild) []ResolvedFact { return currentLinuxHypervisorFacts(b.s) }, - emittedRoots: []string{"hypervisors"}, - probeConsumers: []string{"virtual"}, + root: "hypervisors", + policy: coreFactGateable, + assemble: func(b *coreFactBuild) []ResolvedFact { return currentLinuxHypervisorFacts(b.s) }, + emittedRoots: []string{"hypervisors"}, }, { - root: "hypervisors", - class: coreFactSharedProbe, - assemble: func(b *coreFactBuild) []ResolvedFact { return currentWindowsHypervisorFacts(b.s) }, - emittedRoots: []string{"hypervisors"}, - probeConsumers: []string{"virtual"}, + root: "hypervisors", + policy: coreFactGateable, + assemble: func(b *coreFactBuild) []ResolvedFact { return currentWindowsHypervisorFacts(b.s) }, + emittedRoots: []string{"hypervisors"}, }, { - root: "az_metadata", - class: coreFactSharedProbe, + root: "az_metadata", + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return azureFacts(b.s.Context(), newAzureClient(azureMetadataBaseURL, nil), b.virtualization) }, - emittedRoots: []string{"az_metadata", "cloud"}, - probeConsumers: []string{"virtual"}, + emittedRoots: []string{"az_metadata", "cloud"}, }, { - root: "ec2_metadata", - class: coreFactSharedProbe, + root: "ec2_metadata", + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { return ec2Facts(b.s, newEC2Client(ec2MetadataBaseURL, nil), b.virtualization) }, - emittedRoots: []string{"cloud", "ec2_metadata", "ec2_userdata"}, - probeConsumers: []string{"virtual"}, + emittedRoots: []string{"cloud", "ec2_metadata", "ec2_userdata"}, }, { - root: "gce", - class: coreFactSharedProbe, + root: "gce", + policy: coreFactGateable, assemble: func(b *coreFactBuild) []ResolvedFact { - return platformGCEFacts(b.s.Context(), b.goos, b.virtualization, dmiBIOSVendor(b.dmi), newGCEClient(gceMetadataBaseURL, nil)) + return platformGCEFacts(b.s.Context(), b.goos, b.virtualization, dmiBIOSVendor(b.s.cachedDMI()), newGCEClient(gceMetadataBaseURL, nil)) }, - emittedRoots: []string{"cloud", "gce"}, - probeConsumers: []string{"dmi", "virtual"}, + emittedRoots: []string{"cloud", "gce"}, }, } +func (d coreFactDescriptor) shouldRun(disabled map[string]bool) bool { + if d.policy == coreFactAlwaysEager { + return true + } + for _, root := range d.emittedRoots { + if !disabled[root] { + return true + } + } + return false +} + func newCoreFactBuild(s *Session) *coreFactBuild { goos := s.goos() virtualization := detectVirtualization(s) @@ -222,18 +219,7 @@ func newCoreFactBuild(s *Session) *coreFactBuild { virtualization: virtualization, virtualFact: virtualFact, isVirtualFact: isVirtualFact, - dmi: s.cachedDMI(), - } -} - -func standaloneCoreFactRoots() []string { - roots := make([]string, 0) - for _, descriptor := range coreFactDescriptors { - if descriptor.class == coreFactStandalone { - roots = append(roots, descriptor.root) - } } - return roots } func builtinFactGroupsFromDescriptors() []FactGroup { diff --git a/internal/engine/descriptors_test.go b/internal/engine/descriptors_test.go index 1f1934410..6200637ee 100644 --- a/internal/engine/descriptors_test.go +++ b/internal/engine/descriptors_test.go @@ -1,12 +1,55 @@ package engine import ( + "fmt" "reflect" "slices" "strings" "testing" ) +func TestCoreFactDescriptors_exactRows(t *testing.T) { + rows := make([]string, 0, len(coreFactDescriptors)) + for _, descriptor := range coreFactDescriptors { + rows = append(rows, fmt.Sprintf("%s|%s|%d|%s|%s", + descriptor.root, + descriptor.group, + descriptor.groupOrder, + descriptor.policy, + strings.Join(descriptor.emittedRoots, ","), + )) + } + got := strings.Join(rows, "\n") + want := strings.TrimSpace(` +facterversion||0|alwaysEager|facterversion +is_virtual||0|alwaysEager|is_virtual +path|path|5|alwaysEager|path +virtual||0|alwaysEager|virtual +networking|networking|2|gateable|networking +processors|processor|6|gateable|processors +memory|memory|1|gateable|memory +os|operating system|3|gateable|filesystems,kernel,os,system_profiler +dmi||0|gateable|dmi +disks||0|gateable|disks,mountpoints,partitions,zfs,zpool +ssh||0|gateable|ssh +identity||0|gateable|identity +system_uptime||0|gateable|load_averages,system_uptime +selinux||0|alwaysEager|os +fips_enabled||0|gateable|fips_enabled +timezone||0|gateable|timezone +augeas||0|gateable|augeas +xen||0|gateable|xen +packages|packages|4|gateable|packages +hypervisors||0|gateable|hypervisors +hypervisors||0|gateable|hypervisors +az_metadata||0|gateable|az_metadata,cloud +ec2_metadata||0|gateable|cloud,ec2_metadata,ec2_userdata +gce||0|gateable|cloud,gce`) + if got != want { + t.Fatalf("descriptor rows:\n%s\n\nwant:\n%s", got, want) + } +} + func TestCoreFactDescriptors_projectBuiltinGroups(t *testing.T) { want := []FactGroup{ {Name: "memory", Facts: []string{"memory"}}, @@ -42,7 +85,7 @@ func TestCoreFactDescriptors_coverEmittedRoots(t *testing.T) { if len(descriptor.emittedRoots) == 0 { t.Fatalf("descriptor %q has no emitted roots", descriptor.root) } - if descriptor.emitsUnder == "" && !slices.Contains(descriptor.emittedRoots, descriptor.root) { + if descriptor.root != "selinux" && !slices.Contains(descriptor.emittedRoots, descriptor.root) { t.Fatalf("descriptor %q emitted roots = %#v, want root included", descriptor.root, descriptor.emittedRoots) } for _, root := range descriptor.emittedRoots { @@ -58,27 +101,40 @@ func TestCoreFactDescriptors_coverEmittedRoots(t *testing.T) { } } -func TestCoreFactDescriptors_standaloneGatesAreTableDriven(t *testing.T) { - roots := standaloneCoreFactRoots() - if len(roots) == 0 { - t.Fatal("standaloneCoreFactRoots() = empty, want gated categories from descriptors") +func TestCoreFactDescriptors_eachAssemblyStaysWithinDeclaredRoots(t *testing.T) { + host := &fakeHostOS{ + platform: "linux", + emptyRunDefault: true, + files: map[string][]byte{ + "/etc/os-release": []byte("ID=test\n"), + }, } - for _, root := range roots { - descriptor, ok := coreFactDescriptorByRoot(root, coreFactStandalone) - if !ok { - t.Fatalf("standalone root %q missing descriptor", root) - } - if !slices.Contains(descriptor.emittedRoots, root) { - t.Fatalf("standalone descriptor %q emitted roots = %#v, want root included", root, descriptor.emittedRoots) + build := newCoreFactBuild(gatingProbeSession(host)) + for _, descriptor := range coreFactDescriptors { + for _, fact := range descriptor.assemble(build) { + root, _, _ := strings.Cut(fact.Name, ".") + if !slices.Contains(descriptor.emittedRoots, root) { + t.Fatalf("descriptor %q emitted undeclared root %q from fact %q; declared roots = %#v", + descriptor.root, root, fact.Name, descriptor.emittedRoots) + } } } } -func coreFactDescriptorByRoot(root string, class coreFactGatingClass) (coreFactDescriptor, bool) { +func assertDescriptorDeclaresFacts(t *testing.T, descriptorRoot string, facts []ResolvedFact) { + t.Helper() for _, descriptor := range coreFactDescriptors { - if descriptor.root == root && descriptor.class == class { - return descriptor, true + if descriptor.root != descriptorRoot { + continue + } + for _, fact := range facts { + root, _, _ := strings.Cut(fact.Name, ".") + if !slices.Contains(descriptor.emittedRoots, root) { + t.Fatalf("descriptor %q emitted undeclared root %q from positive fact %q; declared roots = %#v", + descriptor.root, root, fact.Name, descriptor.emittedRoots) + } } + return } - return coreFactDescriptor{}, false + t.Fatalf("descriptor %q not found", descriptorRoot) } diff --git a/internal/engine/discovery_plan.go b/internal/engine/discovery_plan.go index a83daa89b..ecab57fc2 100644 --- a/internal/engine/discovery_plan.go +++ b/internal/engine/discovery_plan.go @@ -6,6 +6,7 @@ import ( type discoveryPlan struct { externalDirs []string + cachePath string noExternalFacts bool disabledFacts map[string]bool ambientDisabled map[string]string @@ -19,8 +20,13 @@ type discoveryPlan struct { } func (e *Engine) planDiscovery(s *Session, queries []string) (discoveryPlan, []error) { + return e.planDiscoveryWithDefaults(s, queries, e.defaultsForDiscovery()) +} + +func (e *Engine) planDiscoveryWithDefaults(s *Session, queries []string, defaults DiscoveryDefaults) (discoveryPlan, []error) { plan := discoveryPlan{ externalDirs: slices.Clone(e.cfg.ExternalDirs), + cachePath: defaults.CachePath, noExternalFacts: e.cfg.NoExternalFacts, disabledFacts: cloneBoolMap(e.cfg.DisabledFacts), useCache: e.cfg.UseCache, @@ -35,7 +41,7 @@ func (e *Engine) planDiscovery(s *Session, queries []string) (discoveryPlan, []e } var failures []error - config, ok, err := e.configForDiscovery(s) + config, ok, err := e.configForDiscovery(s, defaults) if err != nil { failures = append(failures, err) } else if ok { @@ -61,7 +67,7 @@ func (e *Engine) planDiscovery(s *Session, queries []string) (discoveryPlan, []e var defaultExternalDirs []string systemDefaults := e.cfg.SystemDefaults && !plan.noExternalFacts if systemDefaults && len(plan.externalDirs) == 0 && len(config.ExternalDirs) == 0 { - defaultExternalDirs = e.defaultExternalDirs() + defaultExternalDirs = slices.Clone(defaults.ExternalFactDirs) } configForDirs := config configForDirs.NoExternalFacts = false @@ -142,22 +148,25 @@ func (e *Engine) unionDisabledFacts(s *Session, config Config, includeEnv bool) return disabled, ambient } -func (e *Engine) configForDiscovery(s *Session) (Config, bool, error) { +func (e *Engine) configForDiscovery(s *Session, defaults DiscoveryDefaults) (Config, bool, error) { if e.cfg.ConfigLoaded { return cloneConfig(e.cfg.Config), true, nil } if e.cfg.ConfigFile == "" && !e.cfg.SystemDefaults { return Config{}, false, nil } - config, err := ParseConfig(e.cfg.ConfigFile, s.logger) + config, err := ParseConfig(e.cfg.ConfigFile, s.logger, defaults) return config, true, err } -func (e *Engine) defaultExternalDirs() []string { - if e.cfg.DefaultExternalDirsSet { - return slices.Clone(e.cfg.DefaultExternalDirs) +func (e *Engine) defaultsForDiscovery() DiscoveryDefaults { + if e.cfg.Defaults != nil { + return e.cfg.Defaults.clone() + } + if e.cfg.SystemDefaults || e.cfg.UseCache { + return CurrentDiscoveryDefaults() } - return CurrentDefaultExternalFactDirs() + return DiscoveryDefaults{} } func cloneBoolMap(in map[string]bool) map[string]bool { diff --git a/internal/engine/disks.go b/internal/engine/disks.go index 8f7be04ee..36c9983ec 100644 --- a/internal/engine/disks.go +++ b/internal/engine/disks.go @@ -940,10 +940,6 @@ func parseNetBSDDkctlWedges(input string, sectorSize int) map[string]any { return partitions } -func partitionsFact(partitions, mountpoints map[string]any) map[string]any { - return partitionsFactWithMountEntries(partitions, nil, mountpoints) -} - // partitionsFacts returns the partitions fact, or nothing when device // enumeration yields no entries: Ruby Facter omits the fact instead of // emitting an empty map (the resting state on macOS). diff --git a/internal/engine/disks_test.go b/internal/engine/disks_test.go index e701579c8..9d369c2ef 100644 --- a/internal/engine/disks_test.go +++ b/internal/engine/disks_test.go @@ -9,7 +9,7 @@ import ( "testing" ) -func TestPartitionsFactAddsFirstMountpointForDevice(t *testing.T) { +func TestPartitionsFactWithMountEntriesAddsFirstMountpointForDevice(t *testing.T) { partitions := map[string]any{ "/dev/sda2": map[string]any{ "filesystem": "btrfs", @@ -27,7 +27,7 @@ func TestPartitionsFactAddsFirstMountpointForDevice(t *testing.T) { }, } - got := partitionsFact(partitions, mountpoints) + got := partitionsFactWithMountEntries(partitions, nil, mountpoints) want := map[string]any{ "/dev/sda2": map[string]any{ "filesystem": "btrfs", @@ -38,7 +38,7 @@ func TestPartitionsFactAddsFirstMountpointForDevice(t *testing.T) { }, } if !reflect.DeepEqual(got, want) { - t.Fatalf("partitionsFact() = %#v, want %#v", got, want) + t.Fatalf("partitionsFactWithMountEntries() = %#v, want %#v", got, want) } } @@ -69,7 +69,7 @@ func TestPartitionsFactWithMountEntriesUsesResolverOrderForDuplicateDeviceLikeRu } } -func TestPartitionsFactMatchesFreeBSDGPTMountsByPartlabel(t *testing.T) { +func TestPartitionsFactWithMountEntriesMatchesFreeBSDGPTMountsByPartlabel(t *testing.T) { partitions := map[string]any{ "vtbd0p2": map[string]any{"partlabel": "efiesp"}, "vtbd0p5": map[string]any{"partlabel": "rootfs"}, @@ -79,7 +79,7 @@ func TestPartitionsFactMatchesFreeBSDGPTMountsByPartlabel(t *testing.T) { "/boot/efi": map[string]any{"device": "/dev/gpt/efiesp", "filesystem": "msdosfs"}, } - got := partitionsFact(partitions, mountpoints) + got := partitionsFactWithMountEntries(partitions, nil, mountpoints) root := got["vtbd0p5"].(map[string]any) if root["mount"] != "/" || root["filesystem"] != "ufs" { t.Fatalf("root partition = %#v, want mount and filesystem from mountpoint", root) @@ -128,20 +128,20 @@ func TestPartitionForMountDeviceMatchesNamesLabelsAndUUIDs(t *testing.T) { } } -func TestPartitionsFactReturnsPartitionsWithoutMountpoints(t *testing.T) { +func TestPartitionsFactWithMountEntriesReturnsPartitionsWithoutMountpoints(t *testing.T) { partitions := map[string]any{ "/dev/sda1": map[string]any{"filesystem": "ext3"}, } - got := partitionsFact(partitions, nil) + got := partitionsFactWithMountEntries(partitions, nil, nil) if !reflect.DeepEqual(got, partitions) { - t.Fatalf("partitionsFact() = %#v, want %#v", got, partitions) + t.Fatalf("partitionsFactWithMountEntries() = %#v, want %#v", got, partitions) } } -func TestPartitionsFactReturnsNilForEmptyPartitions(t *testing.T) { - if got := partitionsFact(map[string]any{}, map[string]any{"/": map[string]any{"device": "/dev/sda1"}}); got != nil { - t.Fatalf("partitionsFact() = %#v, want nil", got) +func TestPartitionsFactWithMountEntriesReturnsNilForEmptyPartitions(t *testing.T) { + if got := partitionsFactWithMountEntries(map[string]any{}, nil, map[string]any{"/": map[string]any{"device": "/dev/sda1"}}); got != nil { + t.Fatalf("partitionsFactWithMountEntries() = %#v, want nil", got) } } @@ -678,14 +678,14 @@ func TestParseNetBSDDkctlWedges_returnsDevicePartitions(t *testing.T) { } } -func TestPartitionsFactJoinsAllOpenBSDMountpointsFromMountpointDevices(t *testing.T) { +func TestPartitionsFactWithMountEntriesJoinsAllOpenBSDMountpointsFromMountpointDevices(t *testing.T) { partitions := parseBSDDisklabelPartitions("sd0", openBSDDisklabelSD0) mountpoints := openBSDMountpointsFact(`/dev/sd0a on / type ffs (local) /dev/sd0d on /usr type ffs (local, nodev) /dev/sd0e on /home type ffs (local, nodev, nosuid) `, "") - got := partitionsFact(partitions, mountpoints) + got := partitionsFactWithMountEntries(partitions, nil, mountpoints) for _, tt := range []struct { device string mount string @@ -696,7 +696,7 @@ func TestPartitionsFactJoinsAllOpenBSDMountpointsFromMountpointDevices(t *testin } { partition, ok := got[tt.device].(map[string]any) if !ok { - t.Fatalf("partitionsFact() = %#v, want partition %q", got, tt.device) + t.Fatalf("partitionsFactWithMountEntries() = %#v, want partition %q", got, tt.device) } if partition["mount"] != tt.mount { t.Fatalf("%s mount = %#v, want %#v", tt.device, partition["mount"], tt.mount) @@ -1598,7 +1598,9 @@ func TestCurrentZFSFactsRunsZFSAndZpoolUpgradeCommands(t *testing.T) { } } - got := factsByName(currentZFSFacts("freebsd", run)) + facts := currentZFSFacts("freebsd", run) + assertDescriptorDeclaresFacts(t, "disks", facts) + got := factsByName(facts) want := map[string]any{ "zfs.feature_numbers": []string{"1", "2"}, "zfs.version": "2", diff --git a/internal/engine/ec2_test.go b/internal/engine/ec2_test.go index 095a886dc..8cee98f66 100644 --- a/internal/engine/ec2_test.go +++ b/internal/engine/ec2_test.go @@ -182,6 +182,7 @@ func TestEC2Facts_returnsMetadataAndUserdataForAWSHypervisors(t *testing.T) { client := newEC2Client(server.URL+"/latest", server.Client()) facts := ec2Facts(testSession, client, virtualization{Name: "aws", IsVirtual: true}) + assertDescriptorDeclaresFacts(t, "ec2_metadata", facts) got := Collection(facts) metadata, ok := got["ec2_metadata"].(map[string]any) if !ok { diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 1a8865e92..334c96684 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -44,6 +44,10 @@ type EngineConfig struct { Logger *slog.Logger // Facts are registered facts, fixed at construction. Facts []ProgrammaticFact + // Defaults supplies invocation-local ambient paths to internal callers. + // A nil value derives fresh process defaults for each system-following or + // cache-enabled discovery. + Defaults *DiscoveryDefaults // The remaining fields are CLI-only knobs set by internal/app; the public // facts package exposes no options for them. @@ -69,11 +73,6 @@ type EngineConfig struct { // every Discover. ConfigLoaded bool Config Config - // DefaultExternalDirs overrides process default external dirs for - // internal/app tests and CLI adapter wiring. Nil is a valid override when - // DefaultExternalDirsSet is true. - DefaultExternalDirsSet bool - DefaultExternalDirs []string // IncludeTypedDotted enables CLI/config force-dot query projection. IncludeTypedDotted bool } @@ -96,7 +95,7 @@ func NewEngine(cfg EngineConfig) (*Engine, error) { cfg.ExternalDirs = slices.Clone(cfg.ExternalDirs) cfg.DisabledFacts = cloneBoolMap(cfg.DisabledFacts) cfg.ExtraDisabled = slices.Clone(cfg.ExtraDisabled) - cfg.DefaultExternalDirs = slices.Clone(cfg.DefaultExternalDirs) + cfg.Defaults = cloneDiscoveryDefaults(cfg.Defaults) cfg.Config = cloneConfig(cfg.Config) cfg.Facts = slices.Clone(cfg.Facts) for i, fact := range cfg.Facts { @@ -249,7 +248,7 @@ func (e *Engine) Discover(ctx context.Context, queries ...string) (*Snapshot, er } if plan.useCache && ctx.Err() == nil { - cache := NewFactCache(DefaultCachePath(), plan.cacheTTLs, plan.cacheGroups, s.logger) + cache := NewFactCache(plan.cachePath, plan.cacheTTLs, plan.cacheGroups, s.logger) cacheFacts := FilterDisabledFacts(facts, plan.disabledFacts) remaining, cached := cache.ResolveFacts(cacheFacts) cached = FilterDisabledFacts(cached, plan.disabledFacts) diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 6a550c69f..734ec78b0 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -56,13 +56,12 @@ func TestNewEngineFreezesConfigAndNormalizesFactNames(t *testing.T) { }} eng, err := NewEngine(EngineConfig{ - ExternalDirs: externalDirs, - DisabledFacts: blocked, - ConfigLoaded: true, - Config: config, - DefaultExternalDirsSet: true, - DefaultExternalDirs: defaultDirs, - Facts: facts, + ExternalDirs: externalDirs, + DisabledFacts: blocked, + ConfigLoaded: true, + Config: config, + Defaults: &DiscoveryDefaults{ExternalFactDirs: defaultDirs}, + Facts: facts, }) if err != nil { t.Fatal(err) @@ -83,8 +82,8 @@ func TestNewEngineFreezesConfigAndNormalizesFactNames(t *testing.T) { if got := eng.cfg.DisabledFacts["networking"]; !got { t.Fatalf("DisabledFacts[networking] = false, want frozen true") } - if got, want := eng.cfg.DefaultExternalDirs, []string{"/default"}; !reflect.DeepEqual(got, want) { - t.Fatalf("DefaultExternalDirs = %#v, want %#v", got, want) + if got, want := eng.cfg.Defaults.ExternalFactDirs, []string{"/default"}; !reflect.DeepEqual(got, want) { + t.Fatalf("Defaults.ExternalFactDirs = %#v, want %#v", got, want) } if got, want := eng.cfg.Config.Disabled, []string{"ssh"}; !reflect.DeepEqual(got, want) { t.Fatalf("Config.Disabled = %#v, want %#v", got, want) @@ -182,13 +181,12 @@ func TestPlanDiscoveryMergesLoadedConfig(t *testing.T) { func TestPlanDiscoveryPreservesExplicitInputsAndUsesDefaultDirs(t *testing.T) { eng, err := NewEngine(EngineConfig{ - ExternalDirs: []string{"/explicit"}, - DisabledFacts: map[string]bool{"explicit": true}, - ConfigLoaded: true, - Config: Config{ExternalDirs: []string{"/config"}, Disabled: []string{"config"}}, - SystemDefaults: true, - DefaultExternalDirsSet: true, - DefaultExternalDirs: []string{"/default"}, + ExternalDirs: []string{"/explicit"}, + DisabledFacts: map[string]bool{"explicit": true}, + ConfigLoaded: true, + Config: Config{ExternalDirs: []string{"/config"}, Disabled: []string{"config"}}, + SystemDefaults: true, + Defaults: &DiscoveryDefaults{ExternalFactDirs: []string{"/default"}}, }) if err != nil { t.Fatal(err) @@ -205,9 +203,8 @@ func TestPlanDiscoveryPreservesExplicitInputsAndUsesDefaultDirs(t *testing.T) { } defaultEng, err := NewEngine(EngineConfig{ - SystemDefaults: true, - DefaultExternalDirsSet: true, - DefaultExternalDirs: []string{"/default"}, + SystemDefaults: true, + Defaults: &DiscoveryDefaults{ExternalFactDirs: []string{"/default"}}, }) if err != nil { t.Fatal(err) @@ -226,14 +223,14 @@ func TestPlanDiscoveryPreservesExplicitInputsAndUsesDefaultDirs(t *testing.T) { } } -func TestDefaultExternalDirsWithoutOverrideUsesPlatformDefaults(t *testing.T) { - eng, err := NewEngine(EngineConfig{}) +func TestDefaultsForDiscoveryWithoutOverrideUsesPlatformDefaults(t *testing.T) { + eng, err := NewEngine(EngineConfig{SystemDefaults: true}) if err != nil { t.Fatal(err) } - if got, want := eng.defaultExternalDirs(), CurrentDefaultExternalFactDirs(); !reflect.DeepEqual(got, want) { - t.Fatalf("defaultExternalDirs() = %#v, want platform defaults %#v", got, want) + if got, want := eng.defaultsForDiscovery().ExternalFactDirs, CurrentDefaultExternalFactDirs(); !reflect.DeepEqual(got, want) { + t.Fatalf("defaultsForDiscovery().ExternalFactDirs = %#v, want platform defaults %#v", got, want) } } @@ -331,12 +328,11 @@ func TestEngineDiscoverReturnsPartialSnapshotWhenContextCancelled(t *testing.T) func TestEngineDiscoverUsesCachedValueForConfiguredFacts(t *testing.T) { cacheDir := t.TempDir() - oldDefaultCachePath := DefaultCachePath - DefaultCachePath = func() string { return cacheDir } - t.Cleanup(func() { DefaultCachePath = oldDefaultCachePath }) + defaults := DiscoveryDefaults{CachePath: cacheDir} eng, err := NewEngine(EngineConfig{ UseCache: true, + Defaults: &defaults, ConfigLoaded: true, Config: Config{TTLs: []FactTTL{{Fact: "cache_probe", TTL: "30 days"}}}, Facts: []ProgrammaticFact{{ @@ -365,6 +361,7 @@ func TestEngineDiscoverUsesCachedValueForConfiguredFacts(t *testing.T) { // The current cache contract is value precedence: a fresh cache entry wins. cachedEng, err := NewEngine(EngineConfig{ UseCache: true, + Defaults: &defaults, ConfigLoaded: true, Config: Config{TTLs: []FactTTL{{Fact: "cache_probe", TTL: "30 days"}}}, Facts: []ProgrammaticFact{{ diff --git a/internal/engine/external.go b/internal/engine/external.go index e7c225dad..063cbcdf2 100644 --- a/internal/engine/external.go +++ b/internal/engine/external.go @@ -27,7 +27,11 @@ var ErrNullByte = errors.New("external fact contains a null byte reference") // ErrExternalFactTooLarge reports an external fact source exceeding the byte cap. var ErrExternalFactTooLarge = errors.New("external fact exceeds size limit") -const externalFactResolutionEnv = "FACTER_EXTERNAL_FACTS_RUNNING" +const ( + externalFactResolutionEnv = "FACTER_EXTERNAL_FACTS_RUNNING" + externalFactCommandTimeout = 30 * time.Second + externalFactMaxBytes = 1 << 20 +) // reservedDisableControlName is the resolved external-fact name reserved for the // disabled-set control variable. Any FACTS_*/FACTER_* variable resolving to this @@ -36,8 +40,6 @@ const externalFactResolutionEnv = "FACTER_EXTERNAL_FACTS_RUNNING" const reservedDisableControlName = "disable" var errExternalFactOpen = errors.New("open external fact") -var externalFactCommandTimeout = 30 * time.Second -var externalFactMaxBytes = 1 << 20 type externalFactLoaderMode int diff --git a/internal/engine/external_test.go b/internal/engine/external_test.go index 74fe792de..e49d4c228 100644 --- a/internal/engine/external_test.go +++ b/internal/engine/external_test.go @@ -1,6 +1,7 @@ package engine import ( + "bytes" "context" "encoding/json" "errors" @@ -13,6 +14,7 @@ import ( "slices" "strings" "testing" + "testing/synctest" "time" ) @@ -1548,36 +1550,34 @@ func TestLoadExternalFacts_ignoresFailedExecutableFact(t *testing.T) { } func TestLoadExternalFacts_timesOutHungExecutableFact(t *testing.T) { - oldTimeout := externalFactCommandTimeout - externalFactCommandTimeout = 10 * time.Millisecond - t.Cleanup(func() { externalFactCommandTimeout = oldTimeout }) - - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, "site.txt"), []byte("site=lab\n"), 0o600); err != nil { - t.Fatal(err) - } - path := filepath.Join(dir, "hung_fact") - if err := os.WriteFile(path, []byte("#!/bin/sh\nsleep 10\nprintf 'ignored=true\\n'\n"), 0o700); err != nil { - t.Fatal(err) - } + synctest.Test(t, func(t *testing.T) { + var deadlineAfter time.Duration + host := &fakeExternalFactLoaderHost{ + runCommandFunc: func(ctx context.Context, _ string, _ ...string) ([]byte, []byte, error) { + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("external executable context has no deadline") + } + deadlineAfter = time.Until(deadline) + <-ctx.Done() + return nil, nil, ctx.Err() + }, + } + loader := externalFactLoader{s: NewSession(), host: host} - got, err := loadExternalFactsForTest(testSession, []string{dir}) - if err != nil { - t.Fatalf("loadExternalFactsForTest(testSession) err = %v, want nil for timed out executable fact", err) - } - want := []ResolvedFact{{Name: "site", Value: "lab", Type: "external"}} - if !reflect.DeepEqual(got, want) { - t.Fatalf("loadExternalFactsForTest(testSession) = %#v, want %#v", got, want) - } + _, err := loader.loadExternalCommandFacts("hung_fact", "hung_fact") + if !errors.Is(err, errExternalFactExec) { + t.Fatalf("loadExternalCommandFacts() error = %v, want errExternalFactExec", err) + } + if deadlineAfter != externalFactCommandTimeout { + t.Fatalf("executable deadline = %v, want %v", deadlineAfter, externalFactCommandTimeout) + } + }) } func TestExternalFactLoader_rejectsOversizedStructuredFactFile(t *testing.T) { - oldLimit := externalFactMaxBytes - externalFactMaxBytes = 8 - t.Cleanup(func() { externalFactMaxBytes = oldLimit }) - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, "huge.json"), []byte(`{"site":"larger than limit"}`), 0o600); err != nil { + if err := os.WriteFile(filepath.Join(dir, "huge.json"), bytes.Repeat([]byte{'x'}, externalFactMaxBytes+1), 0o600); err != nil { t.Fatal(err) } @@ -1652,17 +1652,10 @@ func TestExternalFactLoader_rejectsOversizedExecutableFactOutput(t *testing.T) { } } -func TestRunExternalFactCommand_rejectsOversizedStdout(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("uses /bin/sh") - } - oldLimit := externalFactMaxBytes - externalFactMaxBytes = 8 - t.Cleanup(func() { externalFactMaxBytes = oldLimit }) - - out, stderr, err := runExternalFactCommand(context.Background(), "/bin/sh", "-c", "printf 'site=larger-than-limit\\n'") +func TestReadExternalFactData_rejectsLimitPlusOne(t *testing.T) { + _, err := readExternalFactData(bytes.NewReader(bytes.Repeat([]byte{'x'}, externalFactMaxBytes+1))) if !errors.Is(err, ErrExternalFactTooLarge) { - t.Fatalf("runExternalFactCommand() stdout=%q stderr=%q err=%v, want ErrExternalFactTooLarge", out, stderr, err) + t.Fatalf("readExternalFactData() error = %v, want ErrExternalFactTooLarge", err) } } diff --git a/internal/engine/gce_test.go b/internal/engine/gce_test.go index 16a316aa9..f49dc31b7 100644 --- a/internal/engine/gce_test.go +++ b/internal/engine/gce_test.go @@ -44,6 +44,7 @@ func TestLinuxGCEFactsFetchRecursiveMetadataAndNormalizeInstance(t *testing.T) { t.Cleanup(server.Close) facts := linuxGCEFacts(context.Background(), "linux", "Google", newGCEClient(server.URL, server.Client())) + assertDescriptorDeclaresFacts(t, "gce", facts) got := factValues(facts) metadata, ok := got["gce"].(map[string]any) diff --git a/internal/engine/networking.go b/internal/engine/networking.go index 8d00ef44e..184c9aabb 100644 --- a/internal/engine/networking.go +++ b/internal/engine/networking.go @@ -165,9 +165,6 @@ func networkingInterfaces(s *Session) map[string]any { } func networkingInterfacesForPlatform(s *Session, goos string, snapshotProvider func() ([]networkInterfaceSnapshot, error)) map[string]any { - if goos == "plan9" { - return currentPlan9Interfaces(s.readFile, s.glob) - } snapshots, err := snapshotProvider() if err != nil { if goos == "windows" { diff --git a/internal/engine/networking_test.go b/internal/engine/networking_test.go index 235bd510b..eb06d4635 100644 --- a/internal/engine/networking_test.go +++ b/internal/engine/networking_test.go @@ -201,11 +201,12 @@ func TestNetworkingInterfacesWindowsLogsFailureLikeRubyResolver(t *testing.T) { } } -func TestNetworkingInterfacesForPlatformPlan9UsesSessionGlob(t *testing.T) { +func TestNetworkingCoreFactsPlan9UsesSessionGlob(t *testing.T) { t.Parallel() s := NewSession() - s.host = &fakeHostOS{ + host := &fakeHostOS{ + platform: "plan9", files: map[string][]byte{ "/net/ipifc/0/status": []byte(plan9Fixture(t, "ipifc_status")), "/net/ether0/addr": []byte(plan9Fixture(t, "ether0_addr")), @@ -214,17 +215,22 @@ func TestNetworkingInterfacesForPlatformPlan9UsesSessionGlob(t *testing.T) { "/net/ipifc/*/status": {"/net/ipifc/0/status"}, }, } - calledSnapshotProvider := false + s.host = host - got := networkingInterfacesForPlatform(s, "plan9", func() ([]networkInterfaceSnapshot, error) { - calledSnapshotProvider = true - return nil, nil - }) - if calledSnapshotProvider { - t.Fatal("networkingInterfacesForPlatform(plan9) called snapshot provider") + facts := Collection(networkingCoreFacts(s)) + networking, ok := facts["networking"].(map[string]any) + if !ok { + t.Fatalf("networkingCoreFacts(plan9) = %#v, want networking map", facts) + } + interfaces, ok := networking["interfaces"].(map[string]any) + if !ok { + t.Fatalf("networking.interfaces = %#v, want map", networking["interfaces"]) + } + if _, ok := interfaces["ether0"]; !ok { + t.Fatalf("networking.interfaces = %#v, want ether0", interfaces) } - if _, ok := got["ether0"]; !ok { - t.Fatalf("networkingInterfacesForPlatform(plan9) = %#v, want ether0", got) + if want := []string{"/net/ipifc/*/status"}; !reflect.DeepEqual(host.globCalls, want) { + t.Fatalf("glob calls = %#v, want %#v", host.globCalls, want) } } diff --git a/internal/engine/os.go b/internal/engine/os.go index 77e320344..df3d02f34 100644 --- a/internal/engine/os.go +++ b/internal/engine/os.go @@ -108,24 +108,6 @@ func architectureName(goos, machine string) string { return machine } -func windowsHardwareArchitecture(processor string, level int) (string, string) { - switch strings.ToUpper(strings.TrimSpace(processor)) { - case "AMD64": - return "x86_64", "x64" - case "ARM", "ARM64": - return "arm", "arm" - case "IA64": - return "ia64", "ia64" - case "INTEL", "386": - if level > 0 && level < 5 { - return "i" + strconv.Itoa(level) + "86", "x86" - } - return "i686", "x86" - default: - return "unknown", "unknown" - } -} - func windowsHardwareFromGoArch(goarch string) string { switch goarch { case "amd64": @@ -265,22 +247,6 @@ type windowsOSVersionInfo struct { MajorMinor string } -type windowsOSDescription struct { - ConsumerRelease bool - Description string -} - -func currentWindowsOSDescription(input string) *windowsOSDescription { - info := parseWindowsOSVersionInfo(input) - if info.ProductType == "" && info.Description == "" { - return nil - } - return &windowsOSDescription{ - ConsumerRelease: info.ProductType == "1", - Description: info.Description, - } -} - // currentWindowsKernel returns the Windows kernel name, release, and version // parsed from RtlGetVersion output. ok is false when the version is // unavailable, in which case a debug line is logged, mirroring the Ruby diff --git a/internal/engine/os_test.go b/internal/engine/os_test.go index f8f452e36..2e1e11609 100644 --- a/internal/engine/os_test.go +++ b/internal/engine/os_test.go @@ -373,13 +373,13 @@ func TestMacOSSystemProfilerProbesOmitEmptyOutput(t *testing.T) { } } -func TestCurrentWindowsOSDescriptionMatchesRubyResolver(t *testing.T) { +func TestParseWindowsOSVersionInfoMatchesRubyResolverInput(t *testing.T) { t.Parallel() tests := []struct { name string input string - want *windowsOSDescription + want windowsOSVersionInfo }{ { name: "query returns no result", @@ -388,12 +388,12 @@ func TestCurrentWindowsOSDescriptionMatchesRubyResolver(t *testing.T) { { name: "consumer release with empty description", input: "ProductType=1\r\nOtherTypeDescription=\r\n", - want: &windowsOSDescription{ConsumerRelease: true}, + want: windowsOSVersionInfo{ProductType: "1"}, }, { name: "missing product type keeps description and is not consumer", input: "ProductType=\r\nOtherTypeDescription=description\r\n", - want: &windowsOSDescription{Description: "description"}, + want: windowsOSVersionInfo{Description: "description"}, }, } @@ -401,9 +401,9 @@ func TestCurrentWindowsOSDescriptionMatchesRubyResolver(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := currentWindowsOSDescription(tt.input) - if !reflect.DeepEqual(got, tt.want) { - t.Fatalf("currentWindowsOSDescription() = %#v, want %#v", got, tt.want) + got := parseWindowsOSVersionInfo(tt.input) + if got != tt.want { + t.Fatalf("parseWindowsOSVersionInfo() = %#v, want %#v", got, tt.want) } }) } @@ -678,34 +678,9 @@ func TestArchitectureName_matchesRubyFacterUnameCompatibility(t *testing.T) { } } -func TestWindowsHardwareArchitecture_matchesRubyResolver(t *testing.T) { - tests := []struct { - name string - processor string - level int - wantHardware string - wantArchitecture string - }{ - {name: "amd64", processor: "AMD64", wantHardware: "x86_64", wantArchitecture: "x64"}, - {name: "arm", processor: "ARM", wantHardware: "arm", wantArchitecture: "arm"}, - {name: "ia64", processor: "IA64", wantHardware: "ia64", wantArchitecture: "ia64"}, - {name: "intel level below 5", processor: "INTEL", level: 4, wantHardware: "i486", wantArchitecture: "x86"}, - {name: "intel level above 5", processor: "INTEL", level: 8, wantHardware: "i686", wantArchitecture: "x86"}, - {name: "unknown", wantHardware: "unknown", wantArchitecture: "unknown"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - hardware, architecture := windowsHardwareArchitecture(tt.processor, tt.level) - if hardware != tt.wantHardware || architecture != tt.wantArchitecture { - t.Fatalf("windowsHardwareArchitecture(%q, %d) = %q, %q, want %q, %q", tt.processor, tt.level, hardware, architecture, tt.wantHardware, tt.wantArchitecture) - } - }) - } -} - func TestWindowsOSNameFamilyHardwareAndArchitectureMatchRubyFacts(t *testing.T) { - hardware, architecture := windowsHardwareArchitecture("AMD64", 0) + hardware := windowsHardwareFromGoArch("amd64") + architecture := windowsArchitectureFromHardware(hardware) if got := osName("windows", linuxDistro{}); got != "windows" { t.Fatalf("osName(windows) = %q, want windows", got) @@ -751,6 +726,10 @@ func TestWindowsHardwareAndArchitectureMappings(t *testing.T) { }{ {goarch: "amd64", wantHardware: "x86_64", wantArch: "x64"}, {goarch: "386", wantHardware: "i686", wantArch: "x86"}, + {goarch: "arm", wantHardware: "arm", wantArch: "arm"}, + {goarch: "arm64", wantHardware: "arm", wantArch: "arm"}, + {goarch: "ia64", wantHardware: "ia64", wantArch: "ia64"}, + {goarch: "unknown", wantHardware: "unknown", wantArch: "unknown"}, {goarch: "riscv64", wantHardware: "riscv64", wantArch: "riscv64"}, } for _, tt := range tests { @@ -2253,6 +2232,7 @@ func TestMacOSSystemProfilerFactsIncludesHardwareFacts(t *testing.T) { HardwareUUID: "11111111-2222-3333-4444-555555555555", SubsystemVendorID: "0x106b", }) + assertDescriptorDeclaresFacts(t, "os", facts) collection := Collection(facts) systemProfiler, ok := collection["system_profiler"].(map[string]any) diff --git a/internal/engine/plan9.go b/internal/engine/plan9.go index 5a55660ec..28aefc42c 100644 --- a/internal/engine/plan9.go +++ b/internal/engine/plan9.go @@ -266,12 +266,8 @@ func plan9ProcessorsCoreFacts(info processorInfo, isa string) []ResolvedFact { } func plan9NetworkingCoreFacts(s *Session) []ResolvedFact { - return plan9NetworkingCoreFactsWithGlob(s, s.glob) -} - -func plan9NetworkingCoreFactsWithGlob(s *Session, glob pathGlobber) []ResolvedFact { hostname := parsePlan9Sysname(readFileString("/dev/sysname", s.readFile)) - interfaces := currentPlan9Interfaces(s.readFile, glob) + interfaces := currentPlan9Interfaces(s.readFile, s.glob) primary, interfaces := currentNetworkingData("plan9", interfaces, s.commandOutput, s.readFile) ipv4, _ := primaryInterfaceFact(interfaces, primary, "ip").(string) primaryBinding := primaryIPv4Binding(interfaces, ipv4) diff --git a/internal/engine/plan9_parser_test.go b/internal/engine/plan9_parser_test.go index 8cd07bf2c..3d47f69a8 100644 --- a/internal/engine/plan9_parser_test.go +++ b/internal/engine/plan9_parser_test.go @@ -285,20 +285,22 @@ func TestPlan9NetworkingCoreFactsUseInjectedHostFiles(t *testing.T) { t.Parallel() s := NewSession() - s.host = &fakeHostOS{ + host := &fakeHostOS{ files: map[string][]byte{ "/dev/sysname": []byte(plan9Fixture(t, "sysname")), "/net/ipifc/0/status": []byte(plan9Fixture(t, "ipifc_status")), "/net/ether0/addr": []byte(plan9Fixture(t, "ether0_addr")), "/net/iproute": []byte(plan9Fixture(t, "iproute")), }, + globs: map[string][]string{ + "/net/ipifc/*/status": {"/net/ipifc/0/status"}, + }, + } + s.host = host + facts := Collection(plan9NetworkingCoreFacts(s)) + if want := []string{"/net/ipifc/*/status"}; !reflect.DeepEqual(host.globCalls, want) { + t.Fatalf("glob calls = %#v, want %#v", host.globCalls, want) } - facts := Collection(plan9NetworkingCoreFactsWithGlob(s, func(pattern string) ([]string, error) { - if pattern != "/net/ipifc/*/status" { - t.Fatalf("glob pattern = %q, want /net/ipifc/*/status", pattern) - } - return []string{"/net/ipifc/0/status"}, nil - })) networking, ok := facts["networking"].(map[string]any) if !ok { t.Fatalf("networking = %#v, want map", facts["networking"]) @@ -470,14 +472,14 @@ func TestPlan9NetworkingCoreFactsEmitFirstSliceOnly(t *testing.T) { "/net/iproute": []byte(plan9Fixture(t, "iproute")), } s := NewSession() - s.host = &fakeHostOS{files: files} + s.host = &fakeHostOS{ + files: files, + globs: map[string][]string{ + "/net/ipifc/*/status": {"/net/ipifc/0/status"}, + }, + } - facts := plan9NetworkingCoreFactsWithGlob(s, func(pattern string) ([]string, error) { - if pattern != "/net/ipifc/*/status" { - t.Fatalf("glob pattern = %q, want /net/ipifc/*/status", pattern) - } - return []string{"/net/ipifc/0/status"}, nil - }) + facts := plan9NetworkingCoreFacts(s) got := Collection(facts) networking, ok := got["networking"].(map[string]any) if !ok { diff --git a/internal/engine/seam_gate_test.go b/internal/engine/seam_gate_test.go index 66ecc04b9..ee48a2b55 100644 --- a/internal/engine/seam_gate_test.go +++ b/internal/engine/seam_gate_test.go @@ -78,3 +78,91 @@ func TestNoRawHostIOInResolvers(t *testing.T) { }) } } + +func TestNoMutableDiscoveryPolicyGlobals(t *testing.T) { + files := []string{"cache.go", "config.go", "defaults.go", "external.go", "../app/app.go"} + forbiddenNames := map[string]bool{ + "DefaultCachePath": true, + "DefaultConfigPath": true, + "NativeDefaultConfigPath": true, + "defaultExternalFactDirs": true, + "cacheWriteFile": true, + "cacheRemove": true, + "externalFactCommandTimeout": true, + "externalFactMaxBytes": true, + } + + fset := token.NewFileSet() + for _, path := range files { + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", path, err) + } + for _, decl := range file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.VAR { + continue + } + for _, spec := range gen.Specs { + valueSpec := spec.(*ast.ValueSpec) + for i, name := range valueSpec.Names { + if forbiddenNames[name.Name] || mutableDiscoveryPolicyReplacement(path, name.Name, valueSpec, i) { + pos := fset.Position(name.Pos()) + t.Errorf("%s:%d: mutable discovery policy %s must be invocation-local data or a constant", path, pos.Line, name.Name) + } + } + } + } + } +} + +func TestMutableDiscoveryPolicyReplacement_rejectsEquivalentLimits(t *testing.T) { + tests := []struct { + path string + name string + }{ + {path: "defaults.go", name: "commandDeadline"}, + {path: "defaults.go", name: "sizeCap"}, + {path: "external.go", name: "commandDeadline"}, + {path: "external.go", name: "sizeCap"}, + } + for _, tt := range tests { + t.Run(tt.path+"/"+tt.name, func(t *testing.T) { + spec := &ast.ValueSpec{ + Names: []*ast.Ident{ast.NewIdent(tt.name)}, + Values: []ast.Expr{&ast.BasicLit{Kind: token.INT, Value: "1"}}, + } + if !mutableDiscoveryPolicyReplacement(tt.path, tt.name, spec, 0) { + t.Fatalf("mutableDiscoveryPolicyReplacement(%q, %q) = false, want true", tt.path, tt.name) + } + }) + } +} + +func mutableDiscoveryPolicyReplacement(path, name string, spec *ast.ValueSpec, index int) bool { + lower := strings.ToLower(name) + if strings.Contains(lower, "timeout") || strings.Contains(lower, "deadline") || + ((strings.Contains(lower, "byte") || strings.Contains(lower, "size")) && + (strings.Contains(lower, "max") || strings.Contains(lower, "limit") || strings.Contains(lower, "cap"))) { + return true + } + if path != "cache.go" && path != "config.go" && path != "defaults.go" && path != "../app/app.go" { + return false + } + if _, ok := spec.Type.(*ast.FuncType); ok { + return true + } + if index >= len(spec.Values) { + return false + } + switch value := spec.Values[index].(type) { + case *ast.FuncLit: + return true + case *ast.Ident: + return strings.HasPrefix(value.Name, "platform") || strings.Contains(strings.ToLower(value.Name), "cache") + case *ast.SelectorExpr: + return strings.Contains(strings.ToLower(value.Sel.Name), "default") || value.Sel.Name == "Remove" + default: + return false + } +} diff --git a/openspec/changes/complete-engine-architecture-contracts/.openspec.yaml b/openspec/changes/complete-engine-architecture-contracts/.openspec.yaml new file mode 100644 index 000000000..cd2ce7e9f --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-16 diff --git a/openspec/changes/complete-engine-architecture-contracts/design.md b/openspec/changes/complete-engine-architecture-contracts/design.md new file mode 100644 index 000000000..9c520e43d --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/design.md @@ -0,0 +1,96 @@ +## Context + +The core-fact descriptor table records emitted roots and four gating classes, but production only gates the `standalone` class. Multi-output resolvers therefore still run when every output is disabled, and `newCoreFactBuild` acquires DMI eagerly even when neither DMI nor GCE output is kept. This conflicts with ADR-0015 and the existing `fact-disable-controls` requirement. + +Engine and CLI tests also replace eight package variables for default paths/directories, cache I/O failures, and external-fact limits. Production does not mutate them and the current race suite is clean, but the variables still contradict the immutable-Engine/no-global-state contract and prevent isolated parallel tests. Three resolver helpers and two Plan 9 networking seams are reachable only from tests. + +The audit rejected a broad networking boundary refactor. `networkingCoreFacts(*Session)` already returns the final category facts, while the injected `goos` and probe parameters are the fixture surface required by ADR-0010. This design only removes the two proven Plan 9 shadow paths. + +## Goals / Non-Goals + +**Goals:** + +- Make descriptor-declared emitted roots control resolver gating without changing enabled output. +- Preserve shared probe reuse through existing `Session` memoization and acquire DMI only when a kept consumer needs it. +- Remove mutable production globals while preserving discovery-time freshness and CLI/library input behavior. +- Preserve the fixed 30-second executable timeout, 1 MiB external-source limit, and cache warning semantics. +- Delete test-only/shadow code after moving useful assertions onto production paths. + +**Non-Goals:** + +- No fact names, values, ordering, format bytes, schema entries, public Go APIs, CLI flags, or dependencies change. +- No general resolver dependency graph or scheduler is introduced. +- No broad networking interface, `hostOS`, or fixture refactor is performed. +- `virtual`, `is_virtual`, other inline facts, and the `selinux` compatibility exception do not become resolution-gated. +- Platform-inapplicable roots are not treated as disabled roots. + +## Decisions + +### 1. Use one emitted-root predicate instead of a dependency graph + +Each descriptor will have the minimum live scheduling distinction: always-eager or gateable. A gateable descriptor runs when at least one declared top-level emitted root is enabled and is skipped only when every declared root is explicitly disabled. A standalone resolver is the one-root case of the same rule. + +The descriptor order and assembly functions remain the orchestrator. `emittedRoots` stays authoritative and receives a row-exact agreement test. The unused, ambiguous `probeConsumers` and `emitsUnder` fields are removed; shared-probe behavior comes from the kept assembly functions calling existing `Session` memoizers. + +Alternatives considered: + +- Retaining four class names would keep values with no distinct behavior and invite another metadata/implementation split. +- Building a probe-consumer graph would require resolving ambiguous non-root names (`azure` versus `az_metadata`, for example) and adds scheduling machinery that the existing memoizers already avoid. +- Deleting all metadata would leave multi-output gating dependent on another handwritten list. + +Provider descriptors must include every root they can emit, including shared `cloud`: Azure declares `az_metadata` and `cloud`; EC2 declares `ec2_metadata`, `ec2_userdata`, and `cloud`; GCE declares `gce` and `cloud`. OS, disk, and uptime descriptors likewise declare their complete root sets. + +### 2. Keep eager virtualization, make DMI demand-driven + +`detectVirtualization` remains in core-build construction because `virtual` and `is_virtual` are intentionally always eager under ADR-0015. DMI is removed from eager build construction. The DMI assembly and GCE assembly call `Session.cachedDMI()` independently when kept; the existing memoizer guarantees one probe when both need it and no probe when neither does. + +Identity/SSH sharing follows the same rule without new metadata: kept SSH resolution may call `cachedIdentity`, while an independently kept identity assembly calls the same memoizer. + +### 3. Capture ambient defaults in one invocation-local value + +An internal concrete defaults value will contain the facts-native and compatible config paths, cache path, and ordered default external-fact directories. The current platform/environment adapter produces this value once per `Engine.Discover` and once per CLI `Run`; tests may supply a literal value through internal construction seams. Slices are cloned when frozen. + +The zero-value public Engine remains hermetic. A system-following Engine derives a fresh defaults value for every discovery, preserving current environment/config freshness. The CLI derives one value per invocation and passes it through config parsing, fact-group listing, and Engine construction. This removes mutable path/directory functions without adding a new interface or public option. + +Native config still precedes the compatible config; explicit config and external directories still override defaults; `--no-external-facts` still suppresses every external source; cache selection and TTL policy remain discovery-scoped. + +Alternatives considered: + +- Storing defaults in a package singleton would retain the contract violation. +- Memoizing defaults on Engine construction would make repeated system-following discoveries stale. +- Adding one interface per filesystem/network operation would broaden the architecture for values that are already naturally represented as data. + +### 4. Treat cache I/O and external limits as fixed policy + +Cache production code calls `writeCacheFile` and `os.Remove` directly. Tests of permission-error diagnostics target small warning-policy helpers with explicit errors rather than replacing process-global functions. + +The external executable deadline and byte ceiling become constants. The timeout test uses the existing fake loader host to block on `ctx.Done()` inside `testing/synctest`, allowing the real 30-second deadline to advance virtually; it does not drive real `os/exec` I/O, which is not durably blocking to `synctest`. Size tests use a real limit-plus-one fixture. The loader remains configurable only through its existing host/session seam, not through policy fields. + +### 5. Remove false test surfaces + +Tests using `partitionsFact` move to the production `partitionsFactWithMountEntries` path. Windows mapping assertions move to `windowsHardwareFromGoArch`, `windowsArchitectureFromHardware`, and `parseWindowsOSVersionInfo`; the unreachable wrapper/type helpers are deleted. + +The unreachable Plan 9 branch in `networkingInterfacesForPlatform` is removed because production dispatches to `plan9NetworkingCoreFacts` first. Tests of `plan9NetworkingCoreFactsWithGlob` move to a `Session` backed by `fakeHostOS.globs`, after which the wrapper is deleted. All pure platform parser fixtures remain. + +## Risks / Trade-offs + +- **Incomplete emitted-root metadata could over-skip a resolver** → Add a row-exact descriptor test, output-subset agreement tests, and all-disabled/one-kept probe-count cases for every multi-output/provider descriptor. +- **Skipping work can remove debug messages or benign side effects** → Treat that as the intended ADR-0015 behavior, update the changelog, and pin public output plus ambient-disable diagnostic bytes. +- **Default capture could change precedence or freshness** → Exercise native/compatible config precedence, explicit/default external dirs, cache path selection, repeated discovery, concurrent isolation, and CLI contract tests. +- **Real timeout/size tests could become slow or memory-heavy** → Use `testing/synctest` and bounded 1 MiB-plus-one fixtures. +- **Plan 9 cleanup could accidentally change assembly** → Retarget tests to the production Session seam and run the native Plan 9 release gate with networking parity. + +## Migration Plan + +1. Add failing resolver-gating/probe-count and invocation-isolation tests. +2. Implement descriptor scheduling and demand-driven DMI, then update the changelog. +3. Introduce invocation-local defaults and remove mutable path/directory globals. +4. Constify external policy, remove cache I/O globals, and retarget tests. +5. Delete unreachable helper/shadow paths and retain assertions on production seams. +6. Run local formatting, unit, race, shuffle, vet, build, and dead-code gates; then validate the exact checkout on nlab and its supported guests. + +Rollback is a normal source revert; there is no data or configuration migration. + +## Open Questions + +None. The deep audit resolved the catalog, default-input, networking, and cleanup boundaries before implementation. diff --git a/openspec/changes/complete-engine-architecture-contracts/proposal.md b/openspec/changes/complete-engine-architecture-contracts/proposal.md new file mode 100644 index 000000000..2eb1dbca1 --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/proposal.md @@ -0,0 +1,28 @@ +## Why + +Facts records two architecture contracts that the implementation does not yet fully satisfy: disabling every output of a shared core resolver must skip that resolver, and immutable Engines must not depend on package-global mutable hooks. Deep inspection also found a small set of test-only and shadow paths that make those gaps harder to see, while rejecting a broader networking refactor as unnecessary. + +## What Changes + +- Make the core-fact descriptor catalog decide whether every gateable resolver runs, using its exact emitted top-level roots; preserve eager inline facts and the `selinux` compatibility exception. +- Defer shared DMI acquisition until a kept DMI or GCE output needs it, while retaining eager virtualization discovery for `virtual` and `is_virtual`. +- Replace mutable config, cache, and default external-directory hooks with invocation-local immutable discovery inputs at the existing Engine/CLI seams. +- Remove mutable cache I/O test hooks and make external-fact timeout and byte limits fixed policy, while preserving warning, timeout, size-limit, and error semantics. +- Delete three unreachable resolver helpers and the two Plan 9 networking shadow seams, retargeting useful assertions to production paths. +- Add structural and public-surface tests that enforce descriptor agreement, resolver gating, invocation isolation, input precedence, and unchanged output behavior. +- Update `CHANGELOG.md` for the corrected disabled-resolver behavior. No fact names or schema entries change. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `fact-disable-controls`: gate a multi-output or shared-output resolver when every top-level root it can emit is disabled, while running it whenever any sibling root remains enabled. +- `facts-library-api`: derive ambient system-following defaults into immutable invocation-local discovery inputs so concurrent Engines never require process-global mutation. + +## Impact + +The change affects core-fact descriptor scheduling, discovery input planning, cache and external-fact internals, CLI construction, and their tests under `internal/engine`, `internal/app`, and the root `facts` package. Public Go APIs, CLI flags, output bytes, input precedence, external-fact limits, supported fact schema, and dependencies remain unchanged. diff --git a/openspec/changes/complete-engine-architecture-contracts/specs/fact-disable-controls/spec.md b/openspec/changes/complete-engine-architecture-contracts/specs/fact-disable-controls/spec.md new file mode 100644 index 000000000..e99d3335c --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/specs/fact-disable-controls/spec.md @@ -0,0 +1,66 @@ +## MODIFIED Requirements + +### Requirement: Disabling skips resolution for a dedicated resolver + +Facts SHALL skip a gateable resolver when every top-level fact root it can produce is disabled, and otherwise SHALL resolve it and prune only the disabled output. + +#### Scenario: A standalone-resolver fact is resolution-gated + +- **WHEN** a fact produced by its own resolver (such as `packages`) is disabled +- **THEN** its resolver MUST NOT run and its collection work MUST be skipped + +#### Scenario: Every output of a multi-output resolver is disabled + +- **WHEN** every declared top-level root of a multi-output resolver is disabled +- **THEN** that resolver MUST NOT run +- **AND** disabling a sub-fact alone MUST NOT count as disabling its top-level root + +#### Scenario: A shared resolver still runs for kept siblings + +- **WHEN** a disabled fact shares a resolver that also produces a top-level root not in the disabled set +- **THEN** the resolver MUST run and the disabled output MUST be pruned from the result + +#### Scenario: A disabled sub-fact is pruned + +- **WHEN** a disabled target names a sub-fact such as `os.release` +- **THEN** its parent resolver MUST run and the sub-fact MUST be pruned from the value + +### Requirement: Disable semantics derive from one gating descriptor table + +The engine SHALL derive resolver gating, fact-group expansion, disabled-fact filtering and pruning, and ambient-disable diagnostics from a single gating descriptor table describing each core fact category — root fact name, Facter-compatible group name, scheduling policy, and the maximum set of top-level roots it can emit on any supported host. The fact-name hierarchy SHALL be walked through one shared helper wherever ancestor or descendant relationships are evaluated. Agreement between gate names, group membership, emitted-root metadata, and resolver output MUST be structurally enforced by tests, not by convention. + +#### Scenario: Descriptor agreement is enforced + +- **WHEN** a category's gate name, group membership, scheduling policy, or maximum emitted-root set disagrees with the row-pinned gating descriptor table +- **THEN** an engine test MUST fail rather than silently mis-gating or mis-expanding +- **AND** every root actually emitted on a fixture host MUST be declared, while a platform-inapplicable declared root MAY be absent from that run + +#### Scenario: Hierarchy walks agree + +- **WHEN** a disabled name is evaluated for post-resolution filtering, descendant pruning, ambient-source attribution, or CLI-disable subsumption +- **THEN** every path MUST reach the same ancestor and descendant conclusions through the shared hierarchy helper + +#### Scenario: Gateable descriptor keeps one sibling + +- **WHEN** at least one emitted root declared by a gateable descriptor remains enabled +- **THEN** its resolver MUST run +- **AND** disabled roots MUST be removed by the existing pruning behavior + +#### Scenario: Gateable descriptor loses every sibling + +- **WHEN** every emitted root declared by a gateable descriptor is explicitly disabled +- **THEN** its resolver MUST NOT run +- **AND** the skipped descriptor MUST NOT initiate a shared memoized probe +- **AND** another scheduled descriptor, including an always-eager descriptor, MAY still initiate and share that probe + +#### Scenario: Shared cloud roots are scheduled independently + +- **WHEN** a metadata provider's provider-specific roots are disabled but `cloud` remains enabled +- **THEN** that provider resolver MUST still run +- **AND** it MUST be skipped only when `cloud` and all of its provider-specific roots are disabled + +#### Scenario: Always-eager compatibility behavior remains + +- **WHEN** `facterversion`, `is_virtual`, `path`, `virtual`, or `selinux` is named in the disabled set +- **THEN** its inline or compatibility resolver behavior MUST remain eager +- **AND** disabling `selinux` by name remains a no-op because its facts emit under `os.selinux` diff --git a/openspec/changes/complete-engine-architecture-contracts/specs/facts-library-api/spec.md b/openspec/changes/complete-engine-architecture-contracts/specs/facts-library-api/spec.md new file mode 100644 index 000000000..9b9d9e51b --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/specs/facts-library-api/spec.md @@ -0,0 +1,48 @@ +## MODIFIED Requirements + +### Requirement: Immutable engine with explicit snapshot discovery +An Engine SHALL be immutable after construction, with all fact registrations and configuration fixed at `New`. Resolution SHALL happen only through `Discover(ctx)`, which returns an immutable Snapshot of the canonical tree. The library SHALL hold no package-global mutable state, SHALL capture ambient system-following defaults in an invocation-local value, and SHALL keep Engines and Snapshots safe for concurrent use. + +#### Scenario: Discovery honors context cancellation +- **WHEN** the context passed to `Discover` is cancelled or exceeds its deadline while resolvers (including command execution and cloud-metadata requests) are running +- **THEN** `Discover` returns promptly with an error satisfying `errors.Is(err, ctx.Err())` + +#### Scenario: Freshness requires re-discovery +- **WHEN** system state changes after a Snapshot was discovered +- **THEN** the existing Snapshot is unchanged, and a new `Discover` call returns a new Snapshot reflecting the new state + +#### Scenario: Engines are isolated +- **WHEN** two Engines with different configurations or invocation-local defaults discover concurrently in one process +- **THEN** each Snapshot reflects only its own Engine's configuration and discovery inputs, with no cross-engine interference and no data races + +#### Scenario: Test seams do not mutate process policy +- **WHEN** tests exercise alternate config paths, cache paths, external directories, cache failures, or the fixed executable deadline and byte-limit boundaries +- **THEN** they MUST use invocation-local inputs, fixed boundary values, production-owned helpers, or injected host probes +- **AND** they MUST NOT replace mutable package variables + +### Requirement: Discovery uses one input plan per run +The library SHALL derive source loading, disabled-set, cache, query-selection, and ambient default-path policy from one internal discovery plan for each `Discover` call. The plan MUST capture facts-native and compatible config paths, the cache path, and ordered default external-fact directories once per discovery. It MUST be recomputed per discovery so config files, external fact directories, environment facts, executable facts, cache contents, and ambient defaults remain fresh across repeated discovery on the same immutable Engine. + +#### Scenario: Config is read at discovery time +- **WHEN** an Engine configured with `WithConfigFile` discovers facts, the config file changes, and the same Engine discovers facts again +- **THEN** the second Snapshot reflects the updated config-derived external dirs, blocklists, and cache TTL/group policy + +#### Scenario: System defaults are invocation-local +- **WHEN** two concurrent system-following discoveries receive different ambient config, cache, or external-directory defaults through the internal host seam +- **THEN** each discovery MUST use only the defaults captured in its own input plan +- **AND** native config and external-directory locations MUST retain precedence over compatible locations + +#### Scenario: Query selection happens in discovery +- **WHEN** a consumer calls `Discover(ctx, "os.family")` +- **THEN** the returned Snapshot is backed by facts selected with the same projection semantics used by CLI query projection where the contracts overlap +- **AND** the public `Discover(ctx, queries...)` method shape remains unchanged + +#### Scenario: Cache policy stays discovery-scoped +- **WHEN** an Engine is configured with cache enabled and config-derived TTL/group policy +- **THEN** discovery applies cache resolution and cache refresh according to the per-discovery plan and its captured cache path +- **AND** the Engine does not memoize resolved fact values between discoveries + +#### Scenario: Force-dot resolution is not public library configuration +- **WHEN** a library consumer constructs an Engine through public `facts` options +- **THEN** no public option exists for force-dot resolution +- **AND** the canonical Snapshot tree preserves existing dotted external and registered fact behavior diff --git a/openspec/changes/complete-engine-architecture-contracts/tasks.md b/openspec/changes/complete-engine-architecture-contracts/tasks.md new file mode 100644 index 000000000..d882629ca --- /dev/null +++ b/openspec/changes/complete-engine-architecture-contracts/tasks.md @@ -0,0 +1,32 @@ +## 1. Descriptor-Driven Resolution Gating + +- [x] 1.1 Add failing probe-count tests for every standalone, multi-output, shared-output, cloud-provider, identity/SSH, and DMI/GCE all-disabled versus one-kept case. +- [x] 1.2 Add public `facts` API or `internal/app` contract coverage proving corrected gating leaves resolved output, pruning, cache, queries, and ambient-disable diagnostics unchanged. +- [x] 1.3 Replace the four inert gating classes with the minimum always-eager/gateable policy and make exact `emittedRoots` drive one resolver-run predicate. +- [x] 1.4 Remove `probeConsumers` and `emitsUnder`, move DMI acquisition from eager core-build construction to the kept DMI/GCE assemblies, and preserve eager virtualization plus `selinux` behavior. +- [x] 1.5 Add row-exact descriptor metadata/output agreement tests covering shared `cloud` roots and every multi-output category. +- [x] 1.6 Update `CHANGELOG.md` with the corrected fully-disabled resolver behavior. + +## 2. Invocation-Local Discovery Inputs + +- [x] 2.1 Add failing concurrent-isolation and repeated-discovery tests for distinct config paths, cache paths, and ordered default external directories without package-variable mutation. +- [x] 2.2 Introduce one clone-safe internal defaults value and derive it once per library discovery or CLI invocation through current platform/environment adapters. +- [x] 2.3 Thread invocation defaults through config parsing, discovery planning, fact-group listing, Engine construction, and cache construction while preserving all explicit/default precedence. +- [x] 2.4 Delete `DefaultCachePath`, `DefaultConfigPath`, `NativeDefaultConfigPath`, and `defaultExternalFactDirs`; retarget affected root, engine, and app tests to local inputs and enable safe parallelism. +- [x] 2.5 Delete `cacheWriteFile` and `cacheRemove`, call production I/O directly, and retarget permission-failure assertions to production-owned warning-policy helpers. +- [x] 2.6 Make the external-fact timeout and byte ceiling constants; retarget timeout coverage to `testing/synctest` and size coverage to real limit-plus-one inputs. +- [x] 2.7 Extend the architecture seam gate to reject the removed mutable hook names and equivalent package-level function/limit replacements. + +## 3. False Test Surface Cleanup + +- [x] 3.1 Retarget partition and Windows assertions to live production seams, then delete `partitionsFact`, `windowsHardwareArchitecture`, `windowsOSDescription`, and `currentWindowsOSDescription`. +- [x] 3.2 Retarget Plan 9 networking assembly tests to `Session` plus `fakeHostOS.globs`, then delete the unreachable `networkingInterfacesForPlatform` Plan 9 branch and `plan9NetworkingCoreFactsWithGlob` wrapper. +- [x] 3.3 Run cross-target dead-code analysis and confirm every removed helper/shadow path is absent without introducing replacement dead code. + +## 4. Verification and Review + +- [x] 4.1 Run `gofmt -w` on edited Go files and the focused engine/root/app tests while implementing each workstream test-first. +- [x] 4.2 Run `go test ./...`, `go test -race . ./internal/engine ./internal/app`, repeated shuffled tests, `go vet ./...`, and `make build` locally. +- [x] 4.3 Run `@open-code-review`, classify every finding, fix accepted High/Medium issues, and rerun affected local gates. +- [x] 4.4 Sync the exact checkout to nlab; run Debian full/race/build gates and focused native Windows, FreeBSD, and Plan 9 coverage for defaults, external facts, and networking parity. +- [x] 4.5 Run the remaining supported-guest release gates required by any platform-sensitive diff and record evidence before pull-request publication.