From 927faa322ec26a9947852ab08efc4e6fd358a1e0 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Wed, 28 Oct 2026 17:51:45 -0400 Subject: [PATCH 1/6] fix(render): hold a render to its own environment's exclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A gitops render of one environment was refused over another environment's exclude-services. `render --all --env blue` exited 1 over `staging`, and without --debug the message named no environment at all, which made it read like blue's own declaration was wrong when blue's was correct and read. ValidateWorkspace called ValidateExcludedServices, which holds EVERY environment to the workspace graph. That graph is the current composition (loadExclusionGraph: LoadServices, LoadModules, ConsumedEndpoints), so an environment imported before a module overlay added a service carries no withdrawal for it and fails against a composition it never saw. Any two environments composing different module sets hit this as soon as one gains a service the other has not withdrawn. The operator could not fix it from the declaration side either: re-importing the offending environment requires the product intent to declare it, and platform-obin's declares only dev, blue and green. A caller that selected an environment now passes it and gets ValidateEnvironmentExclusions' existing contract — "a flow is held to its own environment's composition, never refused over another's" — which run, build and deploy already used through NewFlow. A caller with no selection keeps holding every environment, which is right for a whole-workspace check such as the doctor. Module namespaces and host composition stay whole-workspace invariants, unchanged. The regression asserts both halves: the inconsistent environment still refuses when it is the selected one, so the test cannot pass vacuously, and selecting the clean environment succeeds. Co-Authored-By: Claude Opus 5 --- pkg/environments/workspace.go | 36 ++++++++++++++- pkg/gitops/dev.go | 2 +- pkg/gitops/orchestrate.go | 4 +- pkg/gitops/solution.go | 2 +- pkg/orchestration/excluded_services_test.go | 51 +++++++++++++++++++++ 5 files changed, 89 insertions(+), 6 deletions(-) diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index 8e5807bb..e6e54c5d 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -191,7 +191,25 @@ func validateResourcePathComponent(kind, name string) error { // it earlier. // Loading the graph is why this is a pass separate from postLoad, mirroring // ValidateServiceDependencies. -func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) error { +// A selected environment narrows the exclusion check to that environment. The +// exclusion graph is built from the workspace's CURRENT composition +// (loadExclusionGraph: LoadServices, LoadModules, ConsumedEndpoints), so +// holding EVERY environment to it refuses a flow over a declaration that is +// not its own: an environment imported before a module overlay added a service +// has no withdrawal for that service, and a render of a different, correct +// environment then fails over it. Blue hit exactly that — `render --env blue` +// refused over `staging`, whose composition predates robin/sandbox. +// +// So a caller that has selected an environment passes it, and gets +// ValidateEnvironmentExclusions' contract: "a flow is held to its own +// environment's composition, never refused over another's". A caller with no +// selection — a whole-workspace check such as the doctor — passes none and +// keeps holding every environment, which is right for a check whose subject +// IS the workspace. +// +// The other passes stay whole-workspace: module namespaces and host +// composition are invariants of the workspace, not of a selection. +func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, selected ...*Environment) error { environments, err := FromWorkspace(workspace) if err != nil { return err @@ -208,7 +226,7 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) erro return w.Wrap(hostErr) } } - if excludedErr := ValidateExcludedServices(ctx, workspace); excludedErr != nil { + if excludedErr := validateSelectedExclusions(ctx, workspace, selected); excludedErr != nil { return w.Wrap(excludedErr) } needsGraph := false @@ -380,3 +398,17 @@ func validateModuleNamespaces(workspace *resources.Workspace, environments []*En } return nil } + +// validateSelectedExclusions holds exclude-services to the workspace graph for +// the one selected environment, or for every environment when nothing was +// selected. A nil selection is the same as none: a caller that has no +// environment must not silently skip the check. +func validateSelectedExclusions(ctx context.Context, workspace *resources.Workspace, selected []*Environment) error { + for _, env := range selected { + if env == nil { + continue + } + return ValidateEnvironmentExclusions(ctx, workspace, env) + } + return ValidateExcludedServices(ctx, workspace) +} diff --git a/pkg/gitops/dev.go b/pkg/gitops/dev.go index 1d0416a0..d9dc6402 100644 --- a/pkg/gitops/dev.go +++ b/pkg/gitops/dev.go @@ -134,7 +134,7 @@ func buildRenderedServiceImages( rebuild bool, sink orchestration.OutputSink, ) ([]string, error) { - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return nil, err } if err := prepareSnapshotRegistry(ctx, env); err != nil { diff --git a/pkg/gitops/orchestrate.go b/pkg/gitops/orchestrate.go index 45ae2375..1b99a542 100644 --- a/pkg/gitops/orchestrate.go +++ b/pkg/gitops/orchestrate.go @@ -46,7 +46,7 @@ func renderModuleTree( if err := selectionguard.RejectUnboundExecution(workspace.Dir(), module.Dir()); err != nil { return RenderResult{}, err } - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return RenderResult{}, err } if env.WithdrawsModule(module) { @@ -618,7 +618,7 @@ func RenderService(ctx context.Context, workspace *resources.Workspace, module * } func renderService(ctx context.Context, workspace *resources.Workspace, module *resources.Module, service *resources.Service, env *environments.Environment, project string, build renderBuild, sink orchestration.OutputSink) (RenderResult, error) { - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return RenderResult{}, err } if err := CheckCompositionPermissions(ctx, workspace, env, true, module.Name); err != nil { diff --git a/pkg/gitops/solution.go b/pkg/gitops/solution.go index ad7b3e72..ab82414b 100644 --- a/pkg/gitops/solution.go +++ b/pkg/gitops/solution.go @@ -59,7 +59,7 @@ func RenderSolution(ctx context.Context, req *SolutionRenderRequest) (RenderResu if err := selectionguard.RejectUnboundExecution(req.Workspace.Dir(), req.Source); err != nil { return RenderResult{}, err } - if err := environments.ValidateWorkspace(ctx, req.Workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, req.Workspace, req.Environment); err != nil { return RenderResult{}, err } env := req.Environment diff --git a/pkg/orchestration/excluded_services_test.go b/pkg/orchestration/excluded_services_test.go index 70ad63af..59bfc74e 100644 --- a/pkg/orchestration/excluded_services_test.go +++ b/pkg/orchestration/excluded_services_test.go @@ -122,3 +122,54 @@ func TestPlanRefusesARootGroupReferenceToAnExcludedService(t *testing.T) { require.ErrorContains(t, err, "from this environment (exclude-services") require.ErrorContains(t, err, "platform/authority") } + +// TestFlowIsNotRefusedOverAnotherEnvironmentsExclusion is the Blue render +// defect: a flow selecting one environment was refused over a DIFFERENT +// environment's exclude-services. +// +// The exclusion graph is built from the workspace's current composition, so an +// environment imported before a module overlay added a service carries no +// withdrawal for it. Holding every environment to that graph meant a correct +// `--env blue` render failed over `staging`, whose composition predates the +// service — and the operator could not fix it from the declaration side, +// because the product intent declares no `staging` to re-import. +// +// Here `stale` excludes shop/vault while shop/api still consumes it, so +// `stale` genuinely does not hold up. Selecting `staging`, whose own +// declaration is clean, must still succeed. +func TestFlowIsNotRefusedOverAnotherEnvironmentsExclusion(t *testing.T) { + ctx := context.Background() + files := exclusionWorkspaceFiles("") + files["workspace.codefly.yaml"] = `name: acme +layout: modules +modules: + - name: shop +environments: + - name: staging + namespace: acme + - name: stale + namespace: acme + exclude-services: + - service: shop/vault +` + workspace := writeTempWorkspace(t, files) + module, err := workspace.LoadModuleFromName(ctx, "shop") + require.NoError(t, err) + api, err := module.LoadServiceFromName(ctx, "api") + require.NoError(t, err) + + // The other environment really is inconsistent with the graph: selecting it + // refuses, which is what makes the next assertion meaningful rather than + // vacuous. + stale, err := SelectEnvironment(workspace, "stale") + require.NoError(t, err) + _, err = NewFlow(ctx, workspace, module, api, stale, RunMode) + require.ErrorContains(t, err, "declares a dependency on it", + "the inconsistent environment must still refuse when it is the selected one") + + // Selecting the clean environment must not inherit that refusal. + staging, err := SelectEnvironment(workspace, "staging") + require.NoError(t, err) + _, err = NewFlow(ctx, workspace, module, api, staging, RunMode) + require.NoError(t, err, "a flow is held to its own environment, never refused over another's") +} From acc337fb215693c840d839784a172904f46f92c3 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Wed, 28 Oct 2026 18:04:12 -0400 Subject: [PATCH 2/6] fix(environments): name the environment in an exclusion refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without --debug the refusal printed no environment at all — just `exclude-services withdraws "authority/local"…` — so its reader could not tell which environment's declaration was at fault and re-declared something already declared, twice. The name was there, as an `environment %q:` prefix on the wrapped error, but a prefix is a separate link in the error chain and the plain renderer shows only the leaf; the name appeared solely in the --debug chain. So each message in validateExclusionsAgainst now names the environment itself, and the redundant prefix is gone from both validators rather than duplicating it. Every message in that function is covered, not only the dependency one that surfaced: removing the prefix while leaving the siblings unnamed would have taken the name away from refusals that previously carried it. Co-Authored-By: Claude Opus 5 --- pkg/environments/excluded_services.go | 26 ++++++++++++---------- pkg/environments/excluded_services_test.go | 4 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/environments/excluded_services.go b/pkg/environments/excluded_services.go index 87c0a744..6ae0ccb1 100644 --- a/pkg/environments/excluded_services.go +++ b/pkg/environments/excluded_services.go @@ -161,10 +161,12 @@ func ValidateEnvironmentExclusions(ctx context.Context, workspace *resources.Wor if err != nil { return err } - if err := env.validateExclusionsAgainst(graph); err != nil { - return fmt.Errorf("environment %q: %w", env.Name, err) - } - return nil + // No "environment %q:" prefix here: validateExclusionsAgainst names the + // environment inside each message, because a prefix is a separate link in + // the error chain and the plain renderer shows only the leaf — which is how + // a `--env blue` refusal printed no environment at all and sent its reader + // to re-declare something that was already declared. + return env.validateExclusionsAgainst(graph) } // ValidateExcludedServices holds every environment's exclude-services to the @@ -188,7 +190,7 @@ func ValidateExcludedServices(ctx context.Context, workspace *resources.Workspac } } if err := env.validateExclusionsAgainst(graph); err != nil { - return fmt.Errorf("environment %q: %w", env.Name, err) + return err } } return nil @@ -253,21 +255,21 @@ func (env *Environment) validateExclusionsAgainst(graph *exclusionGraph) error { } for _, entry := range env.ExcludeServices { if !composed[entry.Service] { - return fmt.Errorf("exclude-services names %q, which the workspace does not compose", entry.Service) + return fmt.Errorf("environment %q excludes %q, which the workspace does not compose", env.Name, entry.Service) } for _, consumer := range consumers[entry.Service] { if excluded[consumer] || slices.Contains(entry.WithdrawnFrom, consumer) { continue } - return fmt.Errorf("exclude-services withdraws %q, but %s declares a dependency on it: name %s under its withdrawn-from to withdraw that edge with it, or stop excluding %s", - entry.Service, consumer, consumer, entry.Service) + return fmt.Errorf("environment %q withdraws %q, but %s declares a dependency on it: name %s under that environment's withdrawn-from to withdraw the edge with it, or stop excluding %s there", + env.Name, entry.Service, consumer, consumer, entry.Service) } for _, consumer := range entry.WithdrawnFrom { if !slices.Contains(consumers[entry.Service], consumer) { - return fmt.Errorf("exclude-services %q withdrawn-from names %s, which declares no dependency on it", entry.Service, consumer) + return fmt.Errorf("environment %q excludes %q with a withdrawn-from naming %s, which declares no dependency on it", env.Name, entry.Service, consumer) } if excluded[consumer] { - return fmt.Errorf("exclude-services %q withdrawn-from names %s, which this environment excludes too", entry.Service, consumer) + return fmt.Errorf("environment %q excludes %q with a withdrawn-from naming %s, which it excludes too", env.Name, entry.Service, consumer) } } } @@ -277,7 +279,7 @@ func (env *Environment) validateExclusionsAgainst(graph *exclusionGraph) error { for index := range graph.consumed { consumption := &graph.consumed[index] if excluded[resources.ServiceUnique(consumption.Module, consumption.Service)] { - return fmt.Errorf("solution %s consumes %s/%s/%s (api.consumes), which exclude-services withdraws from this environment", consumption.Solution, consumption.Module, consumption.Service, consumption.Endpoint) + return fmt.Errorf("solution %s consumes %s/%s/%s (api.consumes), which environment %q withdraws", consumption.Solution, consumption.Module, consumption.Service, consumption.Endpoint, env.Name) } } for _, named := range env.serviceDeclarations() { @@ -290,7 +292,7 @@ func (env *Environment) validateExclusionsAgainst(graph *exclusionGraph) error { } } if excluded[target] { - return fmt.Errorf("%s names %s, which exclude-services withdraws from this environment: drop the declaration, or stop excluding %s", named.block, named.name, target) + return fmt.Errorf("%s names %s, which environment %q withdraws: drop the declaration, or stop excluding %s there", named.block, named.name, env.Name, target) } } return nil diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index 48bf62ea..58925314 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -116,7 +116,7 @@ func TestExcludeServicesRefusesWhatStillConsumesIt(t *testing.T) { "a withdrawal from a non-consumer": {` exclude-services: - service: shop/vault withdrawn-from: [shop/api, shop/cache] -`, "withdrawn-from names shop/cache, which declares no dependency on it"}, +`, `environment "staging" excludes "shop/vault" with a withdrawn-from naming shop/cache, which declares no dependency on it`}, "a service the workspace does not compose": {` exclude-services: - service: shop/vaults `, `"shop/vaults", which the workspace does not compose`}, @@ -221,7 +221,7 @@ service-dependencies: reloaded, err := resources.LoadWorkspaceFromDir(context.Background(), root) require.NoError(t, err) err = environments.ValidateWorkspace(context.Background(), reloaded) - require.ErrorContains(t, err, `exclude-services withdraws "shop/cache", but portal/web declares a dependency on it`) + require.ErrorContains(t, err, `environment "staging" withdraws "shop/cache", but portal/web declares a dependency on it`) } // A module naming the excluded service its entry routes the host to a service From 70b2dc2b62f79b562d748c43e627b4a89470c84e Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Wed, 28 Oct 2026 18:13:44 -0400 Subject: [PATCH 3/6] test(environments): exercise the changed selected-environment path, and document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier regression went through NewFlow, which already used ValidateEnvironmentExclusions at flow.go:484, so it never reached the ValidateWorkspace path this PR changes — it passed against unfixed main, which I verified rather than assumed. The claim in the PR body that it could not pass vacuously was wrong about which path it covered. TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment calls ValidateWorkspace directly and is behaviourally red at the parent: giving the parent the new signature while keeping ValidateExcludedServices fails on "a selected environment must not be refused over another environment's exclusion". It also pins the two halves that must not move — the inconsistent environment still refuses when it is the one selected, and a caller selecting nothing, nil included, still holds every environment. Docs corrected rather than added to. An earlier draft offered `doctor workspace` as the whole-workspace example; doctor already validates the selected environment through its own scope check (cmd/doctor_workspace.go:794), so that was inaccurate. Every CLI path that validates exclusions selects an environment; the whole-workspace form is what remains for a caller that selected none. docs/configuration-contract.md now states that these refusals are judged for the selected environment and name it. Co-Authored-By: Claude Opus 5 --- docs/commands.md | 18 ++++++++ docs/configuration-contract.md | 9 +++- pkg/environments/excluded_services_test.go | 50 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/docs/commands.md b/docs/commands.md index 554ae783..3bc807b0 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -121,6 +121,24 @@ never `--stand-alone` or `--exclude-root`; `--exclude-dependency` and out. The selected environment's `exclude-services` also removes services from the composition roots; a module withdrawn whole contributes no root, including its service-entry. CLI and run-profile exclusions still apply on top. + +**An operation is held to the environment it selected, never to another's +`exclude-services`.** Run, build, deploy and `deploy gitops render` each +validate the exclusions of the one environment they were given, and a refusal +names that environment in the message itself rather than only under `--debug`. +This matters because the exclusion graph is the workspace's **current** +composition: an environment imported before a module overlay added a service +carries no `withdrawn-from` for it, so holding every environment to that graph +refused a correct render of a different environment — and the operator could +not satisfy it, since re-importing the offending environment requires the +product intent to declare it. + +Every CLI path that validates exclusions selects an environment: run, build, +test and deploy through the flow, `deploy gitops render` and the solution and +dev renders through the workspace validation, and `codefly doctor workspace` +through its own scope check. The whole-workspace form, which holds every +environment at once, remains the behaviour for a caller that has selected +none. The deterministic port hash folds every endpoint into a few thousand buckets, so a `--naming-scope` can hash two endpoints to one port and abort the run; `--temporary-ports` sidesteps the hash, under the ordering its diff --git a/docs/configuration-contract.md b/docs/configuration-contract.md index d37a17dd..78ca9516 100644 --- a/docs/configuration-contract.md +++ b/docs/configuration-contract.md @@ -243,7 +243,14 @@ environments: Nothing may still reach what is withdrawn: a service that declares a dependency on it is refused unless `withdrawn-from` names it, and so are a solution consuming one of its endpoints, an environment declaration naming it -and a configuration reference to one of its endpoints. A service the module's +and a configuration reference to one of its endpoints. **Those refusals are +judged for the environment the operation selected, and the message names that +environment**: an operation is never refused over a different environment's +`exclude-services`. That distinction is load-bearing, because the graph a +declaration is held to is the workspace's current composition — an environment +imported before a module overlay added a service carries no `withdrawn-from` +for it, and holding every environment to that graph refuses operations on +environments whose own declarations are complete. A service the module's `interface:` exports, or names its `service-entry`, is withdrawn only with the whole module: an environment that names every service of a module leaves the module out, and its exports leave with it. Withdrawing some of its services is diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index 58925314..d0604246 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -313,3 +313,53 @@ func TestCoordinateContractCarriesExcludedServices(t *testing.T) { env.ExcludeServices[0].WithdrawnFrom[0] = "changed/consumer" require.Equal(t, "saas/accounts", contract.Environment.ExcludeServices[0].WithdrawnFrom[0]) } + +// TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment exercises the changed +// path directly: environments.ValidateWorkspace, which the gitops render +// callers reach. An earlier regression for this went through NewFlow, which +// already used ValidateEnvironmentExclusions at flow.go:484 — so it passed +// against unfixed main and proved nothing about ValidateWorkspace. +// +// The fixture gives `staging` an exclusion the graph refuses (shop/api still +// consumes shop/vault) and leaves `local` clean. A render of `local` must not +// inherit staging's refusal; a render of `staging` must still get it; and a +// caller that selects nothing must still hold the whole workspace, which is +// what `doctor workspace` and the other whole-workspace checks rely on. +func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { + ctx := context.Background() + workspace := writeExclusionWorkspace(t, ` exclude-services: + - service: shop/vault +`) + declared, err := environments.FromWorkspace(workspace) + require.NoError(t, err) + byName := map[string]*environments.Environment{} + for _, env := range declared { + if env != nil { + byName[env.Name] = env + } + } + require.Contains(t, byName, "staging") + require.Contains(t, byName, "local") + require.Empty(t, byName["local"].ExcludeServices, "local is the clean environment") + + // The selected environment is the inconsistent one: still refused, so this + // test cannot pass by the check having been disabled. + err = environments.ValidateWorkspace(ctx, workspace, byName["staging"]) + require.Error(t, err) + require.ErrorContains(t, err, `environment "staging" withdraws "shop/vault"`) + + // The selected environment is clean: must NOT be refused over staging's + // declaration. This is the assertion that fails before the fix. + require.NoError(t, environments.ValidateWorkspace(ctx, workspace, byName["local"]), + "a selected environment must not be refused over another environment's exclusion") + + // Nothing selected: the whole workspace is still held, so the default is + // preserved for doctor and the other whole-workspace callers. + err = environments.ValidateWorkspace(ctx, workspace) + require.Error(t, err, "with no selection every environment is still held") + require.ErrorContains(t, err, `environment "staging" withdraws "shop/vault"`) + + // A nil selection is the same as none, not a silent skip. + err = environments.ValidateWorkspace(ctx, workspace, nil) + require.Error(t, err, "a nil selection must not skip the check") +} From 010513b999b340cb14ae6fe17162fb1d0767a418 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:43:45 -0400 Subject: [PATCH 4/6] test(gitops): hold the render CALLER to its environment, and scope the claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bounded corrections from the focused closure review. A render-caller regression. The direct ValidateWorkspace test proves the validator narrows, but nothing proved the render entries still pass what they selected: dropping the environment from all four production callers (orchestrate.go twice, dev.go, solution.go) left the whole gitops package green. TestRenderModuleIsNotRefusedOverAnotherEnvironmentsExclusion goes through RenderModule with `stale` carrying a graph-inconsistent exclusion and `staging` clean, asserts the inconsistent environment is still refused when it IS the selection (so the second assertion is not vacuous), and fails when orchestrate.go drops its argument. Verified both ways. Accurate scope. The prose said an operation is "never refused over a different environment's exclude-services", which is broader than the code. The selection narrows the graph-consistency check and nothing else: loading a workspace decodes and validates every environment, so a malformed exclusion anywhere still refuses, and module namespaces, host composition, service-scoped names and managed-service keys are all still held across every environment — the same cross-environment shape, left deliberately whole-workspace. Both pages now say so, and TestSelectionNarrowsTheGraphCheckAndNothingElse pins it: a clean environment selects fine and is still refused over an unqualified name declared somewhere else. Doctor. Three comments said doctor relies on the no-selection fallback. It does not — it selects and calls ValidateEnvironmentExclusions directly (cmd/doctor_workspace.go). Corrected without changing doctor's behaviour. Co-Authored-By: Claude Opus 5 --- docs/commands.md | 33 +++++++++----- docs/configuration-contract.md | 6 ++- pkg/environments/excluded_services_test.go | 38 ++++++++++++++-- pkg/environments/workspace.go | 23 +++++++--- pkg/gitops/excluded_services_test.go | 50 ++++++++++++++++++++++ 5 files changed, 128 insertions(+), 22 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index 3bc807b0..f38781d5 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -122,10 +122,22 @@ out. The selected environment's `exclude-services` also removes services from the composition roots; a module withdrawn whole contributes no root, including its service-entry. CLI and run-profile exclusions still apply on top. -**An operation is held to the environment it selected, never to another's -`exclude-services`.** Run, build, deploy and `deploy gitops render` each -validate the exclusions of the one environment they were given, and a refusal -names that environment in the message itself rather than only under `--debug`. +**An operation is held to the environment it selected, never refused because a +DIFFERENT environment's `exclude-services` is inconsistent with the workspace +graph.** Run, build, deploy and `deploy gitops render` each check the one +environment they were given against the graph, and a refusal names that +environment in the message itself rather than only under `--debug`. + +This is scoped to the graph-consistency check, which is where the +cross-environment refusal came from. It is not a general rule that other +environments are ignored. Every environment is still decoded and validated +whenever a workspace is loaded, so a **malformed** `exclude-services` anywhere +— an unqualified name, a duplicate, a service withdrawn from itself — still +refuses any operation, selection or not. Module namespaces, host composition, +service-scoped names (`service-config`, `service-identity`, `service-mesh`) +and `managed-services` keys are likewise still held across every environment, +so a bad declaration in one of those blocks in an unselected environment can +still refuse a correct operation elsewhere. This matters because the exclusion graph is the workspace's **current** composition: an environment imported before a module overlay added a service carries no `withdrawn-from` for it, so holding every environment to that graph @@ -133,12 +145,13 @@ refused a correct render of a different environment — and the operator could not satisfy it, since re-importing the offending environment requires the product intent to declare it. -Every CLI path that validates exclusions selects an environment: run, build, -test and deploy through the flow, `deploy gitops render` and the solution and -dev renders through the workspace validation, and `codefly doctor workspace` -through its own scope check. The whole-workspace form, which holds every -environment at once, remains the behaviour for a caller that has selected -none. +Every CLI path that checks exclusions against the graph selects an +environment: run, build, test and deploy through the flow, `deploy gitops +render` and the solution and dev renders through the workspace validation, and +`codefly doctor workspace` through its own scope check — doctor selects and +calls the selected check directly, rather than relying on the fallback. The +whole-workspace form, which holds every environment's graph at once, remains +the behaviour for a caller that has selected none. The deterministic port hash folds every endpoint into a few thousand buckets, so a `--naming-scope` can hash two endpoints to one port and abort the run; `--temporary-ports` sidesteps the hash, under the ordering its diff --git a/docs/configuration-contract.md b/docs/configuration-contract.md index 78ca9516..1e229166 100644 --- a/docs/configuration-contract.md +++ b/docs/configuration-contract.md @@ -245,8 +245,10 @@ dependency on it is refused unless `withdrawn-from` names it, and so are a solution consuming one of its endpoints, an environment declaration naming it and a configuration reference to one of its endpoints. **Those refusals are judged for the environment the operation selected, and the message names that -environment**: an operation is never refused over a different environment's -`exclude-services`. That distinction is load-bearing, because the graph a +environment**: an operation is not refused because a different environment's +`exclude-services` is inconsistent with the workspace graph. A **malformed** +exclusion is a separate matter — every environment is validated for shape when +the workspace loads, so one of those refuses any operation. That distinction is load-bearing, because the graph a declaration is held to is the workspace's current composition — an environment imported before a module overlay added a service carries no `withdrawn-from` for it, and holding every environment to that graph refuses operations on diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index d0604246..76a681ea 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -323,8 +323,11 @@ func TestCoordinateContractCarriesExcludedServices(t *testing.T) { // The fixture gives `staging` an exclusion the graph refuses (shop/api still // consumes shop/vault) and leaves `local` clean. A render of `local` must not // inherit staging's refusal; a render of `staging` must still get it; and a -// caller that selects nothing must still hold the whole workspace, which is -// what `doctor workspace` and the other whole-workspace checks rely on. +// caller that selects nothing must still hold the whole workspace. +// +// (`doctor workspace` is NOT such a caller: it selects an environment and +// calls ValidateEnvironmentExclusions directly. The no-selection fallback is +// for a caller whose subject really is the whole workspace.) func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { ctx := context.Background() workspace := writeExclusionWorkspace(t, ` exclude-services: @@ -353,8 +356,8 @@ func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { require.NoError(t, environments.ValidateWorkspace(ctx, workspace, byName["local"]), "a selected environment must not be refused over another environment's exclusion") - // Nothing selected: the whole workspace is still held, so the default is - // preserved for doctor and the other whole-workspace callers. + // Nothing selected: the whole workspace is still held, preserving the + // default for a caller whose subject is the workspace itself. err = environments.ValidateWorkspace(ctx, workspace) require.Error(t, err, "with no selection every environment is still held") require.ErrorContains(t, err, `environment "staging" withdraws "shop/vault"`) @@ -363,3 +366,30 @@ func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { err = environments.ValidateWorkspace(ctx, workspace, nil) require.Error(t, err, "a nil selection must not skip the check") } + +// TestSelectionNarrowsTheGraphCheckAndNothingElse pins the limit of the +// change, so the prose describing it cannot drift past the code. +// +// Selecting an environment narrows the GRAPH-consistency check and only that. +// Loading a workspace decodes and validates every environment, so a +// MALFORMED exclusion — an unqualified name, here, in an environment nobody +// selected — still refuses. That is deliberate: a declaration that cannot be +// parsed is not a disagreement between an environment and the graph, it is a +// workspace that does not say what it means. +func TestSelectionNarrowsTheGraphCheckAndNothingElse(t *testing.T) { + ctx := context.Background() + workspace := writeExclusionWorkspace(t, ` exclude-services: + - service: vault +`) + // The clean environment selects fine — Select decodes only the one it is + // asked for, so the malformed declaration elsewhere does not block it. + local, err := environments.Select(workspace, "local") + require.NoError(t, err) + + // But validating with that selection still refuses, because the first + // thing the check does is decode EVERY environment. This is the boundary: + // the selection narrows the graph comparison, not the parsing. + err = environments.ValidateWorkspace(ctx, workspace, local) + require.Error(t, err, "a malformed exclusion elsewhere still refuses a selected, clean environment") + require.ErrorContains(t, err, "exclude-services service") +} diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index e6e54c5d..ff5bfaa1 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -202,13 +202,24 @@ func validateResourcePathComponent(kind, name string) error { // // So a caller that has selected an environment passes it, and gets // ValidateEnvironmentExclusions' contract: "a flow is held to its own -// environment's composition, never refused over another's". A caller with no -// selection — a whole-workspace check such as the doctor — passes none and -// keeps holding every environment, which is right for a check whose subject -// IS the workspace. +// environment's GRAPH, never refused over another's". A caller with no +// selection passes none and keeps holding every environment, which is right +// for a check whose subject IS the workspace. `doctor workspace` is not that +// caller: it selects, and calls ValidateEnvironmentExclusions directly +// (cmd/doctor_workspace.go) rather than relying on this fallback. // -// The other passes stay whole-workspace: module namespaces and host -// composition are invariants of the workspace, not of a selection. +// What the selection narrows is EXACTLY the graph-consistency check, and +// nothing else in this function: +// +// - FromWorkspace above decodes and validates every environment, including +// the SHAPE of its exclude-services (qualified names, duplicates, +// self-withdrawal). A malformed exclusion anywhere still refuses here. +// - module namespaces, host composition, service-scoped names and +// managed-service keys are all still held across every environment. +// +// Those remain whole-workspace deliberately — they are invariants of the +// workspace, not of a selection — but they are the same cross-environment +// shape, so narrowing them is a separate decision, not an oversight here. func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, selected ...*Environment) error { environments, err := FromWorkspace(workspace) if err != nil { diff --git a/pkg/gitops/excluded_services_test.go b/pkg/gitops/excluded_services_test.go index f0faec6e..7b240d6b 100644 --- a/pkg/gitops/excluded_services_test.go +++ b/pkg/gitops/excluded_services_test.go @@ -62,6 +62,56 @@ func TestRenderModuleLeavesAnExcludedServiceOutOfTheTreeAndTheInventory(t *testi `rendered unit "store" is a service environment "staging" excludes`) } +// TestRenderModuleIsNotRefusedOverAnotherEnvironmentsExclusion is the render +// caller's half of the Blue defect, and it is the one a direct test of +// ValidateWorkspace cannot give: it fails if any render entry stops passing +// the environment it was given, however correct the validator underneath is. +// +// `stale` excludes shop/store while shop/api still consumes it and withdraws +// nothing, so that environment genuinely does not hold up against the +// workspace graph. Rendering `staging`, whose own declaration is clean, must +// still succeed — the exclusion graph is built from the workspace's CURRENT +// composition, so an environment imported before a module overlay added a +// service carries no withdrawal for it, and an operator cannot repair an +// environment the product intent does not declare. +func TestRenderModuleIsNotRefusedOverAnotherEnvironmentsExclusion(t *testing.T) { + installFakeAgents(t) + root := t.TempDir() + require.NoError(t, copyTree(filepath.Join("testdata", "container-ports"), root)) + path := filepath.Join(root, resources.WorkspaceConfigurationName) + data, err := os.ReadFile(path) + require.NoError(t, err) + data = append(data, []byte(` - name: stale + namespace: acme + registry: + url: registry.example.com/acme + exclude-services: + - service: shop/store +`)...) + require.NoError(t, os.WriteFile(path, data, 0o644)) + ctx := context.Background() + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + module, err := workspace.LoadModuleFromName(ctx, "shop") + require.NoError(t, err) + + // The other environment really is inconsistent with the graph: selecting + // it is refused. Without this the next assertion would pass vacuously. + stale, err := orchestration.SelectEnvironment(workspace, "stale") + require.NoError(t, err) + _, err = RenderModule(ctx, workspace, module, stale, "acme-stale", nil) + require.ErrorContains(t, err, "shop/store", + "the inconsistent environment must still be refused when it is the selected one") + + // Selecting the clean environment must not inherit that refusal. + staging, err := orchestration.SelectEnvironment(workspace, "staging") + require.NoError(t, err) + result, err := RenderModule(ctx, workspace, module, staging, "acme-staging", nil) + require.NoError(t, err, "a render is held to its own environment, never refused over another's") + require.DirExists(t, filepath.Join(result.Path, "services", "store"), + "and the service the OTHER environment excludes is rendered here") +} + // A module the environment withdraws whole renders nothing there: the render // refuses it by name instead of producing an empty tree. func TestRenderModuleRefusesAModuleTheEnvironmentWithdrawsWhole(t *testing.T) { From 5ec1ac33ca66d59bbe9025326aa3e688c0989c71 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:50:22 -0400 Subject: [PATCH 5/6] docs(environments): say where a malformed exclusion refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prose said every environment is validated "whenever a workspace is loaded" and that a malformed exclusion "refuses any operation". The test added alongside it shows otherwise: LoadWorkspaceFromDir succeeds and Select("local") succeeds, and only then does ValidateWorkspace refuse. Corrected to what actually holds — ValidateWorkspace decodes and validates every environment's declarations before comparing the selected one against the graph, so a malformed exclusion refuses whatever was selected — with the load/select distinction stated rather than implied. Both pages and the test's own comment. No behaviour or test change. Co-Authored-By: Claude Opus 5 --- docs/commands.md | 11 +++++++---- docs/configuration-contract.md | 7 +++++-- pkg/environments/excluded_services_test.go | 13 ++++++++----- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index f38781d5..0435f396 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -130,10 +130,13 @@ environment in the message itself rather than only under `--debug`. This is scoped to the graph-consistency check, which is where the cross-environment refusal came from. It is not a general rule that other -environments are ignored. Every environment is still decoded and validated -whenever a workspace is loaded, so a **malformed** `exclude-services` anywhere -— an unqualified name, a duplicate, a service withdrawn from itself — still -refuses any operation, selection or not. Module namespaces, host composition, +environments are ignored. The check itself decodes and validates EVERY +environment's declarations before it compares the selected one against the +graph, so a **malformed** `exclude-services` anywhere — an unqualified name, a +duplicate, a service withdrawn from itself — still refuses, whatever was +selected. (Loading the workspace does not: a workspace with such a declaration +still loads, and a clean environment still selects. It is the validation that +refuses, not the load.) Module namespaces, host composition, service-scoped names (`service-config`, `service-identity`, `service-mesh`) and `managed-services` keys are likewise still held across every environment, so a bad declaration in one of those blocks in an unselected environment can diff --git a/docs/configuration-contract.md b/docs/configuration-contract.md index 1e229166..b90b8194 100644 --- a/docs/configuration-contract.md +++ b/docs/configuration-contract.md @@ -247,8 +247,11 @@ and a configuration reference to one of its endpoints. **Those refusals are judged for the environment the operation selected, and the message names that environment**: an operation is not refused because a different environment's `exclude-services` is inconsistent with the workspace graph. A **malformed** -exclusion is a separate matter — every environment is validated for shape when -the workspace loads, so one of those refuses any operation. That distinction is load-bearing, because the graph a +exclusion is a separate matter — the check validates every environment's +declarations for shape before comparing the selected one against the graph, so +one of those refuses whatever was selected. It is the validation that refuses, +not the load: such a workspace still loads, and a clean environment still +selects. That distinction is load-bearing, because the graph a declaration is held to is the workspace's current composition — an environment imported before a module overlay added a service carries no `withdrawn-from` for it, and holding every environment to that graph refuses operations on diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index 76a681ea..4ecd1fa2 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -371,11 +371,14 @@ func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { // change, so the prose describing it cannot drift past the code. // // Selecting an environment narrows the GRAPH-consistency check and only that. -// Loading a workspace decodes and validates every environment, so a -// MALFORMED exclusion — an unqualified name, here, in an environment nobody -// selected — still refuses. That is deliberate: a declaration that cannot be -// parsed is not a disagreement between an environment and the graph, it is a -// workspace that does not say what it means. +// ValidateWorkspace decodes and validates every environment's declarations +// before it compares the selected one against the graph, so a MALFORMED +// exclusion — an unqualified name, here, in an environment nobody selected — +// still refuses. Note what the assertions below show about where that +// happens: the workspace LOADS and the clean environment SELECTS; it is the +// validation that refuses, not the load. That is deliberate — a declaration +// that cannot be parsed is not a disagreement between an environment and the +// graph, it is a workspace that does not say what it means. func TestSelectionNarrowsTheGraphCheckAndNothingElse(t *testing.T) { ctx := context.Background() workspace := writeExclusionWorkspace(t, ` exclude-services: From 06f6e38abdb1a5a95fb51f2a06d909b24bf6ce57 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:53:50 -0400 Subject: [PATCH 6/6] test(gitops): say which render entry the regression covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said the test fails if "any render entry" drops its selection. It covers RenderModule. The other three callers that pass a selection — orchestrate.go's second call, dev.go and solution.go — are reviewed source, not covered here, and the comment now says so. No test or behaviour change. Co-Authored-By: Claude Opus 5 --- pkg/gitops/excluded_services_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/gitops/excluded_services_test.go b/pkg/gitops/excluded_services_test.go index 7b240d6b..5d66d006 100644 --- a/pkg/gitops/excluded_services_test.go +++ b/pkg/gitops/excluded_services_test.go @@ -64,8 +64,12 @@ func TestRenderModuleLeavesAnExcludedServiceOutOfTheTreeAndTheInventory(t *testi // TestRenderModuleIsNotRefusedOverAnotherEnvironmentsExclusion is the render // caller's half of the Blue defect, and it is the one a direct test of -// ValidateWorkspace cannot give: it fails if any render entry stops passing -// the environment it was given, however correct the validator underneath is. +// ValidateWorkspace cannot give: it fails if RenderModule stops passing the +// environment it was given, however correct the validator underneath is. +// +// It covers that one entry. The other three callers that pass a selection — +// orchestrate.go's second call, dev.go and solution.go — are reviewed source, +// not covered here. // // `stale` excludes shop/store while shop/api still consumes it and withdraws // nothing, so that environment genuinely does not hold up against the