From 5260cd6642395fefe088342c77aa17b7a6eb26ca Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Thu, 9 Jul 2026 21:07:44 +0000 Subject: [PATCH 1/6] feat(watch): start API-surface trigger informers only for served resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The APIService trigger informer was created unconditionally. On an API server without an aggregation layer (kcp, and other non-aggregating control planes) apiregistration.k8s.io is not served at all, so client-go's reflector retried and logged forever — benign, endlessly repeated noise, which is exactly how a real error gets missed. Both trigger informers (CustomResourceDefinition, APIService) are now gated on discovery reporting the resource served with list+watch, and re-evaluated after every successful catalog refresh rather than once at startup. An aggregation layer installed later is picked up without a restart; an absent one is logged once, not once per refresh. The decision is extracted as the pure selectAPISurfaceTriggers so the no-aggregation case is testable without an API server. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/watch/api_resource_catalog.go | 38 +++++ internal/watch/api_surface_triggers_test.go | 161 ++++++++++++++++++++ internal/watch/manager.go | 20 ++- internal/watch/manager_catalog.go | 118 ++++++++++++-- 4 files changed, 320 insertions(+), 17 deletions(-) create mode 100644 internal/watch/api_surface_triggers_test.go diff --git a/internal/watch/api_resource_catalog.go b/internal/watch/api_resource_catalog.go index 1b100061..12c76deb 100644 --- a/internal/watch/api_resource_catalog.go +++ b/internal/watch/api_resource_catalog.go @@ -101,6 +101,44 @@ func (c *APIResourceCatalog) Scan(sensitive types.SensitiveResourcePolicy) (type return scan, true } +// ServesWatchable reports whether the latest scan saw this exact group/version/resource +// served with both list and watch verbs. It is the precondition for opening an informer +// on a resource the operator does not require: an API server that does not aggregate does +// not serve apiregistration.k8s.io at all, and a blind informer on it retries forever. +// It deliberately ignores the allow/deny watch policy, which governs what the operator +// mirrors, not what it may inspect. Reports false before the first trusted scan. +func (c *APIResourceCatalog) ServesWatchable(gvr schema.GroupVersionResource) bool { + c.mu.RLock() + defer c.mu.RUnlock() + if !c.ready { + return false + } + for _, entry := range c.lastScan.Entries { + if entry.GVR != gvr { + continue + } + return hasVerbs(entry.Verbs, "list", "watch") + } + return false +} + +// hasVerbs reports whether every wanted verb is present in the sorted verb list. +func hasVerbs(verbs []string, wanted ...string) bool { + for _, want := range wanted { + found := false + for _, verb := range verbs { + if verb == want { + found = true + break + } + } + if !found { + return false + } + } + return true +} + // DegradedGroupVersions returns the group/versions the latest scan reported as // failed, sorted for stable logging. func (c *APIResourceCatalog) DegradedGroupVersions() []schema.GroupVersion { diff --git a/internal/watch/api_surface_triggers_test.go b/internal/watch/api_surface_triggers_test.go new file mode 100644 index 00000000..319806bc --- /dev/null +++ b/internal/watch/api_surface_triggers_test.go @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// aggregatingDiscovery is the common test surface plus a served apiregistration.k8s.io/v1, +// i.e. an ordinary kube-apiserver rather than a control plane without an aggregation layer. +func aggregatingDiscovery() staticCatalogDiscovery { + disco := newCommonTestDiscovery() + disco.groups = append(disco.groups, testAPIGroup("apiregistration.k8s.io", "v1")) + disco.resources = append(disco.resources, &metav1.APIResourceList{ + GroupVersion: "apiregistration.k8s.io/v1", + APIResources: []metav1.APIResource{ + {Name: "apiservices", Kind: "APIService", Verbs: metav1.Verbs{"get", "list", "watch"}}, + }, + }) + return disco +} + +func TestAPIResourceCatalog_ServesWatchable(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + discovery staticCatalogDiscovery + gvr schema.GroupVersionResource + want bool + }{ + "crd served on a stock apiserver": { + discovery: newCommonTestDiscovery(), + gvr: crdTriggerGVR(), + want: true, + }, + "apiservices absent without an aggregation layer": { + discovery: newCommonTestDiscovery(), + gvr: apiServiceTriggerGVR(), + want: false, + }, + "apiservices served when the group is aggregated": { + discovery: aggregatingDiscovery(), + gvr: apiServiceTriggerGVR(), + want: true, + }, + "served but not watchable": { + discovery: staticCatalogDiscovery{ + groups: []*metav1.APIGroup{testAPIGroup("apiregistration.k8s.io", "v1")}, + resources: []*metav1.APIResourceList{{ + GroupVersion: "apiregistration.k8s.io/v1", + APIResources: []metav1.APIResource{ + {Name: "apiservices", Kind: "APIService", Verbs: metav1.Verbs{"get", "list"}}, + }, + }}, + }, + gvr: apiServiceTriggerGVR(), + want: false, + }, + "unknown resource": { + discovery: newCommonTestDiscovery(), + gvr: schema.GroupVersionResource{Group: "nope.example.com", Version: "v1", Resource: "widgets"}, + want: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + catalog := NewAPIResourceCatalog() + _, err := catalog.Refresh(tc.discovery) + require.NoError(t, err) + require.Equal(t, tc.want, catalog.ServesWatchable(tc.gvr)) + }) + } +} + +func TestAPIResourceCatalog_ServesWatchable_BeforeFirstScan(t *testing.T) { + t.Parallel() + // A catalog that has never accepted a trusted scan must not claim anything is + // watchable, or the manager would open an informer on an API surface it has not seen. + require.False(t, NewAPIResourceCatalog().ServesWatchable(crdTriggerGVR())) +} + +func TestSelectAPISurfaceTriggers_SkipsUnservedAPIServices(t *testing.T) { + t.Parallel() + + catalog := newCommonTestCatalog(t) + start, unserved := selectAPISurfaceTriggers(catalog, map[schema.GroupVersionResource]struct{}{}) + + require.Equal(t, []schema.GroupVersionResource{crdTriggerGVR()}, start, + "only the served CRD trigger should start") + require.Equal(t, []schema.GroupVersionResource{apiServiceTriggerGVR()}, unserved, + "apiservices must be reported unserved, not started blindly") +} + +func TestSelectAPISurfaceTriggers_StartsBothWhenAggregationIsServed(t *testing.T) { + t.Parallel() + + catalog := NewAPIResourceCatalog() + _, err := catalog.Refresh(aggregatingDiscovery()) + require.NoError(t, err) + + start, unserved := selectAPISurfaceTriggers(catalog, map[schema.GroupVersionResource]struct{}{}) + + require.ElementsMatch(t, apiSurfaceTriggerGVRs(), start) + require.Empty(t, unserved) +} + +func TestSelectAPISurfaceTriggers_NeverRestartsARunningInformer(t *testing.T) { + t.Parallel() + + catalog := NewAPIResourceCatalog() + _, err := catalog.Refresh(aggregatingDiscovery()) + require.NoError(t, err) + + started := map[schema.GroupVersionResource]struct{}{crdTriggerGVR(): {}} + start, unserved := selectAPISurfaceTriggers(catalog, started) + + require.Equal(t, []schema.GroupVersionResource{apiServiceTriggerGVR()}, start) + require.Empty(t, unserved) +} + +// An aggregation layer installed after startup must be picked up on the next catalog +// refresh, without restarting the operator. +func TestSelectAPISurfaceTriggers_PicksUpAggregationInstalledLater(t *testing.T) { + t.Parallel() + + catalog := newCommonTestCatalog(t) + started := map[schema.GroupVersionResource]struct{}{} + + start, unserved := selectAPISurfaceTriggers(catalog, started) + require.Equal(t, []schema.GroupVersionResource{crdTriggerGVR()}, start) + require.Equal(t, []schema.GroupVersionResource{apiServiceTriggerGVR()}, unserved) + for _, gvr := range start { + started[gvr] = struct{}{} + } + + _, err := catalog.Refresh(aggregatingDiscovery()) + require.NoError(t, err) + + start, unserved = selectAPISurfaceTriggers(catalog, started) + require.Equal(t, []schema.GroupVersionResource{apiServiceTriggerGVR()}, start) + require.Empty(t, unserved) +} + +func TestEnsureAPISurfaceTriggerInformers_NoOpBeforeStart(t *testing.T) { + t.Parallel() + + // triggerCtx is nil until Start runs. Calling in through a controller-driven catalog + // refresh before then must not create a factory bound to a dead context. + m := &Manager{Log: logr.Discard()} + m.ensureAPISurfaceTriggerInformers(m.Log) + + require.Nil(t, m.triggerFactory) + require.Empty(t, m.triggersStarted) +} diff --git a/internal/watch/manager.go b/internal/watch/manager.go index 352d4fd0..101d9bad 100644 --- a/internal/watch/manager.go +++ b/internal/watch/manager.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" + "k8s.io/client-go/dynamic/dynamicinformer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -96,6 +97,20 @@ type Manager struct { discoveryClient func() (apiResourceDiscovery, error) // catalogRefreshCh coalesces API-surface trigger watch events into manager reconciliation. catalogRefreshCh chan struct{} + // triggersMu guards the API-surface trigger informer set below. The informers are + // (re-)evaluated after every catalog refresh — which controllers drive, not just the + // manager's own loop — so this is not a startup-only structure. + triggersMu sync.Mutex + // triggerCtx is the manager's lifetime context, the stop channel every trigger informer + // is started with. Set once by Start; nil before then, which defers informer creation. + triggerCtx context.Context + // triggerFactory is the shared dynamic informer factory backing the trigger informers. + triggerFactory dynamicinformer.DynamicSharedInformerFactory + // triggersStarted is the set of trigger resources whose informer is already running. + triggersStarted map[schema.GroupVersionResource]struct{} + // triggersSkipLogged records which unserved trigger resources have already been logged, + // so a permanently absent aggregation layer produces one line, not one per refresh. + triggersSkipLogged map[schema.GroupVersionResource]struct{} // catalogReadyOnce guards the one-time "catalog ready" log line, matching the // firstMessage/firstGroupReady sync.Once pattern used by the audit consumer. catalogReadyOnce sync.Once @@ -180,6 +195,10 @@ func (m *Manager) Start(ctx context.Context) error { defer log.Info("watch ingestion manager stopping") m.initializeManagerState() + // Arm the trigger informers before the first refresh: each successful catalog refresh + // re-evaluates which triggers discovery actually serves, and starts the ones that + // became available. They are stopped by this context, never by a reconcile's. + m.setTriggerContext(ctx) if err := m.bootstrapRuleStore(ctx, log.WithName("bootstrap")); err != nil { log.Error(err, "RuleStore bootstrap failed, continuing with current in-memory state") @@ -189,7 +208,6 @@ func (m *Manager) Start(ctx context.Context) error { if err := m.ReconcileForRuleChange(ctx); err != nil { log.Error(err, "Initial reconciliation failed, will retry periodically") } - m.startAPISurfaceTriggerInformers(ctx, log.WithName("catalog-triggers")) // Periodic reconciliation for CRD detection and missed changes periodicTicker := time.NewTicker(periodicReconcileInterval) diff --git a/internal/watch/manager_catalog.go b/internal/watch/manager_catalog.go index b4a4323d..8b5cc2f7 100644 --- a/internal/watch/manager_catalog.go +++ b/internal/watch/manager_catalog.go @@ -72,6 +72,10 @@ func (m *Manager) RefreshAPIResourceCatalog(ctx context.Context) error { stats := catalog.Stats() recordCatalogStats(ctx, stats) m.logCatalogTransitions(catalog, stats) + // The fresh scan is the only source of truth for which trigger resources this API + // server actually serves, so trigger informers are (re-)armed here rather than once + // at startup. An aggregation layer installed later is picked up on its refresh. + m.ensureAPISurfaceTriggerInformers(m.Log.WithName("catalog-triggers")) } return refreshErr } @@ -360,37 +364,119 @@ func (m *Manager) signalCatalogRefresh() { } } -func (m *Manager) startAPISurfaceTriggerInformers(ctx context.Context, log logr.Logger) { - cfg := m.restConfig() - if cfg == nil { - log.V(1).Info("skipping API surface trigger informers - no REST config available") +// ensureAPISurfaceTriggerInformers starts the CRD and APIService trigger informers, but +// only for the resources discovery reports as served with list+watch. Neither is universal: +// an API server without an aggregation layer serves no apiregistration.k8s.io, and a blind +// informer on it makes client-go's reflector retry and log forever — benign, endlessly +// repeated noise that is exactly how a real error gets missed. +// +// It is idempotent and re-evaluated after every successful catalog refresh, so an +// aggregation layer (or the apiextensions group) installed later is picked up without a +// restart. Informers already started are never restarted, and a skip is logged once per +// resource, not once per refresh. +func (m *Manager) ensureAPISurfaceTriggerInformers(log logr.Logger) { + m.triggersMu.Lock() + defer m.triggersMu.Unlock() + + ctx := m.triggerCtx + if ctx == nil { + // Start has not run yet; the first refresh happens inside it and re-enters here. return } - dynamicClient, err := dynamic.NewForConfig(cfg) - if err != nil { - log.Error(err, "failed to create API surface trigger client") + catalog := m.apiResourceCatalog() + if !catalog.Ready() { + log.V(1).Info("deferring API surface trigger informers - catalog not ready") return } + if m.triggerFactory == nil { + cfg := m.restConfig() + if cfg == nil { + log.V(1).Info("skipping API surface trigger informers - no REST config available") + return + } + dynamicClient, err := dynamic.NewForConfig(cfg) + if err != nil { + log.Error(err, "failed to create API surface trigger client") + return + } + m.triggerFactory = dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 0) + m.triggersStarted = map[schema.GroupVersionResource]struct{}{} + m.triggersSkipLogged = map[schema.GroupVersionResource]struct{}{} + } - factory := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 0) handler := cache.ResourceEventHandlerFuncs{ AddFunc: func(any) { m.signalCatalogRefresh() }, UpdateFunc: func(any, any) { m.signalCatalogRefresh() }, DeleteFunc: func(any) { m.signalCatalogRefresh() }, } - informers := []cache.SharedIndexInformer{ - factory.ForResource(crdTriggerGVR()).Informer(), - factory.ForResource(apiServiceTriggerGVR()).Informer(), + + start, unserved := selectAPISurfaceTriggers(catalog, m.triggersStarted) + for _, gvr := range unserved { + if _, logged := m.triggersSkipLogged[gvr]; logged { + continue + } + m.triggersSkipLogged[gvr] = struct{}{} + log.Info("API surface trigger not served by this API server; "+ + "the catalog refreshes on its periodic tick instead", "gvr", gvr.String()) } - for _, informer := range informers { + + var fresh []cache.SharedIndexInformer + for _, gvr := range start { + informer := m.triggerFactory.ForResource(gvr).Informer() if _, addErr := informer.AddEventHandler(handler); addErr != nil { - log.Error(addErr, "failed to add API surface trigger handler") - return + log.Error(addErr, "failed to add API surface trigger handler", "gvr", gvr.String()) + continue + } + m.triggersStarted[gvr] = struct{}{} + delete(m.triggersSkipLogged, gvr) + fresh = append(fresh, informer) + log.Info("API surface trigger informer started", "gvr", gvr.String()) + } + if len(fresh) == 0 { + return + } + + // Start is idempotent per informer: it launches only the ones not yet running. + m.triggerFactory.Start(ctx.Done()) + go waitForAPISurfaceTriggerSync(ctx, log, fresh) +} + +// apiSurfaceTriggerGVRs are the resources whose changes mean the API surface moved: a CRD +// (custom types appear/disappear) and an APIService (an aggregated group appears/goes +// unhealthy). Neither is guaranteed to be served. +func apiSurfaceTriggerGVRs() []schema.GroupVersionResource { + return []schema.GroupVersionResource{crdTriggerGVR(), apiServiceTriggerGVR()} +} + +// selectAPISurfaceTriggers splits the trigger resources not yet running into the ones +// discovery says are watchable now (start) and the ones it does not serve (unserved). It +// is the whole decision behind ensureAPISurfaceTriggerInformers, kept pure so the +// "no aggregation layer" case is testable without an API server. +func selectAPISurfaceTriggers( + catalog *APIResourceCatalog, + started map[schema.GroupVersionResource]struct{}, +) ([]schema.GroupVersionResource, []schema.GroupVersionResource) { + var start, unserved []schema.GroupVersionResource + for _, gvr := range apiSurfaceTriggerGVRs() { + if _, running := started[gvr]; running { + continue } + if catalog.ServesWatchable(gvr) { + start = append(start, gvr) + continue + } + unserved = append(unserved, gvr) } + return start, unserved +} - factory.Start(ctx.Done()) - go waitForAPISurfaceTriggerSync(ctx, log, informers) +// setTriggerContext records the manager's lifetime context, the stop channel every +// trigger informer is started with. Informers must outlive the reconcile call that +// discovers their resource became available, so they can never use its context. +func (m *Manager) setTriggerContext(ctx context.Context) { + m.triggersMu.Lock() + defer m.triggersMu.Unlock() + m.triggerCtx = ctx } func waitForAPISurfaceTriggerSync(ctx context.Context, log logr.Logger, informers []cache.SharedIndexInformer) { From 0f16a066ba247e5902b2cd72dfaafc2e38e890ad Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Thu, 9 Jul 2026 21:07:44 +0000 Subject: [PATCH 2/6] feat(queue): add --redis-key-prefix so reversers can share one Redis Redis offers 16 logical databases, and --redis-db was the only separator this operator provided. An install that runs one reverser per tenant, or per branch environment, hits that ceiling long before it hits any real Redis limit. Every key (watch resume cursors, attribution facts, command author records) was rooted at the compile-time const "gitops-reverser". That root is now a RedisStoreConfig field threaded into RedisStore, AttributionIndex and CommandAuthorStore, and surfaced as --redis-key-prefix / queue.redis.keyPrefix. It defaults to the historic value, so an upgrade that sets nothing keeps reading the keys the previous release wrote. The prefix is validated rather than escaped: the attribution fact-index gauge SCANs ":author:v1:audit:*", so a glob metacharacter in the prefix would silently count the wrong keyspace. '%' is rejected for the same reason against escapeKeyField. ':' is allowed (a prefix like "cell-a:tenant-7" nests naturally); a trailing ':' is normalized away. Co-Authored-By: Claude Opus 4.8 (1M context) --- Taskfile-build.yml | 4 + .../gitops-reverser/templates/deployment.yaml | 3 + charts/gitops-reverser/values.yaml | 6 + cmd/main.go | 17 ++ cmd/main_audit_server_test.go | 29 ++++ cmd/main_chart_args_test.go | 102 ++++++++++++ internal/queue/attribution_index.go | 14 +- internal/queue/command_author_store.go | 4 +- internal/queue/key_prefix.go | 66 ++++++++ internal/queue/key_prefix_test.go | 150 ++++++++++++++++++ internal/queue/redis_store.go | 22 ++- 11 files changed, 407 insertions(+), 10 deletions(-) create mode 100644 cmd/main_chart_args_test.go create mode 100644 internal/queue/key_prefix.go create mode 100644 internal/queue/key_prefix_test.go diff --git a/Taskfile-build.yml b/Taskfile-build.yml index 9e777210..8c45281f 100644 --- a/Taskfile-build.yml +++ b/Taskfile-build.yml @@ -132,6 +132,10 @@ tasks: - fmt - vet - setup-envtest + # cmd's chart-args test runs `helm template`, which evaluates every template — + # including rbac.yaml, which reads the gitignored chart/config/role.yaml this + # syncs. Without it a fresh checkout fails the unit suite. + - helm-sync cmds: - | export KUBEBUILDER_ASSETS="$({{.ENVTEST}} use {{.ENVTEST_K8S_VERSION}} --bin-dir "$(pwd)/bin" -p path)" diff --git a/charts/gitops-reverser/templates/deployment.yaml b/charts/gitops-reverser/templates/deployment.yaml index b54acffc..68e28e29 100644 --- a/charts/gitops-reverser/templates/deployment.yaml +++ b/charts/gitops-reverser/templates/deployment.yaml @@ -71,6 +71,9 @@ spec: - --redis-username={{ . | quote }} {{- end }} - --redis-db={{ .Values.queue.redis.db }} + {{- with .Values.queue.redis.keyPrefix }} + - {{ printf "--redis-key-prefix=%s" . | quote }} + {{- end }} {{- if not .Values.queue.redis.tls.enabled }} - --redis-insecure {{- end }} diff --git a/charts/gitops-reverser/values.yaml b/charts/gitops-reverser/values.yaml index 7fecb653..36be158c 100644 --- a/charts/gitops-reverser/values.yaml +++ b/charts/gitops-reverser/values.yaml @@ -184,6 +184,12 @@ queue: # Optional Redis username for ACL-based auth (Redis 6+). Not sensitive; stays as a plain value. username: "" db: 0 + # Root namespace for every key this release writes (cursors, attribution facts, command author + # records). Give each reverser its own prefix to share one Redis/Valkey between more than the 16 + # logical databases `db` can separate. A Valkey ACL can enforce it (~:*) rather than trust + # it. Changing it orphans the keys written under the previous prefix: cursors then cold-replay + # once, which is safe. Allowed characters: [A-Za-z0-9], '-', '_', '.' and ':'. + keyPrefix: "gitops-reverser" tls: enabled: false diff --git a/cmd/main.go b/cmd/main.go index 1d6c01e4..6a8e4511 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -194,9 +194,12 @@ func main() { AuthValue: cfg.redisPassword, DB: cfg.redisDB, TLSEnabled: !cfg.redisInsecure, + KeyPrefix: cfg.redisKeyPrefix, }) fatalIfErr(err, "unable to build Redis cursor store") watchMgr.WatchCursorStore = redisStore + setupLog.Info("Redis keyspace", "addr", cfg.redisAddr, "db", cfg.redisDB, + "keyPrefix", redisStore.KeyPrefix()) redisGate = newRedisReadinessGate(redisStore) fatalIfErr(mgr.Add(redisGate), "unable to add redis readiness gate") @@ -346,6 +349,7 @@ type appConfig struct { redisUsername string redisPassword string redisDB int + redisKeyPrefix string redisInsecure bool authorAttribution bool attributionFactTTL time.Duration @@ -433,6 +437,12 @@ func parseFlagsWithArgs(fs *flag.FlagSet, args []string) (appConfig, error) { "Redis password. Prefer setting via REDIS_PASSWORD env var from a Secret.", ) fs.IntVar(&cfg.redisDB, "redis-db", 0, "Redis database index (default 0).") + fs.StringVar(&cfg.redisKeyPrefix, "redis-key-prefix", queue.DefaultKeyPrefix, + "Root namespace for every key this operator writes to Redis/Valkey (cursors, attribution "+ + "facts, command author records). Give each reverser its own prefix to share one "+ + "Redis/Valkey between more than the 16 logical databases --redis-db can separate. "+ + "Allowed characters are [A-Za-z0-9], '-', '_', '.' and ':'; a Valkey ACL can enforce "+ + "the prefix (~:*) rather than trust it.") fs.BoolVar(&cfg.redisInsecure, "redis-insecure", false, "Connect to Redis over plain TCP instead of TLS (default false; TLS). Redis carries each "+ "GitTarget's watch cursors and, when attribution is on, the audit facts — prefer TLS. Set "+ @@ -476,6 +486,13 @@ func parseFlagsWithArgs(fs *flag.FlagSet, args []string) (appConfig, error) { return appConfig{}, err } cfg.redisAddr = strings.TrimSpace(cfg.redisAddr) + // Validated even when redis-addr is empty: a typo'd prefix is a configuration error + // whether or not this run happens to open the connection it names. + normalizedKeyPrefix, err := queue.ValidateKeyPrefix(cfg.redisKeyPrefix) + if err != nil { + return appConfig{}, err + } + cfg.redisKeyPrefix = normalizedKeyPrefix if err := validateAuditConfig(cfg); err != nil { return appConfig{}, err } diff --git a/cmd/main_audit_server_test.go b/cmd/main_audit_server_test.go index c6e48019..b05c6922 100644 --- a/cmd/main_audit_server_test.go +++ b/cmd/main_audit_server_test.go @@ -21,6 +21,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/ConfigButler/gitops-reverser/internal/queue" ) func TestParseFlagsWithArgs_Defaults(t *testing.T) { @@ -97,6 +99,7 @@ func TestParseFlagsWithArgs_CustomAuditValues(t *testing.T) { "--redis-username=user", "--redis-password=pass", "--redis-db=2", + "--redis-key-prefix=cell-a:tenant-7:", "--redis-insecure", "--author-attribution-ttl=20m", "--author-attribution-grace=750ms", @@ -119,11 +122,37 @@ func TestParseFlagsWithArgs_CustomAuditValues(t *testing.T) { assert.Equal(t, "user", cfg.redisUsername) assert.Equal(t, "pass", cfg.redisPassword) assert.Equal(t, 2, cfg.redisDB) + // The trailing colon is normalized away, so "tenant-7:" and "tenant-7" name one keyspace. + assert.Equal(t, "cell-a:tenant-7", cfg.redisKeyPrefix) assert.True(t, cfg.redisInsecure) assert.Equal(t, 20*time.Minute, cfg.attributionFactTTL) assert.Equal(t, 750*time.Millisecond, cfg.attributionGrace) } +func TestParseFlagsWithArgs_RedisKeyPrefixDefault(t *testing.T) { + fs := flag.NewFlagSet("test-redis-key-prefix-default", flag.ContinueOnError) + cfg, err := parseFlagsWithArgs(fs, []string{"--author-attribution=false"}) + require.NoError(t, err) + // An upgrade that sets nothing must keep writing the keys the previous release wrote. + assert.Equal(t, queue.DefaultKeyPrefix, cfg.redisKeyPrefix) +} + +func TestParseFlagsWithArgs_RedisKeyPrefixRejectsGlob(t *testing.T) { + fs := flag.NewFlagSet("test-redis-key-prefix-glob", flag.ContinueOnError) + // A glob metacharacter would corrupt the attribution fact-index SCAN pattern. + _, err := parseFlagsWithArgs(fs, []string{"--redis-key-prefix=tenant-*"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "redis-key-prefix") +} + +func TestParseFlagsWithArgs_RedisKeyPrefixRejectsEmpty(t *testing.T) { + fs := flag.NewFlagSet("test-redis-key-prefix-empty", flag.ContinueOnError) + // Rejected even without Redis: an empty prefix silently un-namespaces the keyspace. + _, err := parseFlagsWithArgs(fs, []string{"--author-attribution=false", "--redis-addr=", "--redis-key-prefix="}) + require.Error(t, err) + assert.Contains(t, err.Error(), "non-empty") +} + func TestParseFlagsWithArgs_RedisAddrRequiredWhenAttributionEnabled(t *testing.T) { fs := flag.NewFlagSet("test-redis-required", flag.ContinueOnError) // Default --author-attribution=true, so an empty redis-addr must be rejected. diff --git a/cmd/main_chart_args_test.go b/cmd/main_chart_args_test.go new file mode 100644 index 00000000..fcd6634b --- /dev/null +++ b/cmd/main_chart_args_test.go @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "flag" + "os" + "os/exec" + "testing" + + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + "sigs.k8s.io/yaml" +) + +// chartInputs are read by the test process itself so `go test` records them as cache +// inputs. helm reads the chart in a subprocess, which the test log cannot see, so without +// these reads a chart-only edit would replay a cached PASS. +var chartInputs = []string{ + "../charts/gitops-reverser/values.yaml", + "../charts/gitops-reverser/templates/deployment.yaml", +} + +// renderManagerArgs renders the chart's Deployment and returns the manager container's +// argv, exactly as the kubelet would hand it to the binary. +func renderManagerArgs(t *testing.T, setValues ...string) []string { + t.Helper() + + for _, path := range chartInputs { + _, err := os.ReadFile(path) + require.NoError(t, err) + } + + args := []string{ + "template", "gitops-reverser", "../charts/gitops-reverser", + "--show-only", "templates/deployment.yaml", + } + for _, sv := range setValues { + args = append(args, "--set", sv) + } + out, err := exec.Command("helm", args...).CombinedOutput() + require.NoErrorf(t, err, "helm template failed: %s", out) + + var deploy appsv1.Deployment + require.NoError(t, yaml.Unmarshal(out, &deploy)) + + for _, c := range deploy.Spec.Template.Spec.Containers { + if c.Name == "manager" { + return c.Args + } + } + t.Fatalf("no manager container in rendered Deployment:\n%s", out) + return nil +} + +// The chart writes the manager's argv and the binary parses it; nothing else enforces that +// the two agree. A value interpolated as a bare `{{ . | quote }}` inside a YAML plain scalar +// keeps its double quotes as literal characters, so the flag arrives with them attached and +// a validating flag rejects it — the manager then CrashLoopBackOffs on a chart that renders, +// lints and packages cleanly. Parse what the chart actually emits. +func TestChartRendersArgsTheBinaryAccepts(t *testing.T) { + tests := map[string]struct { + setValues []string + wantKeyPrefix string + wantRedisAddr string + wantRedisDBSet int + }{ + "chart defaults": { + wantKeyPrefix: "gitops-reverser", + }, + "nested tenant prefix": { + setValues: []string{"queue.redis.keyPrefix=cell-a:tenant-7"}, + wantKeyPrefix: "cell-a:tenant-7", + }, + "redis enabled with auth": { + setValues: []string{ + "queue.redis.addr=redis.example.com:6379", + "queue.redis.auth.username=reverser", + "queue.redis.db=3", + "queue.redis.keyPrefix=tenant-7", + }, + wantKeyPrefix: "tenant-7", + wantRedisAddr: "redis.example.com:6379", + wantRedisDBSet: 3, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + argv := renderManagerArgs(t, tc.setValues...) + + cfg, err := parseFlagsWithArgs(flag.NewFlagSet("manager", flag.ContinueOnError), argv) + require.NoError(t, err, "the binary must accept the argv the chart renders") + + require.Equal(t, tc.wantKeyPrefix, cfg.redisKeyPrefix) + if tc.wantRedisAddr != "" { + require.Equal(t, tc.wantRedisAddr, cfg.redisAddr) + require.Equal(t, tc.wantRedisDBSet, cfg.redisDB) + } + }) + } +} diff --git a/internal/queue/attribution_index.go b/internal/queue/attribution_index.go index 187421d6..9f9063a9 100644 --- a/internal/queue/attribution_index.go +++ b/internal/queue/attribution_index.go @@ -28,8 +28,10 @@ import ( // from one that never arrived. Configurable via --author-attribution-ttl. const DefaultAttributionFactTTL = 15 * time.Minute -// keyPrefix is the fixed root namespace for every Redis key (cursors and facts alike). -const keyPrefix = "gitops-reverser" +// DefaultKeyPrefix is the root namespace every Redis key (cursors, facts, and command +// author records alike) carries when --redis-key-prefix is not set. It is also the value +// every release before the flag existed used, so the default is a no-op upgrade. +const DefaultKeyPrefix = "gitops-reverser" const ( // attributionKeySuffix namespaces audit-sourced resource author facts under the @@ -105,6 +107,10 @@ type AuthorResolution struct { type AttributionIndex struct { client *redis.Client factTTL time.Duration + // keyPrefix is the root namespace this index writes and reads under, shared with the + // RedisStore that built it. Empty only in tests constructed by hand; every key builder + // resolves it so an empty value still lands on DefaultKeyPrefix. + keyPrefix string } // RecordFact stores the attribution fact for one accepted, mutating audit event. A @@ -406,7 +412,7 @@ func attributionResultForFact(fact AuthorFact, weak bool) AttributionResult { // factKeyBase is the per-type prefix shared by every fact key, e.g. // "gitops-reverser:author:v1:audit:apps/deployments". func (a *AttributionIndex) factKeyBase(gr string) string { - return keyPrefix + attributionKeySuffix + gr + return resolveKeyPrefix(a.keyPrefix) + attributionKeySuffix + gr } // factKeyExact is the immutable per-write fact key, e.g. @@ -447,7 +453,7 @@ func (a *AttributionIndex) recordFactIndexSize(ctx context.Context) { } var cursor uint64 var count int64 - pattern := keyPrefix + attributionKeySuffix + "*" + pattern := resolveKeyPrefix(a.keyPrefix) + attributionKeySuffix + "*" for { keys, next, err := a.client.Scan(ctx, cursor, pattern, attributionFactScanBatchSize).Result() if err != nil { diff --git a/internal/queue/command_author_store.go b/internal/queue/command_author_store.go index 747f2374..ad012b48 100644 --- a/internal/queue/command_author_store.go +++ b/internal/queue/command_author_store.go @@ -42,6 +42,8 @@ type CommandAuthor struct { type CommandAuthorStore struct { client *redis.Client ttl time.Duration + // keyPrefix is the root namespace shared with the RedisStore that built this store. + keyPrefix string } // RecordCommandAuthor is the admission-side write: capture the authenticated submitter @@ -78,5 +80,5 @@ func (s *CommandAuthorStore) LookupCommandAuthor( // key identifies the command by UID alone — globally unique, like the watch cursor key, // so namespace/name/kind would be redundant (kept out for a tight key). func (s *CommandAuthorStore) key(uid types.UID) string { - return keyPrefix + commandAuthorKeySuffix + escapeKeyField(string(uid)) + return resolveKeyPrefix(s.keyPrefix) + commandAuthorKeySuffix + escapeKeyField(string(uid)) } diff --git a/internal/queue/key_prefix.go b/internal/queue/key_prefix.go new file mode 100644 index 00000000..d2e5bd11 --- /dev/null +++ b/internal/queue/key_prefix.go @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 + +package queue + +import ( + "fmt" + "strings" +) + +// maxKeyPrefixLength bounds the root namespace. Redis keys are capped at 512MB, so this +// is not a protocol limit — it is a legibility limit, and a guard against a prefix that +// dwarfs the key it namespaces. +const maxKeyPrefixLength = 128 + +// ValidateKeyPrefix checks a --redis-key-prefix value and returns its normalized form. +// +// Two independent constraints shape the allowed character set: +// +// - The attribution telemetry gauge SCANs ":author:v1:audit:*". Redis glob +// metacharacters (*, ?, [, ], \) in the prefix would silently make that pattern match +// the wrong keyspace, so they are rejected rather than escaped — a prefix is an +// operator-chosen identifier, not user data. +// - Key fields (uid, resourceVersion, namespace) are ':'-delimited and %-escaped by +// escapeKeyField. '%' in the prefix would make an escaped key ambiguous with an +// unescaped one, so it is rejected too. +// +// ':' is allowed, because a prefix like "cell-a:tenant-7" is a natural nesting and every +// suffix constant already begins with ':'. A trailing ':' is normalized away so +// "tenant-7:" and "tenant-7" name the same keyspace rather than two that differ by an +// empty segment. +// +// An empty prefix is rejected: an unprefixed keyspace collides with Redis's own key +// namespace conventions and, more importantly, silently un-namespaces an install that +// meant to set the flag and passed the empty string. Use DefaultKeyPrefix to opt out. +func ValidateKeyPrefix(prefix string) (string, error) { + normalized := strings.TrimRight(strings.TrimSpace(prefix), ":") + if normalized == "" { + return "", fmt.Errorf("redis-key-prefix must be a non-empty identifier (default %q)", DefaultKeyPrefix) + } + if len(normalized) > maxKeyPrefixLength { + return "", fmt.Errorf("redis-key-prefix must be at most %d characters, got %d", + maxKeyPrefixLength, len(normalized)) + } + for _, r := range normalized { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case r == '-', r == '_', r == '.', r == ':': + default: + return "", fmt.Errorf( + "redis-key-prefix %q contains %q; allowed characters are [A-Za-z0-9], '-', '_', '.' and ':'", + prefix, string(r)) + } + } + return normalized, nil +} + +// resolveKeyPrefix normalizes a prefix for internal use, falling back to the default when +// empty. Construction paths that did not go through ValidateKeyPrefix (tests, and the +// zero-value RedisStoreConfig) land here rather than building keys with an empty root. +func resolveKeyPrefix(prefix string) string { + normalized := strings.TrimRight(strings.TrimSpace(prefix), ":") + if normalized == "" { + return DefaultKeyPrefix + } + return normalized +} diff --git a/internal/queue/key_prefix_test.go b/internal/queue/key_prefix_test.go new file mode 100644 index 00000000..44d1ae2f --- /dev/null +++ b/internal/queue/key_prefix_test.go @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: Apache-2.0 + +package queue + +import ( + "context" + "strings" + "testing" + + "github.com/alicebob/miniredis/v2" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func TestValidateKeyPrefix(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + in string + want string + wantErr string + }{ + "default": {in: DefaultKeyPrefix, want: "gitops-reverser"}, + "nested with colon": {in: "cell-a:tenant-7", want: "cell-a:tenant-7"}, + "trailing colon dropped": {in: "tenant-7:", want: "tenant-7"}, + "surrounding space": {in: " tenant-7 ", want: "tenant-7"}, + "dots and underscores": {in: "rev_1.example.com", want: "rev_1.example.com"}, + + "empty": {in: "", wantErr: "non-empty"}, + "only colons": {in: ":::", wantErr: "non-empty"}, + "glob star": {in: "tenant-*", wantErr: `contains "*"`}, + "glob question": {in: "tenant-?", wantErr: `contains "?"`}, + "glob bracket": {in: "tenant-[a]", wantErr: `contains "["`}, + "escape char": {in: `tenant\a`, wantErr: `contains "\\"`}, + "percent": {in: "tenant%3A", wantErr: `contains "%"`}, + "slash": {in: "tenant/a", wantErr: `contains "/"`}, + "space inside": {in: "tenant a", wantErr: `contains " "`}, + "too long": {in: strings.Repeat("a", maxKeyPrefixLength+1), wantErr: "at most 128 characters"}, + "exactly max long": { + in: strings.Repeat("a", maxKeyPrefixLength), + want: strings.Repeat("a", maxKeyPrefixLength), + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + got, err := ValidateKeyPrefix(tc.in) + if tc.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +func TestResolveKeyPrefix_FallsBackToDefault(t *testing.T) { + t.Parallel() + // A hand-built store (tests, zero-value config) must never write to an unprefixed + // keyspace — that would be indistinguishable from another tool's keys. + require.Equal(t, DefaultKeyPrefix, resolveKeyPrefix("")) + require.Equal(t, DefaultKeyPrefix, resolveKeyPrefix(" ")) + require.Equal(t, "tenant-7", resolveKeyPrefix("tenant-7:")) +} + +// newPrefixedRedisStore builds a store on a fresh miniredis under an explicit prefix. +func newPrefixedRedisStore(t *testing.T, prefix string) *RedisStore { + t.Helper() + mr := miniredis.RunT(t) + store, err := NewRedisStore(RedisStoreConfig{Addr: mr.Addr(), KeyPrefix: prefix}) + require.NoError(t, err) + return store +} + +func TestRedisStore_KeyPrefixReachesEveryKeyFamily(t *testing.T) { + t.Parallel() + + store := newPrefixedRedisStore(t, "cell-a:tenant-7") + require.Equal(t, "cell-a:tenant-7", store.KeyPrefix()) + + gvr := schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"} + require.Equal(t, + "cell-a:tenant-7:watch:v1:target:gtuid-3:apps/deployments:namespace:team-a:last-rv", + store.watchCursorKey("gtuid-3", gvr, "team-a")) + + idx := store.AttributionIndex(0) + require.Equal(t, "cell-a:tenant-7:author:v1:audit:apps/deployments:object:uid-1:101", + idx.factKeyExact("apps/deployments", "uid-1", "101")) + require.Equal(t, "cell-a:tenant-7:author:v1:audit:apps/deployments:object:uid-1:last", + idx.factKeyLast("apps/deployments", "uid-1")) + require.Equal(t, "cell-a:tenant-7:author:v1:audit:apps/deployments:rv:101", + idx.factKeyRV("apps/deployments", "101")) + + require.Equal(t, "cell-a:tenant-7:author:v1:command:cr-uid", store.CommandAuthorStore().key("cr-uid")) +} + +func TestRedisStore_DefaultPrefixIsUnchanged(t *testing.T) { + t.Parallel() + + // The flag defaults to DefaultKeyPrefix, so an upgrade must not orphan the keys a + // previous release wrote. + store := newPrefixedRedisStore(t, DefaultKeyPrefix) + require.Equal(t, "gitops-reverser:watch:v1:target:gtuid-3:configmaps:cluster:last-rv", + store.watchCursorKey("gtuid-3", coreConfigmapsGVR(), "")) + require.Equal(t, "gitops-reverser:author:v1:command:cr-uid", store.CommandAuthorStore().key("cr-uid")) +} + +// Two reversers sharing one Redis/Valkey and one logical database must not read each +// other's cursors. Sixteen databases is the only separator Redis offers; a prefix is not +// bounded that way. +func TestRedisStore_DistinctPrefixesIsolateCursors(t *testing.T) { + t.Parallel() + + mr := miniredis.RunT(t) + ctx := context.Background() + gvr := coreConfigmapsGVR() + + tenantA, err := NewRedisStore(RedisStoreConfig{Addr: mr.Addr(), KeyPrefix: "tenant-a"}) + require.NoError(t, err) + tenantB, err := NewRedisStore(RedisStoreConfig{Addr: mr.Addr(), KeyPrefix: "tenant-b"}) + require.NoError(t, err) + + require.NoError(t, tenantA.RecordWatchCursor(ctx, "same-uid", gvr, "", "111")) + + rv, ok := tenantA.LookupWatchCursor(ctx, "same-uid", gvr, "") + require.True(t, ok) + require.Equal(t, "111", rv) + + _, ok = tenantB.LookupWatchCursor(ctx, "same-uid", gvr, "") + require.False(t, ok, "tenant-b must not see tenant-a's cursor on the same UID and database") + + require.NoError(t, tenantB.RecordWatchCursor(ctx, "same-uid", gvr, "", "222")) + rv, ok = tenantA.LookupWatchCursor(ctx, "same-uid", gvr, "") + require.True(t, ok) + require.Equal(t, "111", rv, "tenant-b's write must not clobber tenant-a's cursor") +} + +// The attribution telemetry gauge SCANs ":author:v1:audit:*". A prefix that +// contained a glob metacharacter would make it count the wrong keyspace; validation +// rejects those, so the pattern is always a literal prefix plus one trailing star. +func TestAttributionIndex_ScanPatternIsPrefixed(t *testing.T) { + t.Parallel() + + store := newPrefixedRedisStore(t, "tenant-a") + idx := store.AttributionIndex(0) + require.Equal(t, "tenant-a:author:v1:audit:", idx.factKeyBase("")) +} diff --git a/internal/queue/redis_store.go b/internal/queue/redis_store.go index 81e4b264..cd369ff7 100644 --- a/internal/queue/redis_store.go +++ b/internal/queue/redis_store.go @@ -34,6 +34,11 @@ type RedisStoreConfig struct { AuthValue string DB int TLSEnabled bool + // KeyPrefix is the root namespace for every key this connection's stores own. Empty + // means DefaultKeyPrefix. Several reversers can then share one Redis/Valkey without + // sharing a logical database — Redis offers only 16 of those, and --redis-db was the + // only separator this operator provided. + KeyPrefix string } // RedisStore is the required Redis/Valkey-backed store. It owns the connection and @@ -42,7 +47,8 @@ type RedisStoreConfig struct { // and knows nothing about attribution: author attribution is an optional layer built // on the same connection via AttributionIndex. type RedisStore struct { - client *redis.Client + client *redis.Client + keyPrefix string } // NewRedisStore opens the Redis/Valkey connection that backs the resume cursors. @@ -61,7 +67,13 @@ func NewRedisStore(cfg RedisStoreConfig) (*RedisStore, error) { options.TLSConfig = &tls.Config{MinVersion: tls.VersionTLS12} } - return &RedisStore{client: redis.NewClient(options)}, nil + return &RedisStore{client: redis.NewClient(options), keyPrefix: resolveKeyPrefix(cfg.KeyPrefix)}, nil +} + +// KeyPrefix returns the root namespace every key this store's family owns is written +// under. Reported at startup so an operator can confirm which keyspace a pod claims. +func (s *RedisStore) KeyPrefix() string { + return s.keyPrefix } // AttributionIndex builds the optional author-attribution fact index on this store's @@ -71,7 +83,7 @@ func (s *RedisStore) AttributionIndex(factTTL time.Duration) *AttributionIndex { if factTTL <= 0 { factTTL = DefaultAttributionFactTTL } - return &AttributionIndex{client: s.client, factTTL: factTTL} + return &AttributionIndex{client: s.client, factTTL: factTTL, keyPrefix: s.keyPrefix} } // CommandAuthorStore builds the command-authorship store on this connection. Wire it @@ -79,7 +91,7 @@ func (s *RedisStore) AttributionIndex(factTTL time.Duration) *AttributionIndex { // record lives in the same top-level author domain as audit facts but in the separate // command subfamily, with its own fixed cleanup TTL. func (s *RedisStore) CommandAuthorStore() *CommandAuthorStore { - return &CommandAuthorStore{client: s.client, ttl: commandAuthorRecordTTL} + return &CommandAuthorStore{client: s.client, ttl: commandAuthorRecordTTL, keyPrefix: s.keyPrefix} } // Ping checks liveness of the underlying Redis/Valkey connection. The readiness gate @@ -138,6 +150,6 @@ func (s *RedisStore) watchCursorKey( if namespace != "" { scope = "namespace:" + escapeKeyField(namespace) } - return keyPrefix + watchCursorKeySuffix + "target:" + escapeKeyField(gitTargetUID) + ":" + + return s.keyPrefix + watchCursorKeySuffix + "target:" + escapeKeyField(gitTargetUID) + ":" + groupResourceKey(gvr.Group, gvr.Resource) + ":" + scope + ":last-rv" } From cd48a4c52953eeac0f43cc68150f0f14dbe9983a Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Thu, 9 Jul 2026 21:16:50 +0000 Subject: [PATCH 3/6] feat(pkg): publish pkg/manifestanalyzer as the stable adoption contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The acceptance gate that decides "may this folder become a GitTarget?" is the same one the writer enforces before it commits a byte — which makes it the authoritative answer for an onboarding wizard. But Go forbids importing another module's internal/ tree, so a downstream tool could only exec the binary and parse a CLI output that carried no stability promise, or reimplement the rules and guarantee drift from the writer. pkg/manifestanalyzer publishes two entry points — ScanFolder (may this folder be adopted, and if not, why) and ScanRepo (which folders in this repository could be) — over its own JSON-tagged types carrying a SchemaVersion. It delegates every decision to internal/manifestanalyzer, so it cannot disagree with the operator; the public shape is a copy rather than a type alias, so the engine stays free to move. The CLI's --format json for both modes now goes through this package, which is what keeps the two contracts identical. That drops `plan` from scan-mode JSON (structurally always empty there — no cluster state, no desired resources) and adds schemaVersion; the internal RenderScanJSON/RenderRepoJSON are retired. Documented in docs/UPGRADING.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/manifest-analyzer/main.go | 56 ++++-- docs/UPGRADING.md | 27 +++ .../f8-repo-discovery-and-onboarding-scan.md | 5 + internal/manifestanalyzer/render.go | 100 ---------- internal/manifestanalyzer/render_test.go | 83 -------- internal/manifestanalyzer/repowalk.go | 9 - internal/manifestanalyzer/repowalk_test.go | 8 +- pkg/manifestanalyzer/doc.go | 34 ++++ pkg/manifestanalyzer/folder.go | 188 ++++++++++++++++++ pkg/manifestanalyzer/folder_test.go | 176 ++++++++++++++++ pkg/manifestanalyzer/repo.go | 179 +++++++++++++++++ pkg/manifestanalyzer/repo_test.go | 168 ++++++++++++++++ 12 files changed, 821 insertions(+), 212 deletions(-) create mode 100644 pkg/manifestanalyzer/doc.go create mode 100644 pkg/manifestanalyzer/folder.go create mode 100644 pkg/manifestanalyzer/folder_test.go create mode 100644 pkg/manifestanalyzer/repo.go create mode 100644 pkg/manifestanalyzer/repo_test.go diff --git a/cmd/manifest-analyzer/main.go b/cmd/manifest-analyzer/main.go index 71d64399..f7f912fb 100644 --- a/cmd/manifest-analyzer/main.go +++ b/cmd/manifest-analyzer/main.go @@ -49,6 +49,7 @@ import ( "k8s.io/client-go/tools/clientcmd" "github.com/ConfigButler/gitops-reverser/internal/manifestanalyzer" + publicanalyzer "github.com/ConfigButler/gitops-reverser/pkg/manifestanalyzer" ) // Process exit codes. @@ -237,7 +238,24 @@ func failedGroupVersions(failed map[schema.GroupVersion]error) map[string]string // refuse policy over the acceptance decision. It is structure-only: no cluster // state, so the plan is empty, but the acceptance gate is the full one — it applies // the non-API KRM allowlist and the impure-managed-file / mixed-file refusals. +// +// --format json goes through pkg/manifestanalyzer, so the CLI's machine-readable output +// and the published Go contract are the same document and cannot drift. Text output stays +// on the internal renderer, which can show the plan the public report deliberately omits. func runScan(dir, format, policy string, stdout, stderr io.Writer) int { + if format == "json" { + report, err := publicanalyzer.ScanFolder(context.Background(), dir) + if err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return exitUsage + } + if err := report.WriteJSON(stdout); err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return exitUsage + } + return scanExitCode(policy, report.Accepted) + } + scanPolicy := manifestanalyzer.ScanPolicy{ Acceptance: manifestanalyzer.AcceptancePolicy{Allowlist: manifestanalyzer.DefaultAllowlist()}, } @@ -246,17 +264,12 @@ func runScan(dir, format, policy string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "error: %v\n", err) return exitUsage } + manifestanalyzer.RenderScanText(stdout, result) + return scanExitCode(policy, result.Acceptance.Accepted) +} - if format == "json" { - if err := manifestanalyzer.RenderScanJSON(stdout, result); err != nil { - fmt.Fprintf(stderr, "error: %v\n", err) - return exitUsage - } - } else { - manifestanalyzer.RenderScanText(stdout, result) - } - - if policy == "refuse" && !result.Acceptance.Accepted { +func scanExitCode(policy string, accepted bool) int { + if policy == "refuse" && !accepted { return exitRefused } return exitOK @@ -268,19 +281,24 @@ func runScan(dir, format, policy string, stdout, stderr io.Writer) int { // (exitOK, or exitUsage on an I/O error); the repo-level --policy refuse gate is // deferred per the design doc. func runRepoWalk(root, format string, stdout, stderr io.Writer) int { - rep, err := manifestanalyzer.WalkRepo(context.Background(), root) - if err != nil { - fmt.Fprintf(stderr, "error: %v\n", err) - return exitUsage - } - if format == "json" { - if err := manifestanalyzer.RenderRepoJSON(stdout, rep); err != nil { + report, err := publicanalyzer.ScanRepo(context.Background(), root) + if err != nil { fmt.Fprintf(stderr, "error: %v\n", err) return exitUsage } - } else { - manifestanalyzer.RenderRepoText(stdout, rep) + if err := report.WriteJSON(stdout); err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return exitUsage + } + return exitOK + } + + rep, err := manifestanalyzer.WalkRepo(context.Background(), root) + if err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return exitUsage } + manifestanalyzer.RenderRepoText(stdout, rep) return exitOK } diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 781af79a..27fdd0bd 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -7,6 +7,33 @@ guidance that the changelog's breaking-change entries link to. We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with `bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it. +## Unreleased — `manifest-analyzer --format json` now emits a versioned contract (next minor; breaking) + +The analyzer's machine-readable output moved to the new public +[`pkg/manifestanalyzer`](../pkg/manifestanalyzer), which is a supported Go API and the single +definition of the JSON documents the CLI prints. Both affected modes gained a `schemaVersion` +field, and one field was dropped: + +- `--mode scan --format json` no longer carries `plan`. In scan mode the analyzer has no cluster + state and no desired resources, so `plan` was structurally always `{"counts":{},"actions":null}` — + it never carried information. The meaningful fields (`accepted`, `issues`, `retained`) are + unchanged, and `issues` now marshals as `[]` rather than `null` when there are none. +- `--mode repo-walker --format json` is otherwise unchanged. +- Retained entries now omit `identity` for a whole-file retention (an ordinary + `kustomization.yaml`, which names no resource) instead of emitting a zero-valued object. It is + still present for the refused mixed-file case, where a named resource hides inside a build + directive. +- `--mode analyze` and every `--format text` output are unchanged. + +**Migration** + +- Read `schemaVersion` and ignore fields you do not know; new fields are added within a schema major. +- If you exec'd the binary only to reach the acceptance verdict, prefer importing + `pkg/manifestanalyzer` and calling `ScanFolder` / `ScanRepo`. They run the same acceptance gate the + operator's writer enforces, so a tool built on them cannot drift from the operator that will later + adopt (or refuse) the folder. +- If you parsed `plan` from scan mode, you were reading an empty object; drop the field. + ## Unreleased — chart defaults now run Redis-free (next minor; behavior change) The Helm chart now defaults to the simple, Redis-free `configured-author` path, so a bare diff --git a/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md b/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md index 1019934e..21889d52 100644 --- a/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md +++ b/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md @@ -246,6 +246,11 @@ followed — the same posture as `ScanDir`, over the whole tree. It reuses `coll `parseKustomizations`/`renderRoots`, the `Scan`/acceptance gate, and mirrors `gittarget_path_overlap` for nesting. +A product layer consuming this from Go imports [`pkg/manifestanalyzer`](../../../pkg/manifestanalyzer) +— `ScanRepo` and `ScanFolder` — rather than exec'ing the binary. That package is the +supported, versioned projection of the reports below; the engine above stays internal and +free to move. See [../multi-tenant/README.md](../multi-tenant/README.md) item 4. + **Report shape as shipped**, refining the sketch above. Per candidate: `path`, `layout`, `acceptedByOperator`, `refusalReasons[]` (`{code, detail}`), `renderRoot`, `readScope[]`, `inferredNamespace`, `resources`, `overlapsWith[]`. diff --git a/internal/manifestanalyzer/render.go b/internal/manifestanalyzer/render.go index 913cca89..81853bd9 100644 --- a/internal/manifestanalyzer/render.go +++ b/internal/manifestanalyzer/render.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/ConfigButler/gitops-reverser/internal/git/manifestedit" - "github.com/ConfigButler/gitops-reverser/internal/types" ) // RenderJSON writes the report as indented JSON. It is the machine-readable form @@ -197,102 +196,3 @@ func planActionTarget(a PlanAction) string { } return fmt.Sprintf("%s#%d", a.Ref.FilePath, a.Ref.DocumentIndex) } - -// RenderScanJSON writes the scan as indented JSON: the machine-readable form shared -// by the CLI and the GitTarget status path. It omits the live desired objects, which -// are unbounded; it carries only the decided plan. -func RenderScanJSON(w io.Writer, result ScanResult) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(scanToJSON(result)) -} - -// scanJSON is the compact, bounded JSON projection of a ScanResult. -type scanJSON struct { - Accepted bool `json:"accepted"` - Issues []AcceptanceIssue `json:"issues"` - Retained []retainedJSON `json:"retained,omitempty"` - Plan planJSON `json:"plan"` -} - -type retainedJSON struct { - Path string `json:"path"` - DocumentIndex int `json:"documentIndex"` - Identity manifestedit.Identity `json:"identity"` -} - -type planJSON struct { - Counts map[string]int `json:"counts"` - Actions []planActionJSON `json:"actions"` - Diagnostics []manifestedit.Diagnostic `json:"diagnostics,omitempty"` -} - -type planActionJSON struct { - Kind string `json:"kind"` - Path string `json:"path,omitempty"` - // DocumentIndex is a pointer so that index 0 — a real, common target for a patch - // or drop on a file's first document — is preserved, while a create (which has no - // existing document location) omits it. A plain int with omitempty would drop the - // meaningful 0 and weaken the machine-readable contract. - DocumentIndex *int `json:"documentIndex,omitempty"` - Identity manifestedit.Identity `json:"identity"` - Resource string `json:"resource,omitempty"` - Reason string `json:"reason,omitempty"` -} - -// scanToJSON builds the compact JSON projection. -func scanToJSON(result ScanResult) scanJSON { - out := scanJSON{ - Accepted: result.Acceptance.Accepted, - Issues: result.Acceptance.Issues, - Plan: planJSON{ - Counts: planCounts(result.Plan), - Diagnostics: result.Plan.Diagnostics, - }, - } - for _, rd := range result.Acceptance.Retained { - out.Retained = append(out.Retained, retainedJSON{ - Path: rd.Location.Path, DocumentIndex: rd.Location.DocumentIndex, Identity: rd.Identity, - }) - } - for _, a := range result.Plan.Actions { - out.Plan.Actions = append(out.Plan.Actions, planActionJSON{ - Kind: string(a.Kind), - Path: planActionTarget(a), - DocumentIndex: documentIndexJSON(a), - Identity: a.Identity, - Resource: resourceString(a.Resource), - Reason: a.Reason, - }) - } - return out -} - -// documentIndexJSON returns the action's document index as a pointer: nil for a -// create (no existing document location, so the field is omitted), otherwise the -// real index — including a meaningful 0. -func documentIndexJSON(a PlanAction) *int { - if a.Kind == PlanCreate { - return nil - } - i := a.Ref.DocumentIndex - return &i -} - -// planCounts converts the plan's per-kind counts to string keys for JSON. -func planCounts(plan Plan) map[string]int { - out := map[string]int{} - for kind, n := range plan.Counts() { - out[string(kind)] = n - } - return out -} - -// resourceString renders a resolved resource identity, or "" when it is the zero -// value (a document a structure-only store never resolved). -func resourceString(r types.ResourceIdentifier) string { - if r == (types.ResourceIdentifier{}) { - return "" - } - return r.Key() -} diff --git a/internal/manifestanalyzer/render_test.go b/internal/manifestanalyzer/render_test.go index 7b8f1ce0..ce691830 100644 --- a/internal/manifestanalyzer/render_test.go +++ b/internal/manifestanalyzer/render_test.go @@ -146,86 +146,3 @@ func TestRenderScanText_Accepted(t *testing.T) { t.Errorf("expected no-changes plan line: %s", out) } } - -func TestRenderScanJSON(t *testing.T) { - var buf bytes.Buffer - if err := RenderScanJSON(&buf, scanWithRefusalAndPlan(t)); err != nil { - t.Fatalf("RenderScanJSON: %v", err) - } - - var parsed struct { - Accepted bool `json:"accepted"` - Issues []struct { - Kind string `json:"kind"` - } `json:"issues"` - Retained []struct { - Path string `json:"path"` - } `json:"retained"` - Plan struct { - Counts map[string]int `json:"counts"` - Actions []struct { - Kind string `json:"kind"` - Path string `json:"path"` - Resource string `json:"resource"` - } `json:"actions"` - } `json:"plan"` - } - if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { - t.Fatalf("scan JSON is invalid: %v\n%s", err, buf.String()) - } - if parsed.Accepted { - t.Errorf("scan JSON should report refusal") - } - if len(parsed.Issues) == 0 || len(parsed.Retained) != 1 { - t.Errorf("scan JSON issues=%d retained=%d", len(parsed.Issues), len(parsed.Retained)) - } - if parsed.Plan.Counts["create"] != 1 || parsed.Plan.Counts["patch"] != 1 { - t.Errorf("scan JSON plan counts = %+v", parsed.Plan.Counts) - } - if len(parsed.Plan.Actions) != 2 { - t.Errorf("scan JSON should carry both actions, got %+v", parsed.Plan.Actions) - } -} - -// TestRenderScanJSON_PreservesDocumentIndexZero guards the machine-readable contract: -// a patch on a file's first document must serialize documentIndex 0 (not omit it), -// while a create — which has no existing document location — omits it. -func TestRenderScanJSON_PreservesDocumentIndexZero(t *testing.T) { - fsys := fstest.MapFS{"deploy.yaml": {Data: []byte(deployYAML)}} - desired := []DesiredResource{desiredDeployWeb(3), desiredConfigMap("new")} // patch deploy#0 + create - result := Scan(context.Background(), fsys, snapMapper(), desired, ScanPolicy{}) - - var buf bytes.Buffer - if err := RenderScanJSON(&buf, result); err != nil { - t.Fatalf("RenderScanJSON: %v", err) - } - if !strings.Contains(buf.String(), `"documentIndex": 0`) { - t.Errorf("a patch on the first document must keep documentIndex 0:\n%s", buf.String()) - } - - var parsed struct { - Plan struct { - Actions []struct { - Kind string `json:"kind"` - DocumentIndex *int `json:"documentIndex"` - } `json:"actions"` - } `json:"plan"` - } - if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { - t.Fatalf("invalid JSON: %v", err) - } - for _, a := range parsed.Plan.Actions { - switch a.Kind { - case "patch": - if a.DocumentIndex == nil || *a.DocumentIndex != 0 { - t.Errorf("patch action should carry documentIndex 0, got %+v", a.DocumentIndex) - } - case "create": - if a.DocumentIndex != nil { - t.Errorf("create action should omit documentIndex, got %v", *a.DocumentIndex) - } - default: - t.Errorf("unexpected action kind %q", a.Kind) - } - } -} diff --git a/internal/manifestanalyzer/repowalk.go b/internal/manifestanalyzer/repowalk.go index 99cb3a55..8bae6af6 100644 --- a/internal/manifestanalyzer/repowalk.go +++ b/internal/manifestanalyzer/repowalk.go @@ -4,7 +4,6 @@ package manifestanalyzer import ( "context" - "encoding/json" "fmt" "io" "io/fs" @@ -711,14 +710,6 @@ func sortedKeysOf(m map[string]struct{}) []string { return out } -// RenderRepoJSON writes the repo report as indented JSON — the product's interface, -// matching the existing --format json convention. -func RenderRepoJSON(w io.Writer, rep RepoReport) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(rep) -} - // RenderRepoText writes a compact human summary of the repo report: one line per // candidate, then the roll-up. It is a convenience view; JSON is the contract. func RenderRepoText(w io.Writer, rep RepoReport) { diff --git a/internal/manifestanalyzer/repowalk_test.go b/internal/manifestanalyzer/repowalk_test.go index 3ee87410..926c1716 100644 --- a/internal/manifestanalyzer/repowalk_test.go +++ b/internal/manifestanalyzer/repowalk_test.go @@ -5,6 +5,7 @@ package manifestanalyzer import ( "bytes" "context" + "encoding/json" "os" "path/filepath" "testing" @@ -61,8 +62,13 @@ func checkGoldenFixture(t *testing.T, fixture string) { } rep.Root = "" + // The engine's own report shape, not the published one. pkg/manifestanalyzer owns the + // machine-readable contract and pins it separately; this corpus pins the classification + // facts (layout, refusal codes, overlaps, namespace inference, rendered/editable split). var buf bytes.Buffer - if err := RenderRepoJSON(&buf, rep); err != nil { + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + if err := enc.Encode(rep); err != nil { t.Fatalf("render json: %v", err) } diff --git a/pkg/manifestanalyzer/doc.go b/pkg/manifestanalyzer/doc.go new file mode 100644 index 00000000..64cf6439 --- /dev/null +++ b/pkg/manifestanalyzer/doc.go @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package manifestanalyzer is the public, stable answer to two questions a tool built +// around GitOps Reverser needs to ask about a Git repository, without a cluster and +// without writing anything: +// +// ScanFolder — may this folder become a GitTarget, and if not, why? +// ScanRepo — which folders in this repository could, and what shape is each one? +// +// The decisions come from the same acceptance gate the operator's writer enforces before +// it commits a byte, so a tool built on this package cannot drift from the operator that +// will later refuse the folder. Nothing here re-implements a rule. +// +// # Stability +// +// This package is the supported surface. The types below carry JSON tags and a +// [SchemaVersion]; fields are added, never repurposed or removed within a schema major. +// Everything under internal/ is not covered by that promise and is not importable from +// another module. +// +// The command-line equivalents are `manifest-analyzer --mode scan --format json` and +// `--mode repo-walker --format json`, which emit exactly the documents [FolderReport] +// and [RepoReport] marshal to. Exec the binary if Go is not your language; import this +// package if it is. +// +// # What it does not do +// +// Neither entry point resolves types against a live cluster, so neither reports whether a +// document's kind is actually served, nor produces a write plan. Both are structure-only: +// they read bytes, never follow symlinks, and never write. The operator applies the same +// gate plus the cluster-aware checks when a GitTarget adopts the folder — a folder this +// package accepts can still be refused for a reason only a cluster can see (an unresolved +// kind, an out-of-scope resource). +package manifestanalyzer diff --git a/pkg/manifestanalyzer/folder.go b/pkg/manifestanalyzer/folder.go new file mode 100644 index 00000000..e58a745d --- /dev/null +++ b/pkg/manifestanalyzer/folder.go @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: Apache-2.0 + +package manifestanalyzer + +import ( + "context" + "encoding/json" + "io" + "io/fs" + + internalanalyzer "github.com/ConfigButler/gitops-reverser/internal/manifestanalyzer" +) + +// SchemaVersion identifies the JSON contract [FolderReport] and [RepoReport] marshal to. +// It bumps only on a breaking change — a field removed, renamed, or given a new meaning. +// Adding a field is not breaking, so consumers must ignore fields they do not know. +const SchemaVersion = "v1" + +// IssueKind classifies why a folder was not accepted. The values are the operator's own +// refusal codes, stable across releases. +type IssueKind string + +const ( + // IssueDuplicate marks a document that duplicates an earlier manifest identity. + IssueDuplicate IssueKind = "duplicate-identity" + // IssueNonKRM marks YAML that does not parse as a Kubernetes manifest. + IssueNonKRM IssueKind = "non-krm-yaml" + // IssueInvalidYAML marks a document that does not parse as YAML. + IssueInvalidYAML IssueKind = "invalid-yaml" + // IssueImpureManagedFile marks a file holding managed resources that also holds a + // non-managed document. A managed file may contain only valid KRM documents. + IssueImpureManagedFile IssueKind = "impure-managed-file" + // IssueMixedFile marks a managed file that also holds an allowlisted non-API KRM + // document (a kustomization). Allowlisted KRM must be retained in its own file. + IssueMixedFile IssueKind = "mixed-managed-allowlisted" + // IssueUnresolvedKRM marks recognized KRM that cannot be tied to a single served, + // followable resource. Only a cluster-aware scan reports this. + IssueUnresolvedKRM IssueKind = "unresolved-krm" + // IssueOutOfScope marks a watched kind whose resource falls outside the GitTarget's + // scope (right kind, wrong namespace). Only a cluster-aware scan reports this. + IssueOutOfScope IssueKind = "out-of-scope" + // IssueUnsupportedKustomize marks a kustomization.yaml using a feature the writer + // cannot map back to editable source documents (generators, patches, components, + // Helm inflation, replacements, transformers, name prefixes, remote bases). + IssueUnsupportedKustomize IssueKind = "unsupported-kustomize" + // IssueForeignFile marks a non-YAML regular file the operator cannot manage. + IssueForeignFile IssueKind = "foreign-file" + // IssueForeignSymlink marks a symlink, which a writer could follow out of the subtree. + IssueForeignSymlink IssueKind = "foreign-symlink" + // IssueForeignSubmodule marks a nested Git submodule. + IssueForeignSubmodule IssueKind = "foreign-submodule" + // IssueIgnoreShadowsManaged marks a .gittargetignore pattern that matches a path the + // operator writes, which would blind it to its own file. + IssueIgnoreShadowsManaged IssueKind = "ignore-shadows-managed" + // IssueWriteEscapesScope marks a planned write that would leave the GitTarget's path. + IssueWriteEscapesScope IssueKind = "write-escapes-scope" + // IssueWriteFanIn marks an in-place edit of a source file that more than one kustomize + // render root reaches. + IssueWriteFanIn IssueKind = "write-fan-in" +) + +// Issue is one reason a folder is not accepted, or one fact a stricter policy may treat +// as blocking. +type Issue struct { + Kind IssueKind `json:"kind"` + // Path is the offending file, slash-separated and relative to the scan root. Empty + // when the issue is about the folder as a whole. + Path string `json:"path"` + // DocumentIndex is the zero-based index of the offending document within Path. + DocumentIndex int `json:"documentIndex"` + // Message is a human-readable explanation. It is not a stable string. + Message string `json:"message"` +} + +// Identity names one Kubernetes document as it appears in the file. +type Identity struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Namespace string `json:"namespace"` + Name string `json:"name"` +} + +// RetainedDocument is content the operator keeps but never writes: build directives such +// as a kustomization.yaml. They are read as context, never edited as resources. +type RetainedDocument struct { + Path string `json:"path"` + DocumentIndex int `json:"documentIndex"` + // Identity is set only when a named Kubernetes resource is hiding inside an + // allowlisted build-directive file — the mixed-file case, which is refused. It is nil + // for the ordinary whole-file retention of a kustomization.yaml, which names no + // resource. + Identity *Identity `json:"identity,omitempty"` + // Unsupported reports that this retained file uses a construct the writer cannot map + // back to editable source. It is the cause of an IssueUnsupportedKustomize. + Unsupported bool `json:"unsupported,omitempty"` +} + +// FolderReport answers "may this folder become a GitTarget?". +type FolderReport struct { + SchemaVersion string `json:"schemaVersion"` + // Root is the scanned folder as passed to ScanFolder. Informational; empty for + // ScanFolderFS. + Root string `json:"root,omitempty"` + // Accepted is the gate decision. When false, Issues says why. + Accepted bool `json:"accepted"` + // Issues is empty when Accepted, and never nil in the marshaled JSON. + Issues []Issue `json:"issues"` + // Retained lists the build directives read as context. + Retained []RetainedDocument `json:"retained,omitempty"` +} + +// ScanFolder runs the adoption gate over a folder on disk. It is read-only, needs no +// cluster, and never follows symlinks. +// +// The returned error covers only I/O: a folder that cannot be adopted is a successful +// scan with Accepted=false, not an error. +func ScanFolder(ctx context.Context, root string) (FolderReport, error) { + result, err := internalanalyzer.ScanDir(ctx, root, nil, nil, folderScanPolicy()) + if err != nil { + return FolderReport{}, err + } + report := folderReportFrom(result.Acceptance) + report.Root = root + return report, nil +} + +// ScanFolderFS is ScanFolder over an arbitrary [io/fs.FS] — an in-memory tree, a tarball, +// or a Git tree exposed as a filesystem. It cannot fail: an unreadable file becomes an +// issue, not an error. +func ScanFolderFS(ctx context.Context, fsys fs.FS) FolderReport { + return folderReportFrom(internalanalyzer.Scan(ctx, fsys, nil, nil, folderScanPolicy()).Acceptance) +} + +// WriteJSON writes the report as indented JSON — byte-for-byte what +// `manifest-analyzer --mode scan --format json` prints. +func (r FolderReport) WriteJSON(w io.Writer) error { + if r.Issues == nil { + r.Issues = []Issue{} + } + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(r) +} + +// folderScanPolicy is the structure-only adoption gate: the default allowlist, no +// followability lookup, and no desired state. It is the same policy the CLI's scan mode +// applies, kept in one place so the two can never disagree. +func folderScanPolicy() internalanalyzer.ScanPolicy { + return internalanalyzer.ScanPolicy{ + Acceptance: internalanalyzer.AcceptancePolicy{Allowlist: internalanalyzer.DefaultAllowlist()}, + } +} + +// folderReportFrom projects the internal acceptance decision onto the public contract. +// It is deliberately a copy rather than a type alias: the public shape must be free to +// stay still while the internal one moves. +func folderReportFrom(acc internalanalyzer.Acceptance) FolderReport { + report := FolderReport{ + SchemaVersion: SchemaVersion, + Accepted: acc.Accepted, + Issues: make([]Issue, 0, len(acc.Issues)), + } + for _, issue := range acc.Issues { + report.Issues = append(report.Issues, Issue{ + Kind: IssueKind(issue.Kind), + Path: issue.Path, + DocumentIndex: issue.DocumentIndex, + Message: issue.Message, + }) + } + for _, rd := range acc.Retained { + doc := RetainedDocument{ + Path: rd.Location.Path, + DocumentIndex: rd.Location.DocumentIndex, + Unsupported: rd.Unsupported, + } + if rd.Identity.Kind != "" { + doc.Identity = &Identity{ + APIVersion: rd.Identity.APIVersion, + Kind: rd.Identity.Kind, + Namespace: rd.Identity.Namespace, + Name: rd.Identity.Name, + } + } + report.Retained = append(report.Retained, doc) + } + return report +} diff --git a/pkg/manifestanalyzer/folder_test.go b/pkg/manifestanalyzer/folder_test.go new file mode 100644 index 00000000..fb343095 --- /dev/null +++ b/pkg/manifestanalyzer/folder_test.go @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: Apache-2.0 + +package manifestanalyzer_test + +import ( + "bytes" + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "testing/fstest" + + "github.com/stretchr/testify/require" + + "github.com/ConfigButler/gitops-reverser/pkg/manifestanalyzer" +) + +const configMapYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: settings + namespace: demo +data: + key: value +` + +const kustomizationHelmYAML = `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: + - name: podinfo + repo: https://stefanprodan.github.io/podinfo +` + +func TestScanFolder_AcceptsPlainKRM(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "cm.yaml"), []byte(configMapYAML), 0o600)) + + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + + require.True(t, report.Accepted) + require.Empty(t, report.Issues) + require.Equal(t, manifestanalyzer.SchemaVersion, report.SchemaVersion) + require.Equal(t, root, report.Root) +} + +// A folder the operator would refuse must come back as a successful scan with +// Accepted=false. Refusal is a verdict, never an error. +func TestScanFolder_RefusalIsNotAnError(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "cm.yaml"), []byte(configMapYAML), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(kustomizationHelmYAML), 0o600)) + + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + + require.False(t, report.Accepted) + require.NotEmpty(t, report.Issues) + kinds := make([]manifestanalyzer.IssueKind, 0, len(report.Issues)) + for _, issue := range report.Issues { + kinds = append(kinds, issue.Kind) + } + require.Contains(t, kinds, manifestanalyzer.IssueUnsupportedKustomize, + "Helm inflation is the permanent support boundary and must be named as such") +} + +func TestScanFolder_MissingDirIsAnError(t *testing.T) { + t.Parallel() + _, err := manifestanalyzer.ScanFolder(context.Background(), filepath.Join(t.TempDir(), "absent")) + require.Error(t, err) +} + +func TestScanFolderFS_ReportsDuplicateIdentity(t *testing.T) { + t.Parallel() + + fsys := fstest.MapFS{ + "a.yaml": {Data: []byte(configMapYAML)}, + "b.yaml": {Data: []byte(configMapYAML)}, + } + report := manifestanalyzer.ScanFolderFS(context.Background(), fsys) + + require.False(t, report.Accepted) + require.Equal(t, manifestanalyzer.SchemaVersion, report.SchemaVersion) + require.Empty(t, report.Root, "an fs.FS has no path to report") + + var found bool + for _, issue := range report.Issues { + if issue.Kind == manifestanalyzer.IssueDuplicate { + found = true + require.NotEmpty(t, issue.Path) + require.NotEmpty(t, issue.Message) + } + } + require.True(t, found, "two documents with the same identity must be refused as duplicates") +} + +// The retained set names the build directives the operator reads but never writes. A +// consumer relies on it to explain "we understand this kustomization; we will not edit it". +func TestScanFolder_ReportsRetainedKustomization(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "cm.yaml"), []byte(configMapYAML), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), + []byte("apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nresources:\n - cm.yaml\n"), 0o600)) + + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + require.True(t, report.Accepted) + require.Len(t, report.Retained, 1) + require.Equal(t, "kustomization.yaml", report.Retained[0].Path) + require.False(t, report.Retained[0].Unsupported) + require.Nil(t, report.Retained[0].Identity, + "a whole-file retention names no resource; only the refused mixed-file case does") +} + +// A kustomization whose feature set the writer cannot map back to source is retained and +// flagged, so a consumer can point at the exact file that made the folder unadoptable. +func TestScanFolder_FlagsUnsupportedRetainedKustomization(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(kustomizationHelmYAML), 0o600)) + + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + require.False(t, report.Accepted) + require.Len(t, report.Retained, 1) + require.True(t, report.Retained[0].Unsupported) +} + +// The JSON document is the published contract: schemaVersion is always present, and +// issues is an empty array rather than null so a consumer can iterate it unconditionally. +func TestFolderReport_WriteJSON_Contract(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "cm.yaml"), []byte(configMapYAML), 0o600)) + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, report.WriteJSON(&buf)) + + require.Contains(t, buf.String(), `"issues": []`, "issues must marshal as [] and never null") + + var raw map[string]any + require.NoError(t, json.Unmarshal(buf.Bytes(), &raw)) + require.Equal(t, "v1", raw["schemaVersion"]) + require.Equal(t, true, raw["accepted"]) + require.NotContains(t, raw, "retained", "an empty retained set is omitted") +} + +// The public report must survive a marshal/unmarshal round trip unchanged, or a consumer +// storing it and reading it back would see a different verdict than the one produced. +func TestFolderReport_JSONRoundTrip(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "cm.yaml"), []byte(configMapYAML), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(kustomizationHelmYAML), 0o600)) + report, err := manifestanalyzer.ScanFolder(context.Background(), root) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, report.WriteJSON(&buf)) + + var decoded manifestanalyzer.FolderReport + require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded)) + require.Equal(t, report, decoded) +} diff --git a/pkg/manifestanalyzer/repo.go b/pkg/manifestanalyzer/repo.go new file mode 100644 index 00000000..f4d22bca --- /dev/null +++ b/pkg/manifestanalyzer/repo.go @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: Apache-2.0 + +package manifestanalyzer + +import ( + "context" + "encoding/json" + "io" + + internalanalyzer "github.com/ConfigButler/gitops-reverser/internal/manifestanalyzer" +) + +// Layout is the structural shape of a candidate folder. +type Layout string + +const ( + // LayoutPlain is a directory of raw KRM documents with explicit namespaces and no + // kustomization. Accepted. + LayoutPlain Layout = "plain" + // LayoutKustomizeSingle is a self-contained render root: one kustomization whose + // resources graph stays within its own subtree. Accepted. + LayoutKustomizeSingle Layout = "kustomize-single" + // LayoutKustomizeOverlay is a render root reaching a base outside its own subtree + // (the classic base/ + overlays/{env} shape). Refused today, with a forward-looking + // reason: it becomes accepted when render-root scoping ships. + LayoutKustomizeOverlay Layout = "kustomize-overlay" + // LayoutRefusedStructural is a render root whose kustomization uses a construct the + // writer cannot map back to editable source. This is the permanent support boundary, + // never a "not yet". + LayoutRefusedStructural Layout = "refused-structural" +) + +// Refusal reason codes. The distinction is load-bearing and a consumer must not collapse +// it: one is a "not yet", the other is permanent. +const ( + // ReasonOverlayFanOutNeedsF2 is the forward-looking refusal that flips to accepted + // when render-root scoping ships. + ReasonOverlayFanOutNeedsF2 = "overlay-fan-out-needs-f2" + // ReasonRefusedStructural is the permanent support boundary. + ReasonRefusedStructural = "refused-structural" +) + +// RefusalReason is one machine-readable reason a candidate is not accepted. +type RefusalReason struct { + Code string `json:"code"` + // Detail is human-readable and not a stable string. + Detail string `json:"detail"` +} + +// ResourceCounts splits the KRM a candidate covers into what it renders versus what it +// could actually edit. The two are equal for a plain or self-contained candidate and +// diverge for an overlay, which renders documents it cannot own. +type ResourceCounts struct { + Rendered int `json:"rendered"` + Editable int `json:"editable"` + NonKRM int `json:"nonKrm"` +} + +// Candidate is one folder that could become a GitTarget. +type Candidate struct { + // Path is slash-separated and relative to the repository root. + Path string `json:"path"` + Layout Layout `json:"layout"` + // AcceptedByOperator reports whether the operator would adopt this folder today. + AcceptedByOperator bool `json:"acceptedByOperator"` + RefusalReasons []RefusalReason `json:"refusalReasons,omitempty"` + // RenderRoot reports whether the candidate is a kustomize render root. + RenderRoot bool `json:"renderRoot"` + // ReadScope lists base directories outside this candidate's subtree that its + // kustomization reads. Empty for plain and self-contained candidates. + ReadScope []string `json:"readScope,omitempty"` + // InferredNamespace is the namespace the candidate resolves to, when unambiguous. + InferredNamespace string `json:"inferredNamespace,omitempty"` + Resources ResourceCounts `json:"resources"` + // OverlapsWith lists candidate paths this one nests with. Two overlapping candidates + // can never both become GitTargets — a folder has exactly one owner. + OverlapsWith []string `json:"overlapsWith,omitempty"` +} + +// OverlapConflict records that Ancestor strictly contains Descendant. +type OverlapConflict struct { + Ancestor string `json:"ancestor"` + Descendant string `json:"descendant"` +} + +// RepoSummary is the repository-level roll-up. +type RepoSummary struct { + CandidatesByLayout map[Layout]int `json:"candidatesByLayout"` + Accepted int `json:"accepted"` + Refused int `json:"refused"` + OverlapConflicts []OverlapConflict `json:"overlapConflicts,omitempty"` + // FleetRoot reports that the repository root is a cluster/fleet root. A GitTarget + // points at an app subtree, never at such a root. + FleetRoot bool `json:"fleetRoot,omitempty"` + // UnsupportedConstructs is the sorted, de-duplicated set of unsupported kustomize + // features seen across refused candidates, so a tool can say "this repository uses + // Helm inflation, which the operator does not manage". + UnsupportedConstructs []string `json:"unsupportedConstructs,omitempty"` +} + +// RepoReport answers "which folders in this repository could become GitTargets?". +type RepoReport struct { + SchemaVersion string `json:"schemaVersion"` + // Root is the scanned repository root as passed to ScanRepo. Informational. + Root string `json:"root,omitempty"` + Candidates []Candidate `json:"candidates"` + Summary RepoSummary `json:"summary"` +} + +// ScanRepo walks a whole repository and enumerates the folders that could become +// GitTargets, classifying each one's layout and reporting why a folder is refused. It is +// read-only, needs no cluster, and never follows symlinks. +func ScanRepo(ctx context.Context, root string) (RepoReport, error) { + rep, err := internalanalyzer.WalkRepo(ctx, root) + if err != nil { + return RepoReport{}, err + } + return repoReportFrom(rep), nil +} + +// WriteJSON writes the report as indented JSON — byte-for-byte what +// `manifest-analyzer --mode repo-walker --format json` prints. +func (r RepoReport) WriteJSON(w io.Writer) error { + if r.Candidates == nil { + r.Candidates = []Candidate{} + } + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(r) +} + +// repoReportFrom projects the internal discovery report onto the public contract. +func repoReportFrom(rep internalanalyzer.RepoReport) RepoReport { + out := RepoReport{ + SchemaVersion: SchemaVersion, + Root: rep.Root, + Candidates: make([]Candidate, 0, len(rep.Candidates)), + Summary: RepoSummary{ + CandidatesByLayout: make(map[Layout]int, len(rep.Summary.CandidatesByLayout)), + Accepted: rep.Summary.Accepted, + Refused: rep.Summary.Refused, + FleetRoot: rep.Summary.FleetRoot, + UnsupportedConstructs: rep.Summary.UnsupportedConstructs, + }, + } + for layout, n := range rep.Summary.CandidatesByLayout { + out.Summary.CandidatesByLayout[Layout(layout)] = n + } + for _, conflict := range rep.Summary.OverlapConflicts { + out.Summary.OverlapConflicts = append(out.Summary.OverlapConflicts, OverlapConflict{ + Ancestor: conflict.Ancestor, Descendant: conflict.Descendant, + }) + } + for _, cand := range rep.Candidates { + out.Candidates = append(out.Candidates, candidateFrom(cand)) + } + return out +} + +func candidateFrom(cand internalanalyzer.RepoCandidate) Candidate { + out := Candidate{ + Path: cand.Path, + Layout: Layout(cand.Layout), + AcceptedByOperator: cand.AcceptedByOperator, + RenderRoot: cand.RenderRoot, + ReadScope: cand.ReadScope, + InferredNamespace: cand.InferredNamespace, + Resources: ResourceCounts{ + Rendered: cand.Resources.Rendered, + Editable: cand.Resources.Editable, + NonKRM: cand.Resources.NonKRM, + }, + OverlapsWith: cand.OverlapsWith, + } + for _, reason := range cand.RefusalReasons { + out.RefusalReasons = append(out.RefusalReasons, RefusalReason{Code: reason.Code, Detail: reason.Detail}) + } + return out +} diff --git a/pkg/manifestanalyzer/repo_test.go b/pkg/manifestanalyzer/repo_test.go new file mode 100644 index 00000000..9a3d63de --- /dev/null +++ b/pkg/manifestanalyzer/repo_test.go @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: Apache-2.0 + +package manifestanalyzer_test + +import ( + "bytes" + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ConfigButler/gitops-reverser/pkg/manifestanalyzer" +) + +// corpusRoot is the discovery corpus the engine's own golden tests drive. The public +// package reads the same fixtures rather than growing a second copy: the point of this +// package is that it cannot disagree with the engine, and a duplicated corpus is the +// first way that guarantee rots. Tests run with the package directory as cwd. +const corpusRoot = "../../internal/manifestanalyzer/testdata/repo-walker" + +func fixture(t *testing.T, group, name string) string { + t.Helper() + path := filepath.Join(corpusRoot, group, name) + info, err := os.Stat(path) + require.NoError(t, err, "discovery corpus fixture %s must exist", path) + require.True(t, info.IsDir()) + return path +} + +func TestScanRepo_PlainPerEnvironmentFoldersAreCandidates(t *testing.T) { + t.Parallel() + + report, err := manifestanalyzer.ScanRepo(context.Background(), fixture(t, "supported", "plain-per-env")) + require.NoError(t, err) + + require.Equal(t, manifestanalyzer.SchemaVersion, report.SchemaVersion) + require.NotEmpty(t, report.Candidates) + for _, cand := range report.Candidates { + require.Equal(t, manifestanalyzer.LayoutPlain, cand.Layout) + require.True(t, cand.AcceptedByOperator, "plain KRM folders are the launch layout") + require.Empty(t, cand.RefusalReasons) + require.False(t, cand.RenderRoot) + } + require.Equal(t, len(report.Candidates), report.Summary.Accepted) + require.Zero(t, report.Summary.Refused) +} + +// The two refusal codes mean different things and a consumer must be able to tell them +// apart: one is "not yet, when render-root scoping ships", the other is the permanent +// support boundary. Collapsing them would mislabel an onboardable repository as hopeless. +func TestScanRepo_RefusalCodesStayDistinct(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + group, name string + wantCode string + wantLayout manifestanalyzer.Layout + }{ + "overlay fan-out is a not-yet": { + group: "unsupported", name: "base-overlays", + wantCode: manifestanalyzer.ReasonOverlayFanOutNeedsF2, + wantLayout: manifestanalyzer.LayoutKustomizeOverlay, + }, + "helm inflation is permanent": { + group: "unsupported", name: "helm-inflation", + wantCode: manifestanalyzer.ReasonRefusedStructural, + wantLayout: manifestanalyzer.LayoutRefusedStructural, + }, + "unsupported kustomize is permanent": { + group: "unsupported", name: "unsupported-kustomize", + wantCode: manifestanalyzer.ReasonRefusedStructural, + wantLayout: manifestanalyzer.LayoutRefusedStructural, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + report, err := manifestanalyzer.ScanRepo(context.Background(), fixture(t, tc.group, tc.name)) + require.NoError(t, err) + + var seen bool + for _, cand := range report.Candidates { + if cand.Layout != tc.wantLayout { + continue + } + seen = true + require.False(t, cand.AcceptedByOperator) + codes := make([]string, 0, len(cand.RefusalReasons)) + for _, reason := range cand.RefusalReasons { + codes = append(codes, reason.Code) + } + require.Contains(t, codes, tc.wantCode) + } + require.True(t, seen, "expected at least one %s candidate", tc.wantLayout) + }) + } +} + +func TestScanRepo_ReportsOverlapConflicts(t *testing.T) { + t.Parallel() + + report, err := manifestanalyzer.ScanRepo(context.Background(), fixture(t, "supported", "overlapping")) + require.NoError(t, err) + + require.NotEmpty(t, report.Summary.OverlapConflicts, + "two nested candidates can never both own a folder; the conflict must surface") + for _, conflict := range report.Summary.OverlapConflicts { + require.NotEmpty(t, conflict.Ancestor) + require.NotEmpty(t, conflict.Descendant) + require.NotEqual(t, conflict.Ancestor, conflict.Descendant) + } +} + +func TestScanRepo_FleetRootIsNeverACandidate(t *testing.T) { + t.Parallel() + + root := fixture(t, "supported", "fleet-root") + report, err := manifestanalyzer.ScanRepo(context.Background(), root) + require.NoError(t, err) + + require.True(t, report.Summary.FleetRoot) + for _, cand := range report.Candidates { + require.NotEqual(t, ".", cand.Path, "a GitTarget points at an app subtree, never the fleet root") + } +} + +func TestScanRepo_MissingRootIsAnError(t *testing.T) { + t.Parallel() + _, err := manifestanalyzer.ScanRepo(context.Background(), filepath.Join(t.TempDir(), "absent")) + require.Error(t, err) +} + +func TestRepoReport_WriteJSON_Contract(t *testing.T) { + t.Parallel() + + report, err := manifestanalyzer.ScanRepo(context.Background(), fixture(t, "supported", "kustomize-single")) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, report.WriteJSON(&buf)) + + var raw map[string]any + require.NoError(t, json.Unmarshal(buf.Bytes(), &raw)) + require.Equal(t, "v1", raw["schemaVersion"]) + require.Contains(t, raw, "candidates") + require.Contains(t, raw, "summary") + + var decoded manifestanalyzer.RepoReport + require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded)) + require.Equal(t, report, decoded) +} + +// An empty repository still marshals candidates as an array, so a consumer can iterate it +// without a nil check. +func TestRepoReport_WriteJSON_EmptyCandidatesIsArray(t *testing.T) { + t.Parallel() + + report, err := manifestanalyzer.ScanRepo(context.Background(), t.TempDir()) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, report.WriteJSON(&buf)) + require.Contains(t, buf.String(), `"candidates": []`) +} From 1de3e0562ed2be5a38d1d85d46a9fa3776e4fd09 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Fri, 10 Jul 2026 06:37:37 +0000 Subject: [PATCH 4/6] fix(chart): quote the whole --redis-username arg, not its value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `- --redis-username={{ . | quote }}` renders a YAML plain scalar whose double quotes are literal characters, so the manager received `"reverser"` — quotes included — and would have failed Redis ACL auth. The same defect crashed the manager for --redis-key-prefix, which is validated; username is not, so it would only have surfaced as an authentication failure against a real Redis. Predates this branch and is unreachable at the chart defaults (username is empty, so `with` skips the arg), which is why no e2e leg caught it. Rendered now through the printf-then-quote form the chart already uses for --additional-sensitive-resources, and pinned by the chart-args test. Co-Authored-By: Claude Opus 4.8 (1M context) --- charts/gitops-reverser/templates/deployment.yaml | 2 +- cmd/main_chart_args_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/charts/gitops-reverser/templates/deployment.yaml b/charts/gitops-reverser/templates/deployment.yaml index 68e28e29..66a9a755 100644 --- a/charts/gitops-reverser/templates/deployment.yaml +++ b/charts/gitops-reverser/templates/deployment.yaml @@ -68,7 +68,7 @@ spec: {{- end }} - --redis-addr={{ .Values.queue.redis.addr }} {{- with .Values.queue.redis.auth.username }} - - --redis-username={{ . | quote }} + - {{ printf "--redis-username=%s" . | quote }} {{- end }} - --redis-db={{ .Values.queue.redis.db }} {{- with .Values.queue.redis.keyPrefix }} diff --git a/cmd/main_chart_args_test.go b/cmd/main_chart_args_test.go index fcd6634b..a4fc8867 100644 --- a/cmd/main_chart_args_test.go +++ b/cmd/main_chart_args_test.go @@ -64,6 +64,7 @@ func TestChartRendersArgsTheBinaryAccepts(t *testing.T) { wantKeyPrefix string wantRedisAddr string wantRedisDBSet int + wantUsername string }{ "chart defaults": { wantKeyPrefix: "gitops-reverser", @@ -82,6 +83,7 @@ func TestChartRendersArgsTheBinaryAccepts(t *testing.T) { wantKeyPrefix: "tenant-7", wantRedisAddr: "redis.example.com:6379", wantRedisDBSet: 3, + wantUsername: "reverser", }, } @@ -97,6 +99,10 @@ func TestChartRendersArgsTheBinaryAccepts(t *testing.T) { require.Equal(t, tc.wantRedisAddr, cfg.redisAddr) require.Equal(t, tc.wantRedisDBSet, cfg.redisDB) } + if tc.wantUsername != "" { + // A quoted-value render would reach Redis as `"reverser"` and fail ACL auth. + require.Equal(t, tc.wantUsername, cfg.redisUsername) + } }) } } From cb43d27b328df604c546559f07e4fdba54787d98 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Fri, 10 Jul 2026 06:37:59 +0000 Subject: [PATCH 5/6] chore(coverage): raise the unit baseline to 75.4% --- .coverage-baseline | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.coverage-baseline b/.coverage-baseline index 3c2d8494..add29667 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -75.3 +75.4 From f7499c15cc849072f3b226b3057c7da4f865fda2 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Fri, 10 Jul 2026 06:56:39 +0000 Subject: [PATCH 6/6] refactor(manifest-analyzer)!: name the scan modes after the question they answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pkg/manifestanalyzer freezes ScanFolder and ScanRepo as the supported contract, so this is the moment to make the CLI and the docs speak the same two nouns — folder and repo — before anyone depends on the old ones. --mode scan -> --mode scan-folder --mode repo-walker -> --mode scan-repo `repo-walker` named an internal traversal phase, not a contract: it promised "descend directories" where the job is really "classify this Git source for adoptable targets", and it left no room for the implementation to stop being a walker. A bare `scan` was fine while it was the only scan, and became asymmetric the moment a repo-level scan existed next to it. No back-compat aliases. Both retired names now exit 2 as usage errors rather than falling through to the default analyze mode, which is the only way a stale invocation fails loudly instead of silently analyzing something else. The contract is days old; carrying deprecated spellings costs more than it saves. Internal follows the same nouns: WalkRepo -> ScanRepo, walkRepoFS -> scanRepoFS, repowalk.go -> scan_repo.go, and the testdata corpus repo-walker/ -> scan-repo/. --mode analyze and --mode discovery are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/manifest-analyzer/main.go | 54 +++++++++-------- cmd/manifest-analyzer/main_test.go | 36 ++++++----- docs/UPGRADING.md | 36 +++++++---- .../f8-repo-discovery-and-onboarding-scan.md | 45 ++++++++------ ...granularity-and-cross-environment-edits.md | 6 +- ...mize-support-boundary-and-product-model.md | 2 +- .../{repowalk.go => scan_repo.go} | 14 ++--- .../{repowalk_test.go => scan_repo_test.go} | 60 +++++++++---------- internal/manifestanalyzer/store.go | 2 +- .../supported/fleet-root.golden.json | 0 .../fleet-root/apps/podinfo/deployment.yaml | 0 .../fleet-root/clusters/prod/flux-system.yaml | 0 .../infra/ingress-nginx/deployment.yaml | 0 .../helm-release-document.golden.json | 0 .../app/helmrelease.yaml | 0 .../supported/kustomize-single.golden.json | 0 .../kustomize-single/app/deployment.yaml | 0 .../kustomize-single/app/kustomization.yaml | 0 .../kustomize-single/app/service.yaml | 0 .../supported/no-krm.golden.json | 0 .../supported/no-krm/docs/README.md | 0 .../supported/no-krm/docs/notes.txt | 0 .../supported/overlapping.golden.json | 0 .../overlapping/apps/web/deployment.yaml | 0 .../overlapping/apps/web/extra/configmap.yaml | 0 .../supported/plain-per-env.golden.json | 0 .../apps/app/prod/deployment.yaml | 0 .../plain-per-env/apps/app/prod/service.yaml | 0 .../apps/app/test/deployment.yaml | 0 .../plain-per-env/apps/app/test/service.yaml | 0 .../unsupported/base-overlays.golden.json | 0 .../base-overlays/base/deployment.yaml | 0 .../base-overlays/base/kustomization.yaml | 0 .../base-overlays/base/service.yaml | 0 .../overlays/acc/kustomization.yaml | 0 .../overlays/prod/kustomization.yaml | 0 .../overlays/test/kustomization.yaml | 0 .../unsupported/helm-inflation.golden.json | 0 .../helm-inflation/app/kustomization.yaml | 0 .../overlay-nested-base.golden.json | 0 .../base/common/configmap.yaml | 0 .../base/common/kustomization.yaml | 0 .../overlay-nested-base/base/deployment.yaml | 0 .../base/kustomization.yaml | 0 .../overlays/test/kustomization.yaml | 0 .../overlay-parked-base.golden.json | 0 .../overlay-parked-base/base/deployment.yaml | 0 .../base/kustomization.yaml | 0 .../overlay-parked-base/base/parked.yaml | 0 .../overlays/test/kustomization.yaml | 0 .../unsupported/plain-nonkrm.golden.json | 0 .../plain-nonkrm/app/deployment.yaml | 0 .../unsupported/plain-nonkrm/app/values.yaml | 0 .../unsupported-kustomize.golden.json | 0 .../unsupported-kustomize/app/deployment.yaml | 0 .../app/kustomization.yaml | 0 pkg/manifestanalyzer/doc.go | 4 +- pkg/manifestanalyzer/folder.go | 4 +- pkg/manifestanalyzer/repo.go | 4 +- pkg/manifestanalyzer/repo_test.go | 2 +- 60 files changed, 150 insertions(+), 119 deletions(-) rename internal/manifestanalyzer/{repowalk.go => scan_repo.go} (98%) rename internal/manifestanalyzer/{repowalk_test.go => scan_repo_test.go} (79%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/fleet-root.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/fleet-root/apps/podinfo/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/fleet-root/clusters/prod/flux-system.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/fleet-root/infra/ingress-nginx/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/helm-release-document.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/helm-release-document/app/helmrelease.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/kustomize-single.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/kustomize-single/app/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/kustomize-single/app/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/kustomize-single/app/service.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/no-krm.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/no-krm/docs/README.md (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/no-krm/docs/notes.txt (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/overlapping.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/overlapping/apps/web/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/overlapping/apps/web/extra/configmap.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/plain-per-env.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/plain-per-env/apps/app/prod/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/plain-per-env/apps/app/prod/service.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/plain-per-env/apps/app/test/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/supported/plain-per-env/apps/app/test/service.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/base/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/base/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/base/service.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/overlays/acc/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/overlays/prod/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/base-overlays/overlays/test/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/helm-inflation.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/helm-inflation/app/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base/base/common/configmap.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base/base/common/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base/base/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base/base/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-nested-base/overlays/test/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-parked-base.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-parked-base/base/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-parked-base/base/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-parked-base/base/parked.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/overlay-parked-base/overlays/test/kustomization.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/plain-nonkrm.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/plain-nonkrm/app/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/plain-nonkrm/app/values.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/unsupported-kustomize.golden.json (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/unsupported-kustomize/app/deployment.yaml (100%) rename internal/manifestanalyzer/testdata/{repo-walker => scan-repo}/unsupported/unsupported-kustomize/app/kustomization.yaml (100%) diff --git a/cmd/manifest-analyzer/main.go b/cmd/manifest-analyzer/main.go index f7f912fb..c44c9754 100644 --- a/cmd/manifest-analyzer/main.go +++ b/cmd/manifest-analyzer/main.go @@ -9,27 +9,29 @@ // Usage: // // manifest-analyzer [flags] -// manifest-analyzer --mode repo-walker [flags] +// manifest-analyzer --mode scan-repo [flags] // manifest-analyzer --mode discovery [flags] // -// --mode analyze|scan|repo-walker|discovery what to produce (default analyze) -// analyze: the structural report (files, GVK inventory) -// scan: the adoption dry-run (acceptance + plan), the -// shared scan-mode pipeline with no flush -// repo-walker: the F8 whole-repo onboarding scan — walk -// every folder, classify candidate GitTargets. -// Report-only: --policy is not applied (the -// repo-level refuse gate is deferred) -// discovery: raw Kubernetes API discovery dump +// --mode analyze|scan-folder|scan-repo|discovery what to produce (default analyze) +// analyze: the structural report (files, GVK inventory) +// scan-folder: may THIS folder become a GitTarget? The +// adoption dry-run (acceptance + plan), the +// shared scan pipeline with no flush +// scan-repo: which folders under this repo root could +// become GitTargets? Classifies every +// candidate. Report-only: --policy is not +// applied (the repo-level refuse gate is +// deferred) +// discovery: raw Kubernetes API discovery dump // --format text|json output format (default text) // --policy report|refuse // report: always exit 0 (analysis only) // refuse: exit 1 when the folder would be refused -// (analyze: any acceptance issue; scan: not accepted) +// (analyze: any acceptance issue; scan-folder: not accepted) // // The tool is structure-only and needs no cluster: it reports duplicate // identities, KRM vs. non-KRM classification, multi-document files, and the -// inventory of every GVK found. Scan mode additionally applies the non-API KRM +// inventory of every GVK found. scan-folder additionally applies the non-API KRM // allowlist (kustomization.yaml is retained, not flagged), runs the full adoption // acceptance gate, and renders the plan — which is empty here because no cluster // state is available to compare against. @@ -88,9 +90,9 @@ func runWithDiscoveryClientFactory( ) int { fs := flag.NewFlagSet("manifest-analyzer", flag.ContinueOnError) fs.SetOutput(stderr) - mode := fs.String("mode", "analyze", "what to produce: analyze|scan|repo-walker|discovery") + mode := fs.String("mode", "analyze", "what to produce: analyze|scan-folder|scan-repo|discovery") format := fs.String("format", "text", "output format: text|json") - policy := fs.String("policy", "report", "adoption policy: report|refuse (repo-walker is report-only)") + policy := fs.String("policy", "report", "adoption policy: report|refuse (scan-repo is report-only)") kubeconfig := fs.String( "kubeconfig", "", @@ -99,7 +101,7 @@ func runWithDiscoveryClientFactory( contextName := fs.String("context", "", "kubeconfig context for --mode discovery") fs.Usage = func() { fmt.Fprintln(stderr, "usage: manifest-analyzer [flags] ") - fmt.Fprintln(stderr, " manifest-analyzer --mode repo-walker [flags] ") + fmt.Fprintln(stderr, " manifest-analyzer --mode scan-repo [flags] ") fmt.Fprintln(stderr, " manifest-analyzer --mode discovery [flags]") fs.PrintDefaults() } @@ -130,8 +132,8 @@ func runWithDiscoveryClientFactory( // to stderr. Splitting it out of run keeps the top-level dispatch simple. func validChoices(mode, format, policy string, stderr io.Writer) bool { switch { - case mode != "analyze" && mode != "scan" && mode != "repo-walker" && mode != "discovery": - fmt.Fprintf(stderr, "error: unknown mode %q (want analyze|scan|repo-walker|discovery)\n", mode) + case mode != "analyze" && mode != "scan-folder" && mode != "scan-repo" && mode != "discovery": + fmt.Fprintf(stderr, "error: unknown mode %q (want analyze|scan-folder|scan-repo|discovery)\n", mode) case format != "text" && format != "json": fmt.Fprintf(stderr, "error: unknown format %q (want text|json)\n", format) case policy != "report" && policy != "refuse": @@ -145,10 +147,10 @@ func validChoices(mode, format, policy string, stderr io.Writer) bool { // runDirMode dispatches the directory-argument modes (everything but discovery). func runDirMode(mode, dir, format, policy string, stdout, stderr io.Writer) int { switch mode { - case "scan": - return runScan(dir, format, policy, stdout, stderr) - case "repo-walker": - return runRepoWalk(dir, format, stdout, stderr) + case "scan-folder": + return runScanFolder(dir, format, policy, stdout, stderr) + case "scan-repo": + return runScanRepo(dir, format, stdout, stderr) default: return runAnalyze(dir, format, policy, stdout, stderr) } @@ -234,7 +236,7 @@ func failedGroupVersions(failed map[schema.GroupVersion]error) map[string]string return out } -// runScan runs the adoption dry-run (the shared scan-mode pipeline) and applies the +// runScanFolder runs the adoption dry-run (the shared scan pipeline) and applies the // refuse policy over the acceptance decision. It is structure-only: no cluster // state, so the plan is empty, but the acceptance gate is the full one — it applies // the non-API KRM allowlist and the impure-managed-file / mixed-file refusals. @@ -242,7 +244,7 @@ func failedGroupVersions(failed map[schema.GroupVersion]error) map[string]string // --format json goes through pkg/manifestanalyzer, so the CLI's machine-readable output // and the published Go contract are the same document and cannot drift. Text output stays // on the internal renderer, which can show the plan the public report deliberately omits. -func runScan(dir, format, policy string, stdout, stderr io.Writer) int { +func runScanFolder(dir, format, policy string, stdout, stderr io.Writer) int { if format == "json" { report, err := publicanalyzer.ScanFolder(context.Background(), dir) if err != nil { @@ -275,12 +277,12 @@ func scanExitCode(policy string, accepted bool) int { return exitOK } -// runRepoWalk runs the F8 whole-repo onboarding scan: walk every folder, enumerate +// runScanRepo runs the F8 whole-repo onboarding scan: walk every folder, enumerate // candidate GitTarget subtrees, classify each one's layout and acceptance, and emit the // report. It is read-only and needs no cluster. Exit codes stay simple for this cut // (exitOK, or exitUsage on an I/O error); the repo-level --policy refuse gate is // deferred per the design doc. -func runRepoWalk(root, format string, stdout, stderr io.Writer) int { +func runScanRepo(root, format string, stdout, stderr io.Writer) int { if format == "json" { report, err := publicanalyzer.ScanRepo(context.Background(), root) if err != nil { @@ -294,7 +296,7 @@ func runRepoWalk(root, format string, stdout, stderr io.Writer) int { return exitOK } - rep, err := manifestanalyzer.WalkRepo(context.Background(), root) + rep, err := manifestanalyzer.ScanRepo(context.Background(), root) if err != nil { fmt.Fprintf(stderr, "error: %v\n", err) return exitUsage diff --git a/cmd/manifest-analyzer/main_test.go b/cmd/manifest-analyzer/main_test.go index c1a5d308..868b939c 100644 --- a/cmd/manifest-analyzer/main_test.go +++ b/cmd/manifest-analyzer/main_test.go @@ -99,7 +99,7 @@ func TestRun_ScanTextRefuses(t *testing.T) { // fixtureDir contains a non-KRM values.yaml, so scan mode refuses under --policy // refuse and prints the acceptance verdict and the (empty, structure-only) plan. var out, errBuf bytes.Buffer - if code := run([]string{"--mode", "scan", "--policy", "refuse", fixtureDir(t)}, &out, &errBuf); code != 1 { + if code := run([]string{"--mode", "scan-folder", "--policy", "refuse", fixtureDir(t)}, &out, &errBuf); code != 1 { t.Fatalf("scan refuse with issues: exit = %d, want 1 (stderr=%s)", code, errBuf.String()) } for _, want := range []string{"Acceptance: REFUSED", "Plan: no changes"} { @@ -114,7 +114,7 @@ func TestRun_ScanCleanPasses(t *testing.T) { write(t, clean, "deploy.yaml", deployYAML) var out, errBuf bytes.Buffer - if code := run([]string{"--mode", "scan", "--policy", "refuse", clean}, &out, &errBuf); code != 0 { + if code := run([]string{"--mode", "scan-folder", "--policy", "refuse", clean}, &out, &errBuf); code != 0 { t.Fatalf("scan refuse on clean tree: exit = %d, want 0 (stderr=%s)", code, errBuf.String()) } if !strings.Contains(out.String(), "Acceptance: accepted") { @@ -124,7 +124,7 @@ func TestRun_ScanCleanPasses(t *testing.T) { func TestRun_ScanJSON(t *testing.T) { var out, errBuf bytes.Buffer - code := run([]string{"--mode", "scan", "--format", "json", fixtureDir(t)}, &out, &errBuf) + code := run([]string{"--mode", "scan-folder", "--format", "json", fixtureDir(t)}, &out, &errBuf) if code != 0 { t.Fatalf("exit = %d, stderr=%s", code, errBuf.String()) } @@ -137,10 +137,10 @@ func TestRun_ScanJSON(t *testing.T) { } } -// repoWalkFixture builds a tiny two-folder repo: a plain KRM app folder and a kustomize -// overlay reaching an out-of-subtree base, so repo-walker reports both an accepted plain +// scanRepoFixture builds a tiny two-folder repo: a plain KRM app folder and a kustomize +// overlay reaching an out-of-subtree base, so scan-repo reports both an accepted plain // candidate and a refused kustomize-overlay one. -func repoWalkFixture(t *testing.T) string { +func scanRepoFixture(t *testing.T) string { t.Helper() root := t.TempDir() write(t, mkdir(t, root, "plain"), "deploy.yaml", deployYAML) @@ -162,21 +162,21 @@ func mkdir(t *testing.T, dir, sub string) string { return full } -func TestRun_RepoWalkText(t *testing.T) { +func TestRun_ScanRepoText(t *testing.T) { var out, errBuf bytes.Buffer - if code := run([]string{"--mode", "repo-walker", repoWalkFixture(t)}, &out, &errBuf); code != 0 { + if code := run([]string{"--mode", "scan-repo", scanRepoFixture(t)}, &out, &errBuf); code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%s)", code, errBuf.String()) } for _, want := range []string{"candidates:", "kustomize-overlay", "overlay-fan-out-needs-f2"} { if !strings.Contains(out.String(), want) { - t.Errorf("repo-walker text missing %q:\n%s", want, out.String()) + t.Errorf("scan-repo text missing %q:\n%s", want, out.String()) } } } -func TestRun_RepoWalkJSON(t *testing.T) { +func TestRun_ScanRepoJSON(t *testing.T) { var out, errBuf bytes.Buffer - args := []string{"--mode", "repo-walker", "--format", "json", repoWalkFixture(t)} + args := []string{"--mode", "scan-repo", "--format", "json", scanRepoFixture(t)} if code := run(args, &out, &errBuf); code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%s)", code, errBuf.String()) } @@ -188,13 +188,13 @@ func TestRun_RepoWalkJSON(t *testing.T) { Summary map[string]any `json:"summary"` } if err := json.Unmarshal(out.Bytes(), &parsed); err != nil { - t.Fatalf("repo-walker JSON invalid: %v\n%s", err, out.String()) + t.Fatalf("scan-repo JSON invalid: %v\n%s", err, out.String()) } if len(parsed.Candidates) != 2 { t.Fatalf("want 2 candidates, got %d: %s", len(parsed.Candidates), out.String()) } if parsed.Summary == nil { - t.Errorf("repo-walker JSON missing summary: %s", out.String()) + t.Errorf("scan-repo JSON missing summary: %s", out.String()) } } @@ -274,9 +274,13 @@ func TestRun_Errors(t *testing.T) { {"bad policy", []string{"--policy", "delete", "x"}, 2}, {"discovery rejects dir", []string{"--mode", "discovery", "x"}, 2}, {"missing dir", []string{filepath.Join("definitely", "missing", "dir")}, 2}, - {"scan missing dir", []string{"--mode", "scan", filepath.Join("definitely", "missing")}, 2}, - {"repo-walker missing dir", []string{"--mode", "repo-walker", filepath.Join("definitely", "missing")}, 2}, - {"repo-walker no args", []string{"--mode", "repo-walker"}, 2}, + {"scan-folder missing dir", []string{"--mode", "scan-folder", filepath.Join("definitely", "missing")}, 2}, + {"scan-repo missing dir", []string{"--mode", "scan-repo", filepath.Join("definitely", "missing")}, 2}, + {"scan-repo no args", []string{"--mode", "scan-repo"}, 2}, + // The pre-freeze names carry no back-compat alias: they are usage errors, not + // silent fallbacks onto the default analyze mode. + {"retired scan mode", []string{"--mode", "scan", "x"}, 2}, + {"retired repo-walker mode", []string{"--mode", "repo-walker", "x"}, 2}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 27fdd0bd..69a97244 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -7,18 +7,32 @@ guidance that the changelog's breaking-change entries link to. We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with `bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it. -## Unreleased — `manifest-analyzer --format json` now emits a versioned contract (next minor; breaking) +## Unreleased — `manifest-analyzer` scan modes renamed, and `--format json` now emits a versioned contract (next minor; breaking) The analyzer's machine-readable output moved to the new public [`pkg/manifestanalyzer`](../pkg/manifestanalyzer), which is a supported Go API and the single -definition of the JSON documents the CLI prints. Both affected modes gained a `schemaVersion` -field, and one field was dropped: - -- `--mode scan --format json` no longer carries `plan`. In scan mode the analyzer has no cluster - state and no desired resources, so `plan` was structurally always `{"counts":{},"actions":null}` — - it never carried information. The meaningful fields (`accepted`, `issues`, `retained`) are - unchanged, and `issues` now marshals as `[]` rather than `null` when there are none. -- `--mode repo-walker --format json` is otherwise unchanged. +definition of the JSON documents the CLI prints. Freezing that contract is also the moment to name +the CLI modes after the question each answers, so the CLI, the Go API and the docs use one pair of +nouns — **folder** and **repo**. + +**Modes renamed. There are no back-compat aliases: the old names now exit 2 (usage error).** + +| Before | After | Answers | +|---|---|---| +| `--mode scan` | `--mode scan-folder` | May **this folder** become a `GitTarget`? (`ScanFolder`) | +| `--mode repo-walker` | `--mode scan-repo` | Which folders under **this repo root** could? (`ScanRepo`) | + +`repo-walker` named an internal traversal phase rather than a contract, and a bare `scan` was +asymmetric once a repo-level scan existed. `--mode analyze` and `--mode discovery` are unchanged. + +The JSON documents also gained a `schemaVersion` field, and one field was dropped: + +- `--mode scan-folder --format json` no longer carries `plan`. In folder-scan mode the analyzer has + no cluster state and no desired resources, so `plan` was structurally always + `{"counts":{},"actions":null}` — it never carried information. The meaningful fields (`accepted`, + `issues`, `retained`) are unchanged, and `issues` now marshals as `[]` rather than `null` when + there are none. +- `--mode scan-repo --format json` is otherwise unchanged. - Retained entries now omit `identity` for a whole-file retention (an ordinary `kustomization.yaml`, which names no resource) instead of emitting a zero-valued object. It is still present for the refused mixed-file case, where a named resource hides inside a build @@ -27,12 +41,14 @@ field, and one field was dropped: **Migration** +- Replace `--mode scan` with `--mode scan-folder`, and `--mode repo-walker` with `--mode scan-repo`. + A stale invocation fails loudly rather than falling back to the default `analyze` mode. - Read `schemaVersion` and ignore fields you do not know; new fields are added within a schema major. - If you exec'd the binary only to reach the acceptance verdict, prefer importing `pkg/manifestanalyzer` and calling `ScanFolder` / `ScanRepo`. They run the same acceptance gate the operator's writer enforces, so a tool built on them cannot drift from the operator that will later adopt (or refuse) the folder. -- If you parsed `plan` from scan mode, you were reading an empty object; drop the field. +- If you parsed `plan` from folder-scan mode, you were reading an empty object; drop the field. ## Unreleased — chart defaults now run Redis-free (next minor; behavior change) diff --git a/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md b/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md index 21889d52..0bd46d55 100644 --- a/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md +++ b/docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md @@ -62,7 +62,7 @@ layer — exactly the division already recorded in Scanning every folder sounds like it re-opens every kustomize question. It does not, because **discovery is broad and read-only while the write contract stays narrow and -principled** — they are deliberately different jobs. repo-walker classifies what a folder +principled** — they are deliberately different jobs. scan-repo classifies what a folder *is*; it never widens what the operator will *write*. The write boundary is not a hand-maintained feature allowlist but a **consequence of one rule**: every edit must have a single writable Git destination that round-trips @@ -84,7 +84,7 @@ share one engine (`Scan` is documented as "the one planner shared by the manifest-analyzer CLI and the controller's scan path", `internal/manifestanalyzer/scan.go`), and `manifest-analyzer` already scans a **single** folder read-only with no cluster: -`ScanDir(ctx, dir, nil, nil, policy)` (`--mode scan`). +`ScanDir(ctx, dir, nil, nil, policy)` (`--mode scan-folder`). The building blocks a repo-wide pass needs all exist: @@ -212,7 +212,7 @@ why rendered and editable diverge for overlays). Plus a repo-level summary: `exitUsage=2`). The eventual design: `--policy report` always exits 0 (pure report); `--policy refuse` exits 1 when **zero** candidates are acceptable — i.e. "is this repo onboardable at all?", the useful CI/onboarding gate. **The first cut does not yet apply -`--policy` in repo-walker mode** — it always exits 0 (or 2 on an I/O error); the refuse +`--policy` in scan-repo mode** — it always exits 0 (or 2 on an I/O error); the refuse gate is deferred. ## CLI surface @@ -220,15 +220,23 @@ gate is deferred. A new mode on `manifest-analyzer` (shipped form): ``` -manifest-analyzer --mode repo-walker --format json +manifest-analyzer --mode scan-repo --format json ``` -Naming note: the existing `--mode discovery` is **Kubernetes API discovery** (a -served-GVR dump against a kubeconfig), unrelated to repo discovery. The first cut -named the new mode `repo-walker` (not `repo`) to stay clear of it; renaming the -existing mode to `--mode api-discovery` so "discovery" is not overloaded remains a -possible follow-up. `--policy` is accepted for symmetry but the repo-level refuse gate -is deferred (see [First cut](#first-cut-shipped-2026-07-09)). +Naming note: the modes are named after the product question they answer, matching the +public [`pkg/manifestanalyzer`](../../../pkg/manifestanalyzer) entry points — +`scan-folder` asks "may **this folder** become a GitTarget?" (`ScanFolder`) and +`scan-repo` asks "which folders under **this repo root** could?" (`ScanRepo`). The first +cut shipped these as `scan` and `repo-walker`; both were renamed when the package became +a supported contract, with no back-compat aliases. `repo-walker` named an internal +traversal phase rather than a contract, and a bare `scan` was asymmetric once a +repo-level scan existed. + +The existing `--mode discovery` is **Kubernetes API discovery** (a served-GVR dump +against a kubeconfig), unrelated to repo discovery; renaming it to `--mode api-discovery` +so "discovery" is not overloaded remains a possible follow-up. `--policy` is accepted for +symmetry but the repo-level refuse gate is deferred (see +[First cut](#first-cut-shipped-2026-07-09)). The report should also be exposed as a **library function** (the product is likely part of the same Go binary family), with the CLI mode as the thin wrapper @@ -239,8 +247,8 @@ likely part of the same Go binary family), with the CLI mode as the thin wrapper A lean first slice landed on branch `feat/gitops-api-f8`. It **reports**; it does not yet **propose**. Library-first, as recommended above. -**Surface.** `WalkRepo(ctx, root) (RepoReport, error)` in -`internal/manifestanalyzer/repowalk.go`, with `--mode repo-walker ` +**Surface.** `ScanRepo(ctx, root) (RepoReport, error)` in +`internal/manifestanalyzer/scan_repo.go`, with `--mode scan-repo ` (`--format text|json`) as a thin CLI wrapper. Read-only, no cluster, symlinks never followed — the same posture as `ScanDir`, over the whole tree. It reuses `collectFiles`, `parseKustomizations`/`renderRoots`, the `Scan`/acceptance gate, and mirrors @@ -297,7 +305,7 @@ generation, the `--mode discovery` rename, and the repo-level `--policy refuse` (exit codes stay simple — `exitOK`, or `exitUsage` on an I/O error). **Corpus.** Golden reports under -`internal/manifestanalyzer/testdata/repo-walker/{supported,unsupported}//` with a +`internal/manifestanalyzer/testdata/scan-repo/{supported,unsupported}//` with a sibling `.golden.json` each (regenerate with `UPDATE_GOLDEN=1`), covering plain-per-env, kustomize-single, base+overlays, HelmRelease document vs. helm inflation, unsupported kustomize (incl. `openapi`/`crds`), fleet-root, overlapping, no-krm, and @@ -354,9 +362,10 @@ nested base (deduped `rendered`), an overlay base holding parked YAML (excluded ## Open questions -1. **Mode naming / the `discovery` collision** — _partly settled:_ the new mode shipped - as `repo-walker` (no collision). Whether to also rename the K8s-API mode to - `api-discovery` is still open. +1. **Mode naming / the `discovery` collision** — _partly settled:_ the repo mode is + `scan-repo` and the folder mode is `scan-folder`, mirroring `ScanRepo` / `ScanFolder` + (no collision). Whether to also rename the K8s-API mode to `api-discovery` is still + open. 2. **Stop at `GitTarget`, or also propose `WatchRule`s?** — moot for now: the first cut proposes neither (reports only). 3. **Read-scope depth for overlays pre-F2** — how far up the tree to follow @@ -366,7 +375,7 @@ nested base (deduped `rendered`), an overlay base holding parked YAML (excluded double-counts a shared nested base). How many levels the product should *display* remains a presentation choice. 4. **Library vs. CLI as the product's integration point** — _settled:_ library-first - (`WalkRepo`), CLI (`--mode repo-walker`) as the thin wrapper + CI gate. + (`ScanRepo`), CLI (`--mode scan-repo`) as the thin wrapper + CI gate. 5. **`overlay-fan-out-needs-f2` naming** — the reason code reads as if *fan-out* (a base shared by several overlays) is the trigger, but the real trigger is a base **escaping the render root's own subtree**, true even at fan-out = 1. The first cut carries the @@ -375,5 +384,5 @@ nested base (deduped `rendered`), an overlay base holding parked YAML (excluded and rely on the detail. _Not settled._ 6. **Repo-level `--policy refuse` gate** — the eventual "zero acceptable candidates → exit 1" onboarding gate (see [Exit codes](#the-report-contract)) is **deferred**: the first - cut always exits 0 (or 2 on an I/O error) and `--policy` is not applied in repo-walker + cut always exits 0 (or 2 on an I/O error) and `--policy` is not applied in scan-repo mode. Open: implement it as the CI/onboarding gate when the product needs it. diff --git a/docs/design/gitops-api/gittarget-granularity-and-cross-environment-edits.md b/docs/design/gitops-api/gittarget-granularity-and-cross-environment-edits.md index 84e6e8b2..001540a8 100644 --- a/docs/design/gitops-api/gittarget-granularity-and-cross-environment-edits.md +++ b/docs/design/gitops-api/gittarget-granularity-and-cross-environment-edits.md @@ -314,10 +314,10 @@ later — not a reason to make the operator's write unit span environments. A is also where the code and docs already point: the product model fixes environment = GitTarget = watch scope = write scope with shared read-only bases ([kustomize-support-boundary-and-product-model.md §5](kustomize-support-boundary-and-product-model.md#L208)), -and the repo-walker already classifies an out-of-subtree base as a +and the repo scan already classifies an out-of-subtree base as a forward-looking F2 gap (`overlay-fan-out-needs-f2`), not permanent unsupported structure -([repowalk.go](../../../internal/manifestanalyzer/repowalk.go#L37)). A is the +([scan_repo.go](../../../internal/manifestanalyzer/scan_repo.go#L37)). A is the direction of travel; this decision commits to it. **Onboarding UX is not a reason to pick B.** A produces N targets per app, but @@ -469,7 +469,7 @@ precondition and is refused outright (§1). - **F2 scope gains one concrete capability under A/C:** follow `../../base` for *reading* (today dropped), while the write jail stays at `spec.path`. Much of the read-scope / render-root / out-of-subtree-base logic already exists - read-only in the F8 repo-walker and can be promoted into the live analyzer. + read-only in the F8 repo scan and can be promoted into the live analyzer. - **Cross-environment editing is decided:** product promotion at launch; Option C (base-as-variant) later and only for editing *shared defaults*, not as the "every environment" answer. Neither reopens the fan-in invariant. diff --git a/docs/design/gitops-api/kustomize-support-boundary-and-product-model.md b/docs/design/gitops-api/kustomize-support-boundary-and-product-model.md index 4233a9c0..7f5f7736 100644 --- a/docs/design/gitops-api/kustomize-support-boundary-and-product-model.md +++ b/docs/design/gitops-api/kustomize-support-boundary-and-product-model.md @@ -121,7 +121,7 @@ Explicitly **out of scope**, and worth saying in user docs: much-later feature, not part of this boundary.) This allowlist is a **write** contract, not a discovery one. The read-only -onboarding scanner ([F8 repo-walker](f8-repo-discovery-and-onboarding-scan.md)) +onboarding scanner ([F8 repo scan](f8-repo-discovery-and-onboarding-scan.md)) walks a whole repo and classifies each folder against *exactly* this boundary — `plain` / `kustomize-single` (accepted), `kustomize-overlay` (forward-looking, needs F2), `refused-structural` (the permanent wall above) — without widening diff --git a/internal/manifestanalyzer/repowalk.go b/internal/manifestanalyzer/scan_repo.go similarity index 98% rename from internal/manifestanalyzer/repowalk.go rename to internal/manifestanalyzer/scan_repo.go index 8bae6af6..07eed438 100644 --- a/internal/manifestanalyzer/repowalk.go +++ b/internal/manifestanalyzer/scan_repo.go @@ -152,7 +152,7 @@ type RepoSummary struct { // RepoReport is the whole-repo discovery report: the machine-readable contract the // product layer consumes. type RepoReport struct { - // Root is the scanned repository root as passed to WalkRepo. It is informational. + // Root is the scanned repository root as passed to ScanRepo. It is informational. Root string `json:"root,omitempty"` // Candidates are the enumerated subtrees, sorted by path. Candidates []RepoCandidate `json:"candidates"` @@ -160,12 +160,12 @@ type RepoReport struct { Summary RepoSummary `json:"summary"` } -// WalkRepo is the F8 whole-repo discovery pass (the library entry point; the CLI -// --mode repo-walker is a thin wrapper). It is read-only, writes nothing, needs no +// ScanRepo is the F8 whole-repo discovery pass (the library entry point; the CLI +// --mode scan-repo is a thin wrapper). It is read-only, writes nothing, needs no // cluster, and never follows symlinks — the same posture as ScanDir, just over the // whole tree rather than one subtree. It verifies root is a directory, then walks // os.DirFS(root). -func WalkRepo(ctx context.Context, root string) (RepoReport, error) { +func ScanRepo(ctx context.Context, root string) (RepoReport, error) { info, err := os.Stat(root) if err != nil { return RepoReport{}, err @@ -173,13 +173,13 @@ func WalkRepo(ctx context.Context, root string) (RepoReport, error) { if !info.IsDir() { return RepoReport{}, fmt.Errorf("not a directory: %s", root) } - rep := walkRepoFS(ctx, os.DirFS(root)) + rep := scanRepoFS(ctx, os.DirFS(root)) rep.Root = root return rep, nil } -// walkRepoFS is WalkRepo over an fs.FS, so it is testable against an in-memory tree. -func walkRepoFS(ctx context.Context, fsys fs.FS) RepoReport { +// scanRepoFS is ScanRepo over an fs.FS, so it is testable against an in-memory tree. +func scanRepoFS(ctx context.Context, fsys fs.FS) RepoReport { scan := collectFiles(fsys) kusts := parseKustomizations(scan.YAMLFiles) // Structure-only whole-repo store built with the live writer's allowlist (WriterAllowlist: diff --git a/internal/manifestanalyzer/repowalk_test.go b/internal/manifestanalyzer/scan_repo_test.go similarity index 79% rename from internal/manifestanalyzer/repowalk_test.go rename to internal/manifestanalyzer/scan_repo_test.go index 926c1716..dd4544ba 100644 --- a/internal/manifestanalyzer/repowalk_test.go +++ b/internal/manifestanalyzer/scan_repo_test.go @@ -24,16 +24,16 @@ metadata: namespace: demo ` -// TestWalkRepo_Golden drives the whole discovery corpus under testdata/repo-walker. +// TestScanRepo_Golden drives the whole discovery corpus under testdata/scan-repo. // Each fixture is a self-contained repo with a sibling .golden.json pinning its // report, so layout classification, refusal codes, overlap detection, namespace // inference, and the rendered/editable split are all fixed by real layouts rather than // prose. The corpus is split supported/ vs unsupported/ mirroring the // contextual-namespace corpus. See // docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md. -func TestWalkRepo_Golden(t *testing.T) { +func TestScanRepo_Golden(t *testing.T) { for _, group := range []string{"supported", "unsupported"} { - base := filepath.Join("testdata", "repo-walker", group) + base := filepath.Join("testdata", "scan-repo", group) entries, err := os.ReadDir(base) if err != nil { t.Fatalf("read corpus %s: %v", base, err) @@ -53,9 +53,9 @@ func TestWalkRepo_Golden(t *testing.T) { // golden is path-independent. func checkGoldenFixture(t *testing.T, fixture string) { t.Helper() - rep, err := WalkRepo(context.Background(), fixture) + rep, err := ScanRepo(context.Background(), fixture) if err != nil { - t.Fatalf("WalkRepo(%s): %v", fixture, err) + t.Fatalf("ScanRepo(%s): %v", fixture, err) } if rep.Root != fixture { t.Errorf("Root = %q, want %q", rep.Root, fixture) @@ -88,11 +88,11 @@ func checkGoldenFixture(t *testing.T, fixture string) { } } -// TestWalkRepo_RefusalCodesStayDistinct pins the load-bearing distinction the design +// TestScanRepo_RefusalCodesStayDistinct pins the load-bearing distinction the design // calls out: a forward-looking overlay-fan-out-needs-f2 must never collapse into the // permanent refused-structural boundary. base-overlays is the former; helm-inflation and // unsupported-kustomize are the latter. -func TestWalkRepo_RefusalCodesStayDistinct(t *testing.T) { +func TestScanRepo_RefusalCodesStayDistinct(t *testing.T) { cases := []struct { fixture string wantCode string @@ -105,9 +105,9 @@ func TestWalkRepo_RefusalCodesStayDistinct(t *testing.T) { } for _, tc := range cases { t.Run(tc.fixture, func(t *testing.T) { - rep, err := WalkRepo(context.Background(), filepath.Join("testdata", "repo-walker", tc.fixture)) + rep, err := ScanRepo(context.Background(), filepath.Join("testdata", "scan-repo", tc.fixture)) if err != nil { - t.Fatalf("WalkRepo: %v", err) + t.Fatalf("ScanRepo: %v", err) } if len(rep.Candidates) == 0 { t.Fatalf("expected at least one candidate") @@ -133,15 +133,15 @@ func assertRefused(t *testing.T, fixture string, c RepoCandidate, layout Layout, } } -// TestWalkRepo_RenderedCountDedupesNestedBase guards the double-count: an overlay whose +// TestScanRepo_RenderedCountDedupesNestedBase guards the double-count: an overlay whose // out-of-subtree base pulls in a nested base must count each rendered document exactly // once (rendered=2 here: base/deployment + base/common/configmap), and readScope must be // the minimal [base] rather than [base, base/common]. A regression would report 3. -func TestWalkRepo_RenderedCountDedupesNestedBase(t *testing.T) { - fixture := filepath.Join("testdata", "repo-walker", "unsupported", "overlay-nested-base") - rep, err := WalkRepo(context.Background(), fixture) +func TestScanRepo_RenderedCountDedupesNestedBase(t *testing.T) { + fixture := filepath.Join("testdata", "scan-repo", "unsupported", "overlay-nested-base") + rep, err := ScanRepo(context.Background(), fixture) if err != nil { - t.Fatalf("WalkRepo: %v", err) + t.Fatalf("ScanRepo: %v", err) } if len(rep.Candidates) != 1 { t.Fatalf("want 1 candidate, got %d: %+v", len(rep.Candidates), rep.Candidates) @@ -155,14 +155,14 @@ func TestWalkRepo_RenderedCountDedupesNestedBase(t *testing.T) { } } -// TestWalkRepo_RenderedExcludesParkedYAML guards that rendered counts only what the +// TestScanRepo_RenderedExcludesParkedYAML guards that rendered counts only what the // kustomization graph reaches: a base holding a parked.yaml its kustomization does not // list must not inflate rendered. overlay-parked-base renders 1 (base/deployment), not 2. -func TestWalkRepo_RenderedExcludesParkedYAML(t *testing.T) { - fixture := filepath.Join("testdata", "repo-walker", "unsupported", "overlay-parked-base") - rep, err := WalkRepo(context.Background(), fixture) +func TestScanRepo_RenderedExcludesParkedYAML(t *testing.T) { + fixture := filepath.Join("testdata", "scan-repo", "unsupported", "overlay-parked-base") + rep, err := ScanRepo(context.Background(), fixture) if err != nil { - t.Fatalf("WalkRepo: %v", err) + t.Fatalf("ScanRepo: %v", err) } if len(rep.Candidates) != 1 { t.Fatalf("want 1 candidate, got %d: %+v", len(rep.Candidates), rep.Candidates) @@ -172,14 +172,14 @@ func TestWalkRepo_RenderedExcludesParkedYAML(t *testing.T) { } } -// TestWalkRepo_RefusedPlainSurfacesGateReasons guards that a plain candidate the +// TestScanRepo_RefusedPlainSurfacesGateReasons guards that a plain candidate the // acceptance gate refuses reports WHY (the gate issues) rather than a bare // acceptedByOperator: false. plain-nonkrm holds a non-KRM values.yaml. -func TestWalkRepo_RefusedPlainSurfacesGateReasons(t *testing.T) { - fixture := filepath.Join("testdata", "repo-walker", "unsupported", "plain-nonkrm") - rep, err := WalkRepo(context.Background(), fixture) +func TestScanRepo_RefusedPlainSurfacesGateReasons(t *testing.T) { + fixture := filepath.Join("testdata", "scan-repo", "unsupported", "plain-nonkrm") + rep, err := ScanRepo(context.Background(), fixture) if err != nil { - t.Fatalf("WalkRepo: %v", err) + t.Fatalf("ScanRepo: %v", err) } if len(rep.Candidates) != 1 { t.Fatalf("want 1 candidate, got %d", len(rep.Candidates)) @@ -196,17 +196,17 @@ func TestWalkRepo_RefusedPlainSurfacesGateReasons(t *testing.T) { } } -// TestWalkRepo_SopsBootstrapAcceptedLikeWriter guards that repo-walker's acceptance +// TestScanRepo_SopsBootstrapAcceptedLikeWriter guards that scan-repo's acceptance // matches the live writer's allowlist: a folder holding the operator's .sops.yaml // bootstrap config is accepted (the writer retains it), not refused as non-KRM, and the // .sops.yaml is not counted as non-KRM noise. -func TestWalkRepo_SopsBootstrapAcceptedLikeWriter(t *testing.T) { +func TestScanRepo_SopsBootstrapAcceptedLikeWriter(t *testing.T) { fsys := fstest.MapFS{ "app/deployment.yaml": {Data: []byte(deployYAMLNS)}, "app/.sops.yaml": {Data: []byte( "creation_rules:\n - path_regex: .*\n age: age1exampleexampleexampleexampleexampleexampleexample\n")}, } - rep := walkRepoFS(context.Background(), fsys) + rep := scanRepoFS(context.Background(), fsys) if len(rep.Candidates) != 1 { t.Fatalf("want 1 candidate, got %d: %+v", len(rep.Candidates), rep.Candidates) } @@ -220,10 +220,10 @@ func TestWalkRepo_SopsBootstrapAcceptedLikeWriter(t *testing.T) { } } -// TestWalkRepo_NotADirectory covers the usage error path: a missing or non-directory +// TestScanRepo_NotADirectory covers the usage error path: a missing or non-directory // root is an error, not an empty report. -func TestWalkRepo_NotADirectory(t *testing.T) { - if _, err := WalkRepo(context.Background(), filepath.Join("testdata", "does-not-exist")); err == nil { +func TestScanRepo_NotADirectory(t *testing.T) { + if _, err := ScanRepo(context.Background(), filepath.Join("testdata", "does-not-exist")); err == nil { t.Fatal("expected an error for a missing root") } } diff --git a/internal/manifestanalyzer/store.go b/internal/manifestanalyzer/store.go index 845b0a61..1e4286bc 100644 --- a/internal/manifestanalyzer/store.go +++ b/internal/manifestanalyzer/store.go @@ -806,7 +806,7 @@ func kustomizationUsesUnsupportedFeature(content []byte) bool { // or mutate resource identity (name/namespace) in ways the contextual-namespace writer // cannot map back to an editable source document. Their presence disqualifies a // kustomization as a namespace source. It is the single source of truth for both the -// boolean gate (hasUnsupportedKustomizeFeature) and the repo-walker's per-feature +// boolean gate (hasUnsupportedKustomizeFeature) and the repo scan's per-feature // refusal detail (unsupportedKustomizeFeatures). It returns a fresh slice on every call, // so no shared state can be mutated. func unsupportedKustomizeFeatureKeys() []string { diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/apps/podinfo/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/apps/podinfo/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/apps/podinfo/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/apps/podinfo/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/clusters/prod/flux-system.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/clusters/prod/flux-system.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/clusters/prod/flux-system.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/clusters/prod/flux-system.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/infra/ingress-nginx/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/infra/ingress-nginx/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/fleet-root/infra/ingress-nginx/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/fleet-root/infra/ingress-nginx/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/helm-release-document.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/helm-release-document.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/helm-release-document.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/helm-release-document.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/helm-release-document/app/helmrelease.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/helm-release-document/app/helmrelease.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/helm-release-document/app/helmrelease.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/helm-release-document/app/helmrelease.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/service.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/service.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/kustomize-single/app/service.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/kustomize-single/app/service.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/no-krm.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/no-krm.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/no-krm.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/no-krm.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/no-krm/docs/README.md b/internal/manifestanalyzer/testdata/scan-repo/supported/no-krm/docs/README.md similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/no-krm/docs/README.md rename to internal/manifestanalyzer/testdata/scan-repo/supported/no-krm/docs/README.md diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/no-krm/docs/notes.txt b/internal/manifestanalyzer/testdata/scan-repo/supported/no-krm/docs/notes.txt similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/no-krm/docs/notes.txt rename to internal/manifestanalyzer/testdata/scan-repo/supported/no-krm/docs/notes.txt diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/overlapping.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/overlapping.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/overlapping.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/overlapping.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/overlapping/apps/web/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/overlapping/apps/web/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/overlapping/apps/web/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/overlapping/apps/web/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/overlapping/apps/web/extra/configmap.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/overlapping/apps/web/extra/configmap.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/overlapping/apps/web/extra/configmap.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/overlapping/apps/web/extra/configmap.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env.golden.json b/internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/prod/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/prod/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/prod/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/prod/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/prod/service.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/prod/service.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/prod/service.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/prod/service.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/test/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/test/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/test/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/test/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/test/service.yaml b/internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/test/service.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/supported/plain-per-env/apps/app/test/service.yaml rename to internal/manifestanalyzer/testdata/scan-repo/supported/plain-per-env/apps/app/test/service.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/service.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/service.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/base/service.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/base/service.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/acc/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/acc/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/acc/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/acc/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/prod/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/prod/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/prod/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/prod/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/test/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/test/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/base-overlays/overlays/test/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/base-overlays/overlays/test/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/helm-inflation.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/helm-inflation.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/helm-inflation.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/helm-inflation.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/helm-inflation/app/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/helm-inflation/app/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/helm-inflation/app/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/helm-inflation/app/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/common/configmap.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/common/configmap.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/common/configmap.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/common/configmap.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/common/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/common/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/common/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/common/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/base/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/base/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/overlays/test/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/overlays/test/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-nested-base/overlays/test/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-nested-base/overlays/test/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/parked.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/parked.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/base/parked.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/base/parked.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/overlays/test/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/overlays/test/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/overlay-parked-base/overlays/test/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/overlay-parked-base/overlays/test/kustomization.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm/app/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm/app/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm/app/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm/app/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm/app/values.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm/app/values.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/plain-nonkrm/app/values.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/plain-nonkrm/app/values.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize.golden.json b/internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize.golden.json similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize.golden.json rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize.golden.json diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize/app/deployment.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize/app/deployment.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize/app/deployment.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize/app/deployment.yaml diff --git a/internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize/app/kustomization.yaml b/internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize/app/kustomization.yaml similarity index 100% rename from internal/manifestanalyzer/testdata/repo-walker/unsupported/unsupported-kustomize/app/kustomization.yaml rename to internal/manifestanalyzer/testdata/scan-repo/unsupported/unsupported-kustomize/app/kustomization.yaml diff --git a/pkg/manifestanalyzer/doc.go b/pkg/manifestanalyzer/doc.go index 64cf6439..9190acad 100644 --- a/pkg/manifestanalyzer/doc.go +++ b/pkg/manifestanalyzer/doc.go @@ -18,8 +18,8 @@ // Everything under internal/ is not covered by that promise and is not importable from // another module. // -// The command-line equivalents are `manifest-analyzer --mode scan --format json` and -// `--mode repo-walker --format json`, which emit exactly the documents [FolderReport] +// The command-line equivalents are `manifest-analyzer --mode scan-folder --format json` and +// `--mode scan-repo --format json`, which emit exactly the documents [FolderReport] // and [RepoReport] marshal to. Exec the binary if Go is not your language; import this // package if it is. // diff --git a/pkg/manifestanalyzer/folder.go b/pkg/manifestanalyzer/folder.go index e58a745d..56a9a1d7 100644 --- a/pkg/manifestanalyzer/folder.go +++ b/pkg/manifestanalyzer/folder.go @@ -132,7 +132,7 @@ func ScanFolderFS(ctx context.Context, fsys fs.FS) FolderReport { } // WriteJSON writes the report as indented JSON — byte-for-byte what -// `manifest-analyzer --mode scan --format json` prints. +// `manifest-analyzer --mode scan-folder --format json` prints. func (r FolderReport) WriteJSON(w io.Writer) error { if r.Issues == nil { r.Issues = []Issue{} @@ -143,7 +143,7 @@ func (r FolderReport) WriteJSON(w io.Writer) error { } // folderScanPolicy is the structure-only adoption gate: the default allowlist, no -// followability lookup, and no desired state. It is the same policy the CLI's scan mode +// followability lookup, and no desired state. It is the same policy the CLI's scan-folder mode // applies, kept in one place so the two can never disagree. func folderScanPolicy() internalanalyzer.ScanPolicy { return internalanalyzer.ScanPolicy{ diff --git a/pkg/manifestanalyzer/repo.go b/pkg/manifestanalyzer/repo.go index f4d22bca..37de0de8 100644 --- a/pkg/manifestanalyzer/repo.go +++ b/pkg/manifestanalyzer/repo.go @@ -111,7 +111,7 @@ type RepoReport struct { // GitTargets, classifying each one's layout and reporting why a folder is refused. It is // read-only, needs no cluster, and never follows symlinks. func ScanRepo(ctx context.Context, root string) (RepoReport, error) { - rep, err := internalanalyzer.WalkRepo(ctx, root) + rep, err := internalanalyzer.ScanRepo(ctx, root) if err != nil { return RepoReport{}, err } @@ -119,7 +119,7 @@ func ScanRepo(ctx context.Context, root string) (RepoReport, error) { } // WriteJSON writes the report as indented JSON — byte-for-byte what -// `manifest-analyzer --mode repo-walker --format json` prints. +// `manifest-analyzer --mode scan-repo --format json` prints. func (r RepoReport) WriteJSON(w io.Writer) error { if r.Candidates == nil { r.Candidates = []Candidate{} diff --git a/pkg/manifestanalyzer/repo_test.go b/pkg/manifestanalyzer/repo_test.go index 9a3d63de..7550eb96 100644 --- a/pkg/manifestanalyzer/repo_test.go +++ b/pkg/manifestanalyzer/repo_test.go @@ -19,7 +19,7 @@ import ( // package reads the same fixtures rather than growing a second copy: the point of this // package is that it cannot disagree with the engine, and a duplicated corpus is the // first way that guarantee rots. Tests run with the package directory as cwd. -const corpusRoot = "../../internal/manifestanalyzer/testdata/repo-walker" +const corpusRoot = "../../internal/manifestanalyzer/testdata/scan-repo" func fixture(t *testing.T, group, name string) string { t.Helper()