From e4884580436fbc7d34aaa119212350f2dcb1bd53 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Fri, 23 Oct 2026 03:37:24 +0000 Subject: [PATCH 01/14] feat(environment): import isolated hosted configuration profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Materialize portable authored inputs through the existing importer while refusing credential values and authored destination files. Preserve each environment’s own runnable bindings and carry its actual publication root in the generated cell inventory. --- cmd/environment/configuration_profile.go | 137 ++++++++++++++++++ cmd/environment/configuration_profile_test.go | 82 +++++++++++ cmd/environment/import.go | 3 + docs/commands.md | 14 ++ pkg/environments/environment.go | 3 + pkg/gitops/cell.go | 6 +- pkg/gitops/cell_test.go | 18 +++ pkg/gitops/cellfile/cellfile.go | 2 + 8 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 cmd/environment/configuration_profile.go create mode 100644 cmd/environment/configuration_profile_test.go diff --git a/cmd/environment/configuration_profile.go b/cmd/environment/configuration_profile.go new file mode 100644 index 000000000..941ddc3b8 --- /dev/null +++ b/cmd/environment/configuration_profile.go @@ -0,0 +1,137 @@ +package environment + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/shared" +) + +const profileMarker = "# GENERATED — do not edit.\n# codefly environment import: " + +// materializeConfigurationProfile keeps authored inputs separate from an +// environment's generated configuration, including its own runnable bindings. +// It never copies credential values, follows symlinks, or overwrites authored files. +func materializeConfigurationProfile(ctx context.Context, directory string, env *environments.Environment, dryRun bool) error { + if env.ConfigurationSource == "" { + return nil + } + target, err := env.ConfigurationProfileName() + if err != nil { + return err + } + label := regexp.MustCompile(`^[a-z][a-z0-9-]*$`) + if !label.MatchString(env.ConfigurationSource) || !label.MatchString(target) || target == env.ConfigurationSource { + return fmt.Errorf("configuration-source and destination must be distinct profile names") + } + root, err := filepath.EvalSymlinks(directory) + if err != nil { + return err + } + configurationRoot := filepath.Join(root, "configurations") + resolved, err := filepath.EvalSymlinks(configurationRoot) + if err != nil || resolved != configurationRoot { + return fmt.Errorf("configuration profiles require an ordinary configurations directory") + } + source := filepath.Join(configurationRoot, env.ConfigurationSource) + destination := filepath.Join(configurationRoot, target) + for _, path := range []string{source, destination} { + info, statErr := os.Lstat(path) + if os.IsNotExist(statErr) && path == destination { + continue + } + if statErr != nil || !info.IsDir() { + return fmt.Errorf("configuration profile %s must be an ordinary directory", filepath.Base(path)) + } + } + entries, err := os.ReadDir(source) + if err != nil { + return err + } + files := make(map[string][]byte) + for _, entry := range entries { + if !strings.HasSuffix(entry.Name(), ".env") { + continue + } + if entry.Name() == "runnable-bindings.env" { + return fmt.Errorf("configuration-source must not contain environment-specific runnable bindings") + } + info, err := entry.Info() + if err != nil || !info.Mode().IsRegular() { + return fmt.Errorf("configuration source %s must be a regular file", entry.Name()) + } + body, err := os.ReadFile(filepath.Join(source, entry.Name())) + if err != nil { + return err + } + if strings.HasSuffix(entry.Name(), ".secret.env") { + for _, line := range strings.Split(string(body), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + _, value, found := strings.Cut(line, "=") + if !found || strings.TrimSpace(value) != "" { + return fmt.Errorf("configuration-source secret file %s must contain empty placeholders only", entry.Name()) + } + } + } + output := filepath.Join(destination, entry.Name()) + if info, err := os.Lstat(output); err == nil { + if !info.Mode().IsRegular() { + return fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) + } + existing, err := os.ReadFile(output) + if err != nil || !strings.HasPrefix(string(existing), profileMarker) { + return fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) + } + } else if !os.IsNotExist(err) { + return err + } + files[output] = append([]byte(profileMarker+env.ConfigurationSource+"/"+entry.Name()+"\n"), body...) + } + var retired []string + previous, err := os.ReadDir(destination) + if err != nil && !os.IsNotExist(err) { + return err + } + for _, entry := range previous { + path := filepath.Join(destination, entry.Name()) + if _, current := files[path]; current || !strings.HasSuffix(entry.Name(), ".env") { + continue + } + info, err := entry.Info() + if err != nil || !info.Mode().IsRegular() { + return fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) + } + body, err := os.ReadFile(path) + if err != nil { + return err + } + if strings.HasPrefix(string(body), profileMarker+env.ConfigurationSource+"/") { + retired = append(retired, path) + } + } + if dryRun { + return nil + } + if err := os.MkdirAll(destination, 0o755); err != nil { + return err + } + for path, body := range files { + if err := shared.WriteFileAtomic(ctx, path, body, 0o644); err != nil { + return err + } + } + for _, path := range retired { + if err := os.Remove(path); err != nil { + return err + } + } + return nil +} diff --git a/cmd/environment/configuration_profile_test.go b/cmd/environment/configuration_profile_test.go new file mode 100644 index 000000000..03e2730ae --- /dev/null +++ b/cmd/environment/configuration_profile_test.go @@ -0,0 +1,82 @@ +package environment + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/stretchr/testify/require" +) + +func profileSource(t *testing.T) string { + t.Helper() + root := t.TempDir() + directory := filepath.Join(root, "configurations", "hosted") + require.NoError(t, os.MkdirAll(directory, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(directory, "platform.env"), []byte("endpoint=${endpoint:saas/accounts/authority}\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(directory, "internal-auth.secret.env"), []byte("TOKEN=\n"), 0o644)) + return root +} + +func profileEnvironment(name string) *environments.Environment { + return &environments.Environment{Name: name, ConfigurationSource: "hosted", ConfigurationProfiles: []string{name, "local"}} +} + +func TestImportedProfilesSeparateSharedInputsAndEnvironmentOutputs(t *testing.T) { + root := profileSource(t) + for _, name := range []string{"blue", "green"} { + require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment(name), false)) + body, err := os.ReadFile(filepath.Join(root, "configurations", name, "platform.env")) + require.NoError(t, err) + require.True(t, strings.HasPrefix(string(body), profileMarker)) + require.Contains(t, string(body), "${endpoint:saas/accounts/authority}") + } + blue := filepath.Join(root, "configurations", "blue") + green := filepath.Join(root, "configurations", "green") + require.NoError(t, os.WriteFile(filepath.Join(blue, "runnable-bindings.env"), []byte("# generated by runnable-bindings\nBLUE=resolved-blue\n"), 0o644)) + require.NoError(t, os.Remove(filepath.Join(root, "configurations", "hosted", "platform.env"))) + require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), false)) + _, err := os.Stat(filepath.Join(blue, "platform.env")) + require.True(t, os.IsNotExist(err), "removed source groups retire only import-owned outputs") + _, err = os.Stat(filepath.Join(green, "platform.env")) + require.NoError(t, err, "importing blue never rewrites green") + body, err := os.ReadFile(filepath.Join(blue, "runnable-bindings.env")) + require.NoError(t, err) + require.Contains(t, string(body), "BLUE=resolved-blue", "the runnable generator owns this file") +} + +func TestProfileImportDryRunCreatesNothing(t *testing.T) { + root := profileSource(t) + require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), true)) + _, err := os.Stat(filepath.Join(root, "configurations", "blue")) + require.True(t, os.IsNotExist(err)) +} + +func TestProfileImportRefusesCredentialValuesBeforeWriting(t *testing.T) { + root := profileSource(t) + require.NoError(t, os.WriteFile(filepath.Join(root, "configurations", "hosted", "internal-auth.secret.env"), []byte("TOKEN=fixture-value\n"), 0o600)) + err := materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), false) + require.ErrorContains(t, err, "empty placeholders only") + require.NotContains(t, err.Error(), "fixture-value") + _, err = os.Stat(filepath.Join(root, "configurations", "blue")) + require.True(t, os.IsNotExist(err)) +} + +func TestProfileImportRefusesAuthoredOrSymlinkedFiles(t *testing.T) { + for _, symlink := range []bool{false, true} { + t.Run(map[bool]string{false: "authored", true: "symlink"}[symlink], func(t *testing.T) { + root := profileSource(t) + directory := filepath.Join(root, "configurations", "blue") + require.NoError(t, os.Mkdir(directory, 0o755)) + path := filepath.Join(directory, "platform.env") + if symlink { + require.NoError(t, os.Symlink(filepath.Join(root, "configurations", "hosted", "platform.env"), path)) + } else { + require.NoError(t, os.WriteFile(path, []byte("authored=true\n"), 0o644)) + } + require.Error(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), false)) + }) + } +} diff --git a/cmd/environment/import.go b/cmd/environment/import.go index ae11bd9e6..e80424f34 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -223,6 +223,9 @@ func runImport(ctx context.Context, opts *importOptions) error { if _, err := environments.FromWorkspace(merged); err != nil { return fmt.Errorf("merged environment declarations: %w", err) } + if err := materializeConfigurationProfile(ctx, opts.dir, env, opts.dryRun); err != nil { + return err + } if opts.dryRun { diff, derr := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ A: difflib.SplitLines(string(original)), diff --git a/docs/commands.md b/docs/commands.md index 90bd2589b..f85374642 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2216,6 +2216,20 @@ The merged file is still written, so the diagnostics can be read against it. #### `codefly environment show` +An imported environment may declare `configuration-source: hosted` and select +`configuration-profiles: [blue, local]`. Import materializes the authored +`configurations/hosted/*.env` inputs into the generated `configurations/blue/` +profile. Secret source files must contain empty placeholders. Import refuses +symlinks and authored destination files; dry-run writes nothing. Removed source +groups retire only import-owned files, preserving the environment's separately +generated runnable bindings. Run `codefly generate runnable-bindings --env blue` +after import so addresses are resolved for blue rather than copied from a sibling. + +The cell inventory also carries `source.path` when publication uses a root other +than `deployments`. Consumers must use that root to discover the published module +trees. The local render directory remains a qualification tree; `publish` writes +the environment's declared publication path. + Print the resolved `resources.Environment` as YAML (default) or `--json`. ```bash diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index d03cbc266..829f0eeaf 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -1308,6 +1308,9 @@ type Environment struct { // to ConfigurationProfile: each configuration location reads the first // profile it holds (resources.Environment.ConfigurationProfiles). ConfigurationProfiles []string `yaml:"configuration-profiles,omitempty"` + // ConfigurationSource is an authored workspace profile materialized by import + // into the first selected profile. The destination contains generated files. + ConfigurationSource string `yaml:"configuration-source,omitempty"` // RunnableScopeSelections answers owner-declared slots per binding key // (the same MODULE__OPERATION key generate runnable-bindings emits). diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index a9d27cee1..19570da9d 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -197,7 +197,11 @@ func cellSource(ctx context.Context, workspace *resources.Workspace, env *enviro if len(revision) != 40 { return nil } - return &cellfile.Source{Repo: env.Gitops.RepoURL, Revision: revision, Branch: env.Gitops.Branch} + source := &cellfile.Source{Repo: env.Gitops.RepoURL, Revision: revision, Branch: env.Gitops.Branch} + if path, err := validateRelativePath(env.Gitops.Path); err == nil && path != "" && path != DefaultGitopsPath { + source.Path = filepath.ToSlash(path) + } + return source } // cellNamespace derives one module's namespace entry from its rendered tree diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 34e8a0503..359858978 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -24,6 +24,24 @@ func cellServiceYAML(name string) string { ` } +func TestCellSourceCarriesTheEnvironmentPublicationPath(t *testing.T) { + workspace := writeCellWorkspace(t) + runGit(t, workspace.Dir(), "init", "--initial-branch=main") + runGit(t, workspace.Dir(), "-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "--allow-empty", "-m", "source") + for _, path := range []string{"deployments", "deployments/environments/blue"} { + env := &environments.Environment{Gitops: &environments.EnvironmentGitops{ + RepoURL: "https://github.com/obin-ai/platform-obin.git", Branch: "main", Path: path, + }} + source := cellSource(context.Background(), workspace, env) + require.NotNil(t, source) + if path == DefaultGitopsPath { + require.Empty(t, source.Path, "retained cell files preserve their default publication path") + } else { + require.Equal(t, path, source.Path, "infra must read the same root that publish writes") + } + } +} + // exposedCellServiceYAML is cellServiceYAML for the service whose `http` // endpoint the staging environment routes an ingress to: it is public and // states its exposure, or the route is refused. diff --git a/pkg/gitops/cellfile/cellfile.go b/pkg/gitops/cellfile/cellfile.go index dce2e78f5..5c969f7ab 100644 --- a/pkg/gitops/cellfile/cellfile.go +++ b/pkg/gitops/cellfile/cellfile.go @@ -51,6 +51,8 @@ type Source struct { Repo string `yaml:"repo"` Revision string `yaml:"revision"` Branch string `yaml:"branch"` + // Path is the publication root containing modules/. Absent means deployments. + Path string `yaml:"path,omitempty"` } // Namespace is one module's namespace: what runs in it, the external reach it From 4275a947c358122a5b361bc23d73434f693e6715 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Fri, 23 Oct 2026 03:45:32 +0000 Subject: [PATCH 02/14] fix(gitops): resolve configuration for module-qualified workloads --- docs/commands.md | 8 +++++-- pkg/environments/environment.go | 14 ++++++++++++ pkg/environments/environment_test.go | 16 ++++++++------ pkg/environments/workspace.go | 2 +- pkg/gitops/configuration_projection.go | 14 ++++++------ pkg/gitops/configuration_projection_test.go | 24 +++++++++++++++++++++ pkg/gitops/managed_identity.go | 2 +- 7 files changed, 62 insertions(+), 18 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index f85374642..1e3d896db 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2214,8 +2214,6 @@ readiness validation runs for the selected environment; when it reports the workspace is not ready, the command prints the diagnostics and exits non-zero. The merged file is still written, so the diagnostics can be read against it. -#### `codefly environment show` - An imported environment may declare `configuration-source: hosted` and select `configuration-profiles: [blue, local]`. Import materializes the authored `configurations/hosted/*.env` inputs into the generated `configurations/blue/` @@ -2230,6 +2228,12 @@ than `deployments`. Consumers must use that root to discover the published modul trees. The local render directory remains a qualification tree; `publish` writes the environment's declared publication path. +`service-config.services` accepts `/` keys for compositions +with same-named services. A qualified entry wins over the bare name. Unknown +qualified entries and ambiguous bare entries fail workspace validation. + +#### `codefly environment show` + Print the resolved `resources.Environment` as YAML (default) or `--json`. ```bash diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index 829f0eeaf..593ea2128 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -937,6 +937,20 @@ type EnvironmentServiceConfig struct { Services map[string]EnvironmentServiceConfigMapping `yaml:"services,omitempty"` } +// ValuesFor selects the qualified workload first, then a bare service entry. +// Workspace validation refuses a bare entry shared by multiple modules. +func (c *EnvironmentServiceConfig) ValuesFor(module, service string) map[string]string { + if c == nil { + return nil + } + if module != "" { + if mapping, ok := c.Services[module+"/"+service]; ok { + return mapping.Values + } + } + return c.Services[service].Values +} + // EnvironmentServiceConfigMapping holds one service's resolved values keyed by // the configuration key that consumes them. type EnvironmentServiceConfigMapping struct { diff --git a/pkg/environments/environment_test.go b/pkg/environments/environment_test.go index 968829f9c..48d10d54c 100644 --- a/pkg/environments/environment_test.go +++ b/pkg/environments/environment_test.go @@ -90,9 +90,10 @@ func serviceIdentityDeclaration(serviceName string) string { // service passes the graph check. func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accounts"), - "service-config": serviceConfigDeclaration("accounts"), - "service-identity": serviceIdentityDeclaration("accounts"), + "service-secrets": serviceSecretsDeclaration("accounts"), + "service-config": serviceConfigDeclaration("accounts"), + "qualified service-config": serviceConfigDeclaration("saas/accounts"), + "service-identity": serviceIdentityDeclaration("accounts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() @@ -112,9 +113,10 @@ func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { // projection time; the graph check must reject it. func TestValidateEnvironmentsRejectsUnknownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accunts"), - "service-config": serviceConfigDeclaration("accunts"), - "service-identity": serviceIdentityDeclaration("accunts"), + "service-secrets": serviceSecretsDeclaration("accunts"), + "service-config": serviceConfigDeclaration("accunts"), + "qualified service-config": serviceConfigDeclaration("wrong/accounts"), + "service-identity": serviceIdentityDeclaration("accunts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() @@ -126,7 +128,7 @@ func TestValidateEnvironmentsRejectsUnknownService(t *testing.T) { if err == nil { t.Fatal("expected ValidateEnvironments to reject unknown service") } - if !strings.Contains(err.Error(), "accunts") || !strings.Contains(err.Error(), name) { + if !strings.Contains(err.Error(), strings.TrimPrefix(name, "qualified ")) || (!strings.Contains(err.Error(), "accunts") && !strings.Contains(err.Error(), "wrong/accounts")) { t.Fatalf("error = %v, want it to name the block and the unknown service", err) } }) diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index caf001227..9cc782f9b 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -226,7 +226,7 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) erro // A module-qualified service-identity key names exactly one // service of that module, or nothing (a typo that would leave // the workload on the default identity with no report). - if _, service, qualified := strings.Cut(name, "/"); qualified && (block == "service-identity" || block == "service-mesh") { + if _, service, qualified := strings.Cut(name, "/"); qualified && (block == "service-identity" || block == "service-mesh" || block == "service-config") { if !slices.Contains(byName[service], name) { return w.Wrap(fmt.Errorf("environment %q %s references unknown service %q", env.Name, block, name)) } diff --git a/pkg/gitops/configuration_projection.go b/pkg/gitops/configuration_projection.go index 4b8db23d7..97a7b0bbf 100644 --- a/pkg/gitops/configuration_projection.go +++ b/pkg/gitops/configuration_projection.go @@ -26,10 +26,13 @@ func projectConfigurationValues(ctx context.Context, root, service string, env * } func projectConfigurationValuesInTree(ctx context.Context, tree *renderRoot, root, service string, env *environments.Environment) error { - values := map[string]string{} - if env.ServiceConfig != nil { - values = env.ServiceConfig.Services[service].Values + module, bare, qualified := strings.Cut(service, "/") + if qualified { + service = bare + } else { + module = "" } + values := env.ServiceConfig.ValuesFor(module, service) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { refs = env.ServiceSecrets.Services[service].RemoteKeys @@ -184,10 +187,7 @@ func validateProjectedConfiguration(root string, service *resources.Service, env } func validateProjectedConfigurationInTree(tree *renderRoot, root string, service *resources.Service, env *environments.Environment, scope unitScope) error { - values := map[string]string{} - if env.ServiceConfig != nil { - values = env.ServiceConfig.Services[service.Name].Values - } + values := env.ServiceConfig.ValuesFor(scope.Module, service.Name) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { refs = env.ServiceSecrets.Services[service.Name].RemoteKeys diff --git a/pkg/gitops/configuration_projection_test.go b/pkg/gitops/configuration_projection_test.go index d6d9e48cb..e44e43a5a 100644 --- a/pkg/gitops/configuration_projection_test.go +++ b/pkg/gitops/configuration_projection_test.go @@ -23,6 +23,30 @@ func injectionContract(t *testing.T) *environments.Environment { return &contract.Environment } +func TestQualifiedConfigurationKeepsSameNamedWorkloadsIsolated(t *testing.T) { + env := &environments.Environment{Name: "blue", Namespace: "product", ServiceConfig: &environments.EnvironmentServiceConfig{ + Services: map[string]environments.EnvironmentServiceConfigMapping{ + "wiki/backend": {Values: map[string]string{"PUBLIC_URL": "https://wiki.example"}}, + "proposals/backend": {Values: map[string]string{"PUBLIC_URL": "https://proposals.example"}}, + }, + }} + for _, module := range []string{"wiki", "proposals", "other"} { + t.Run(module, func(t *testing.T) { + root := t.TempDir() + writeConsumerTree(t, root, env.Name, env.Namespace, "backend", "declared.example") + scope := scopeOf(env) + scope.Module = module + require.NoError(t, projectServiceConfiguration(t.Context(), testRenderRoot(t, root), root, &resources.Service{Name: "backend"}, env, scope, serviceInjection{})) + values := containerEnvironment(t, buildOverlay(t, root, env.Name)) + if module == "other" { + require.NotContains(t, values, "PUBLIC_URL") + } else { + require.Equal(t, "https://"+module+".example", values["PUBLIC_URL"]["value"]) + } + }) + } +} + func TestActualInfraBaseValuesReachRenderedWorkload(t *testing.T) { data, err := os.ReadFile(filepath.Join("..", "environments", "testdata", "coordinates", "infra-base-lodestar.json")) require.NoError(t, err) diff --git a/pkg/gitops/managed_identity.go b/pkg/gitops/managed_identity.go index ed9ac7a77..9300cdd06 100644 --- a/pkg/gitops/managed_identity.go +++ b/pkg/gitops/managed_identity.go @@ -29,7 +29,7 @@ func projectServiceConfiguration(ctx context.Context, tree *renderRoot, root str if injection, err = artifactCacheInjection(service, env, scope, injection); err != nil { return err } - if err := projectConfigurationValuesInTree(ctx, tree, root, service.Name, env); err != nil { + if err := projectConfigurationValuesInTree(ctx, tree, root, resources.ServiceUnique(scope.Module, service.Name), env); err != nil { return fmt.Errorf("project service %s configuration: %w", service.Name, err) } // Before the secret projection: the derived secretKeyRefs are part of what From e0b96ba4569159a783faed701d560d9a51915a55 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Fri, 23 Oct 2026 04:12:05 +0000 Subject: [PATCH 03/14] fix(environment): reuse Core profile derivation for shared hosted inputs --- cmd/environment/configuration_profile.go | 108 +++++++++--------- cmd/environment/configuration_profile_test.go | 50 +++++++- docs/commands.md | 17 +-- 3 files changed, 112 insertions(+), 63 deletions(-) diff --git a/cmd/environment/configuration_profile.go b/cmd/environment/configuration_profile.go index 941ddc3b8..33f6d7154 100644 --- a/cmd/environment/configuration_profile.go +++ b/cmd/environment/configuration_profile.go @@ -9,14 +9,15 @@ import ( "strings" "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/configurations" "github.com/codefly-dev/core/shared" ) const profileMarker = "# GENERATED — do not edit.\n# codefly environment import: " -// materializeConfigurationProfile keeps authored inputs separate from an -// environment's generated configuration, including its own runnable bindings. -// It never copies credential values, follows symlinks, or overwrites authored files. +// materializeConfigurationProfile uses Core's profile derivation: shared inputs +// stay authored once, while each environment owns its generated runnable bindings. +// Validate the complete inherited source before writing its generated declaration. func materializeConfigurationProfile(ctx context.Context, directory string, env *environments.Environment, dryRun bool) error { if env.ConfigurationSource == "" { return nil @@ -38,73 +39,71 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env if err != nil || resolved != configurationRoot { return fmt.Errorf("configuration profiles require an ordinary configurations directory") } - source := filepath.Join(configurationRoot, env.ConfigurationSource) destination := filepath.Join(configurationRoot, target) - for _, path := range []string{source, destination} { - info, statErr := os.Lstat(path) - if os.IsNotExist(statErr) && path == destination { - continue - } - if statErr != nil || !info.IsDir() { - return fmt.Errorf("configuration profile %s must be an ordinary directory", filepath.Base(path)) - } - } - entries, err := os.ReadDir(source) + layers, exists, err := configurations.ProfileLayers(ctx, root, "configurations", []string{env.ConfigurationSource}) if err != nil { return err } - files := make(map[string][]byte) - for _, entry := range entries { - if !strings.HasSuffix(entry.Name(), ".env") { - continue - } - if entry.Name() == "runnable-bindings.env" { - return fmt.Errorf("configuration-source must not contain environment-specific runnable bindings") + if !exists { + return fmt.Errorf("configuration-source profile does not exist") + } + for _, layer := range layers { + if layer == destination { + return fmt.Errorf("configuration-source must not derive from its destination") } - info, err := entry.Info() - if err != nil || !info.Mode().IsRegular() { - return fmt.Errorf("configuration source %s must be a regular file", entry.Name()) + resolved, err := filepath.EvalSymlinks(layer) + if err != nil || resolved != layer { + return fmt.Errorf("configuration source must be an ordinary profile directory") } - body, err := os.ReadFile(filepath.Join(source, entry.Name())) + entries, err := os.ReadDir(layer) if err != nil { return err } - if strings.HasSuffix(entry.Name(), ".secret.env") { - for _, line := range strings.Split(string(body), "\n") { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - _, value, found := strings.Cut(line, "=") - if !found || strings.TrimSpace(value) != "" { - return fmt.Errorf("configuration-source secret file %s must contain empty placeholders only", entry.Name()) - } + for _, entry := range entries { + if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { + continue } - } - output := filepath.Join(destination, entry.Name()) - if info, err := os.Lstat(output); err == nil { - if !info.Mode().IsRegular() { - return fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) + info, err := entry.Info() + if err != nil || !info.Mode().IsRegular() { + return fmt.Errorf("configuration source %s must be a regular file", entry.Name()) } - existing, err := os.ReadFile(output) - if err != nil || !strings.HasPrefix(string(existing), profileMarker) { - return fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) + if entry.Name() == "runnable-bindings.env" { + return fmt.Errorf("configuration-source must not contain environment-specific runnable bindings") + } + if strings.HasSuffix(entry.Name(), ".secret.env") { + body, err := os.ReadFile(filepath.Join(layer, entry.Name())) + if err != nil { + return err + } + for _, line := range strings.Split(string(body), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + _, value, found := strings.Cut(line, "=") + if !found || strings.TrimSpace(value) != "" { + return fmt.Errorf("configuration-source secret file %s must contain empty placeholders only", entry.Name()) + } + } } - } else if !os.IsNotExist(err) { - return err } - files[output] = append([]byte(profileMarker+env.ConfigurationSource+"/"+entry.Name()+"\n"), body...) } + if info, err := os.Lstat(destination); err == nil && !info.IsDir() { + return fmt.Errorf("generated profile must be an ordinary directory") + } else if err != nil && !os.IsNotExist(err) { + return err + } + output := filepath.Join(destination, configurations.ProfileDerivationFile) var retired []string previous, err := os.ReadDir(destination) if err != nil && !os.IsNotExist(err) { return err } for _, entry := range previous { - path := filepath.Join(destination, entry.Name()) - if _, current := files[path]; current || !strings.HasSuffix(entry.Name(), ".env") { + if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { continue } + path := filepath.Join(destination, entry.Name()) info, err := entry.Info() if err != nil || !info.Mode().IsRegular() { return fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) @@ -113,7 +112,11 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env if err != nil { return err } - if strings.HasPrefix(string(body), profileMarker+env.ConfigurationSource+"/") { + owned := strings.HasPrefix(string(body), profileMarker) + if !owned && entry.Name() != "runnable-bindings.env" { + return fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) + } + if path != output && owned { retired = append(retired, path) } } @@ -123,10 +126,9 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env if err := os.MkdirAll(destination, 0o755); err != nil { return err } - for path, body := range files { - if err := shared.WriteFileAtomic(ctx, path, body, 0o644); err != nil { - return err - } + body := profileMarker + env.ConfigurationSource + "\nderives-from: " + env.ConfigurationSource + "\n" + if err := shared.WriteFileAtomic(ctx, output, []byte(body), 0o644); err != nil { + return err } for _, path := range retired { if err := os.Remove(path); err != nil { diff --git a/cmd/environment/configuration_profile_test.go b/cmd/environment/configuration_profile_test.go index 03e2730ae..6ebcee99f 100644 --- a/cmd/environment/configuration_profile_test.go +++ b/cmd/environment/configuration_profile_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/configurations" "github.com/stretchr/testify/require" ) @@ -28,25 +29,68 @@ func TestImportedProfilesSeparateSharedInputsAndEnvironmentOutputs(t *testing.T) root := profileSource(t) for _, name := range []string{"blue", "green"} { require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment(name), false)) - body, err := os.ReadFile(filepath.Join(root, "configurations", name, "platform.env")) + body, err := os.ReadFile(filepath.Join(root, "configurations", name, configurations.ProfileDerivationFile)) require.NoError(t, err) require.True(t, strings.HasPrefix(string(body), profileMarker)) - require.Contains(t, string(body), "${endpoint:saas/accounts/authority}") + require.Contains(t, string(body), "derives-from: hosted") + provided, err := configurations.LoadProfileConfigurations(t.Context(), root, "configurations", []string{name}) + require.NoError(t, err) + values := map[string]string{} + for _, info := range provided.Infos { + for _, value := range info.ConfigurationValues { + values[info.Name+"/"+value.Key] = value.Value + } + } + require.Equal(t, "${endpoint:saas/accounts/authority}", values["platform/endpoint"]) } blue := filepath.Join(root, "configurations", "blue") green := filepath.Join(root, "configurations", "green") require.NoError(t, os.WriteFile(filepath.Join(blue, "runnable-bindings.env"), []byte("# generated by runnable-bindings\nBLUE=resolved-blue\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(blue, "platform.env"), []byte(profileMarker+"hosted/platform.env\nendpoint=legacy-copy\n"), 0o644)) require.NoError(t, os.Remove(filepath.Join(root, "configurations", "hosted", "platform.env"))) require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), false)) _, err := os.Stat(filepath.Join(blue, "platform.env")) require.True(t, os.IsNotExist(err), "removed source groups retire only import-owned outputs") - _, err = os.Stat(filepath.Join(green, "platform.env")) + _, err = os.Stat(filepath.Join(green, configurations.ProfileDerivationFile)) require.NoError(t, err, "importing blue never rewrites green") body, err := os.ReadFile(filepath.Join(blue, "runnable-bindings.env")) require.NoError(t, err) require.Contains(t, string(body), "BLUE=resolved-blue", "the runnable generator owns this file") } +func TestImportedProfileUsesCoreStageDerivationAndValidatesInheritedSecrets(t *testing.T) { + root := profileSource(t) + source := filepath.Join(root, "configurations", "hosted-development") + require.NoError(t, os.Mkdir(source, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(source, configurations.ProfileDerivationFile), []byte("derives-from: hosted\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(source, "legal.env"), []byte("ENTITY=Development fixture\n"), 0o644)) + env := profileEnvironment("blue") + env.ConfigurationSource = "hosted-development" + require.NoError(t, materializeConfigurationProfile(t.Context(), root, env, false)) + provided, err := configurations.LoadProfileConfigurations(t.Context(), root, "configurations", []string{"blue", "local"}) + require.NoError(t, err) + groups := map[string]bool{} + for _, info := range provided.Infos { + groups[info.Name] = true + } + require.True(t, groups["platform"]) + require.True(t, groups["legal"]) + require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("production"), false)) + production, err := configurations.LoadProfileConfigurations(t.Context(), root, "configurations", []string{"production", "local"}) + require.NoError(t, err) + for _, info := range production.Infos { + require.NotEqual(t, "legal", info.Name) + } + require.NoError(t, os.WriteFile(filepath.Join(root, "configurations", "hosted", "internal-auth.secret.env"), []byte("TOKEN=fixture-value\n"), 0o600)) + env = profileEnvironment("green") + env.ConfigurationSource = "hosted-development" + err = materializeConfigurationProfile(t.Context(), root, env, false) + require.ErrorContains(t, err, "empty placeholders only") + require.NotContains(t, err.Error(), "fixture-value") + _, err = os.Stat(filepath.Join(root, "configurations", "green")) + require.True(t, os.IsNotExist(err)) +} + func TestProfileImportDryRunCreatesNothing(t *testing.T) { root := profileSource(t) require.NoError(t, materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), true)) diff --git a/docs/commands.md b/docs/commands.md index 1e3d896db..a64f6628d 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2215,13 +2215,16 @@ workspace is not ready, the command prints the diagnostics and exits non-zero. The merged file is still written, so the diagnostics can be read against it. An imported environment may declare `configuration-source: hosted` and select -`configuration-profiles: [blue, local]`. Import materializes the authored -`configurations/hosted/*.env` inputs into the generated `configurations/blue/` -profile. Secret source files must contain empty placeholders. Import refuses -symlinks and authored destination files; dry-run writes nothing. Removed source -groups retire only import-owned files, preserving the environment's separately -generated runnable bindings. Run `codefly generate runnable-bindings --env blue` -after import so addresses are resolved for blue rather than copied from a sibling. +`configuration-profiles: [blue, local]`. Import generates a `profile.codefly.yaml` +in `configurations/blue/` deriving from `hosted`, using Core's profile inheritance. +The profile chain selects the first directory at each location; inheritance adds +shared groups and per-profile values. Import validates every inherited source: +secret files contain empty placeholders, symlinks are refused, and sources may +not contain environment-specific runnable bindings. Dry-run writes nothing. +Import refuses authored destination files and retires only its own earlier copied +outputs, preserving the environment's separately generated runnable bindings. +Run `codefly generate runnable-bindings --env blue` after import so addresses are +resolved for blue rather than copied from a sibling. The cell inventory also carries `source.path` when publication uses a root other than `deployments`. Consumers must use that root to discover the published module From 6834702b69a724436e432d3d8acfbb1ff4c95cb9 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Fri, 23 Oct 2026 04:20:55 +0000 Subject: [PATCH 04/14] fix(deploy): name the service whose build plan is refused --- pkg/orchestration/builder.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/orchestration/builder.go b/pkg/orchestration/builder.go index 60248a61b..d337c3379 100644 --- a/pkg/orchestration/builder.go +++ b/pkg/orchestration/builder.go @@ -338,6 +338,7 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { OutputDirectory: outputDir, }) if err != nil { + w.Warn("cannot ask the agent for its image build plan", wool.Field("service", b.instance.Unique()), wool.ErrField(err)) return nil, w.Wrapf(err, "cannot call build") } if resp == nil { From 11991170aa6dd33ef0b4293ab138bd91937be22c Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Fri, 23 Oct 2026 04:51:30 +0000 Subject: [PATCH 05/14] fix: preserve imported resource values in qualified configuration --- cmd/environment/configuration_profile.go | 2 +- cmd/environment/configuration_profile_test.go | 10 ++++++++ docs/commands.md | 5 ++-- pkg/environments/environment.go | 24 ++++++++++--------- pkg/environments/environment_test.go | 22 +++++++++-------- pkg/gitops/configuration_projection_test.go | 17 +++++++++++++ 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/cmd/environment/configuration_profile.go b/cmd/environment/configuration_profile.go index 33f6d7154..c1dd849e5 100644 --- a/cmd/environment/configuration_profile.go +++ b/cmd/environment/configuration_profile.go @@ -61,7 +61,7 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env } for _, entry := range entries { if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { - continue + return fmt.Errorf("configuration-source supports only flat .env inputs and profile derivation metadata: %s", entry.Name()) } info, err := entry.Info() if err != nil || !info.Mode().IsRegular() { diff --git a/cmd/environment/configuration_profile_test.go b/cmd/environment/configuration_profile_test.go index 6ebcee99f..132b62cd6 100644 --- a/cmd/environment/configuration_profile_test.go +++ b/cmd/environment/configuration_profile_test.go @@ -108,6 +108,16 @@ func TestProfileImportRefusesCredentialValuesBeforeWriting(t *testing.T) { require.True(t, os.IsNotExist(err)) } +func TestProfileImportRefusesUnvalidatedInheritedInputFormats(t *testing.T) { + root := profileSource(t) + require.NoError(t, os.WriteFile(filepath.Join(root, "configurations", "hosted", "credentials.secret.yaml"), []byte("token: fixture-value\n"), 0o600)) + err := materializeConfigurationProfile(t.Context(), root, profileEnvironment("blue"), false) + require.ErrorContains(t, err, "only flat .env inputs") + require.NotContains(t, err.Error(), "fixture-value") + _, err = os.Stat(filepath.Join(root, "configurations", "blue")) + require.True(t, os.IsNotExist(err)) +} + func TestProfileImportRefusesAuthoredOrSymlinkedFiles(t *testing.T) { for _, symlink := range []bool{false, true} { t.Run(map[bool]string{false: "authored", true: "symlink"}[symlink], func(t *testing.T) { diff --git a/docs/commands.md b/docs/commands.md index a64f6628d..e82bc4206 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2219,7 +2219,8 @@ An imported environment may declare `configuration-source: hosted` and select in `configurations/blue/` deriving from `hosted`, using Core's profile inheritance. The profile chain selects the first directory at each location; inheritance adds shared groups and per-profile values. Import validates every inherited source: -secret files contain empty placeholders, symlinks are refused, and sources may +only flat `.env` inputs and profile derivation metadata are accepted; secret +files contain empty placeholders, symlinks are refused, and sources may not contain environment-specific runnable bindings. Dry-run writes nothing. Import refuses authored destination files and retires only its own earlier copied outputs, preserving the environment's separately generated runnable bindings. @@ -2232,7 +2233,7 @@ trees. The local render directory remains a qualification tree; `publish` writes the environment's declared publication path. `service-config.services` accepts `/` keys for compositions -with same-named services. A qualified entry wins over the bare name. Unknown +with same-named services. Qualified values override bare values per key. Unknown qualified entries and ambiguous bare entries fail workspace validation. #### `codefly environment show` diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index 593ea2128..a5c85d542 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -937,18 +937,18 @@ type EnvironmentServiceConfig struct { Services map[string]EnvironmentServiceConfigMapping `yaml:"services,omitempty"` } -// ValuesFor selects the qualified workload first, then a bare service entry. +// ValuesFor overlays qualified values on the bare service's values, per key. // Workspace validation refuses a bare entry shared by multiple modules. func (c *EnvironmentServiceConfig) ValuesFor(module, service string) map[string]string { if c == nil { return nil } + values := make(map[string]string) + maps.Copy(values, c.Services[service].Values) if module != "" { - if mapping, ok := c.Services[module+"/"+service]; ok { - return mapping.Values - } + maps.Copy(values, c.Services[module+"/"+service].Values) } - return c.Services[service].Values + return values } // EnvironmentServiceConfigMapping holds one service's resolved values keyed by @@ -1184,13 +1184,15 @@ func (env *Environment) validateServiceKeyCollisions() error { return nil } for name, config := range env.ServiceConfig.Services { - secrets, declared := env.ServiceSecrets.Services[name] - if !declared { - continue + secretNames := []string{name} + if _, bare, qualified := strings.Cut(name, "/"); qualified { + secretNames = append(secretNames, bare) } - for key := range config.Values { - if _, collides := secrets.RemoteKeys[key]; collides { - return fmt.Errorf("service %q declares %q as both a service-config value and a service-secrets remote key", name, key) + for _, secretName := range secretNames { + for key := range config.Values { + if _, collides := env.ServiceSecrets.Services[secretName].RemoteKeys[key]; collides { + return fmt.Errorf("service %q declares %q as both a service-config value and a service-secrets remote key", name, key) + } } } } diff --git a/pkg/environments/environment_test.go b/pkg/environments/environment_test.go index 48d10d54c..9e6461e4d 100644 --- a/pkg/environments/environment_test.go +++ b/pkg/environments/environment_test.go @@ -792,16 +792,18 @@ environments: DATABASE_HOST: accounts/prod/host ` } - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), []byte(workspaceWith("accounts")), 0o644); err != nil { - t.Fatal(err) - } - _, err := loadDeploymentWorkspace(context.Background(), root) - if err == nil { - t.Fatal("expected load to refuse a key declared as both a value and a secret") - } - if !strings.Contains(err.Error(), "DATABASE_HOST") || !strings.Contains(err.Error(), "accounts") { - t.Fatalf("error = %v, want it to name the service and the key", err) + for _, name := range []string{"accounts", "saas/accounts"} { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), []byte(workspaceWith(name)), 0o644); err != nil { + t.Fatal(err) + } + _, err := loadDeploymentWorkspace(context.Background(), root) + if err == nil { + t.Fatal("expected load to refuse a key declared as both a value and a secret") + } + if !strings.Contains(err.Error(), "DATABASE_HOST") || !strings.Contains(err.Error(), name) { + t.Fatalf("error = %v, want it to name the service and the key", err) + } } other := t.TempDir() diff --git a/pkg/gitops/configuration_projection_test.go b/pkg/gitops/configuration_projection_test.go index e44e43a5a..2baab6e83 100644 --- a/pkg/gitops/configuration_projection_test.go +++ b/pkg/gitops/configuration_projection_test.go @@ -47,6 +47,23 @@ func TestQualifiedConfigurationKeepsSameNamedWorkloadsIsolated(t *testing.T) { } } +func TestQualifiedConfigurationRetainsImportedResourceValues(t *testing.T) { + env := &environments.Environment{Name: "blue", Namespace: "product", ServiceConfig: &environments.EnvironmentServiceConfig{ + Services: map[string]environments.EnvironmentServiceConfigMapping{ + "accounts": {Values: map[string]string{"BACKEND": "managed", "PUBLIC_URL": "https://default.example"}}, + "saas/accounts": {Values: map[string]string{"PUBLIC_URL": "https://app.example"}}, + }, + }} + root := t.TempDir() + writeConsumerTree(t, root, env.Name, env.Namespace, "accounts", "declared.example") + scope := scopeOf(env) + scope.Module = "saas" + require.NoError(t, projectServiceConfiguration(t.Context(), testRenderRoot(t, root), root, &resources.Service{Name: "accounts"}, env, scope, serviceInjection{})) + values := containerEnvironment(t, buildOverlay(t, root, env.Name)) + require.Equal(t, "managed", values["BACKEND"]["value"]) + require.Equal(t, "https://app.example", values["PUBLIC_URL"]["value"]) +} + func TestActualInfraBaseValuesReachRenderedWorkload(t *testing.T) { data, err := os.ReadFile(filepath.Join("..", "environments", "testdata", "coordinates", "infra-base-lodestar.json")) require.NoError(t, err) From 568135d6809b3b7839e61c5d35eedee189b337b7 Mon Sep 17 00:00:00 2001 From: Cristopher Fiorini <301773486+crisfiorini-oss@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:49:58 -0400 Subject: [PATCH 06/14] fix(environment): import product profiles beside coordinate placement --- cmd/environment/configuration_profile.go | 87 ++++++++++++------- cmd/environment/configuration_profile_test.go | 41 +++++++++ cmd/environment/import.go | 68 +++++++++++---- docs/cli-reference.md | 8 +- docs/commands.md | 13 +++ 5 files changed, 167 insertions(+), 50 deletions(-) diff --git a/cmd/environment/configuration_profile.go b/cmd/environment/configuration_profile.go index c1dd849e5..3dc933a31 100644 --- a/cmd/environment/configuration_profile.go +++ b/cmd/environment/configuration_profile.go @@ -47,18 +47,55 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env if !exists { return fmt.Errorf("configuration-source profile does not exist") } + if err = validateConfigurationSources(layers, destination); err != nil { + return err + } + confined, err := os.OpenRoot(configurationRoot) + if err != nil { + return err + } + defer confined.Close() + retired, err := importedProfileRetirements(confined, target) + if err != nil { + return err + } + output := filepath.Join(destination, configurations.ProfileDerivationFile) + if dryRun { + return nil + } + if err := confined.MkdirAll(target, 0o755); err != nil { + return err + } + body := profileMarker + env.ConfigurationSource + "\nderives-from: " + env.ConfigurationSource + "\n" + if err := shared.WriteFileAtomic(ctx, output, []byte(body), 0o644); err != nil { + return err + } + for _, path := range retired { + if err := confined.Remove(path); err != nil { + return err + } + } + return nil +} + +func validateConfigurationSources(layers []string, destination string) error { for _, layer := range layers { if layer == destination { return fmt.Errorf("configuration-source must not derive from its destination") } - resolved, err := filepath.EvalSymlinks(layer) - if err != nil || resolved != layer { + resolved, resolveErr := filepath.EvalSymlinks(layer) + if resolveErr != nil || resolved != layer { return fmt.Errorf("configuration source must be an ordinary profile directory") } entries, err := os.ReadDir(layer) if err != nil { return err } + confined, err := os.OpenRoot(layer) + if err != nil { + return err + } + defer confined.Close() for _, entry := range entries { if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { return fmt.Errorf("configuration-source supports only flat .env inputs and profile derivation metadata: %s", entry.Name()) @@ -71,7 +108,7 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env return fmt.Errorf("configuration-source must not contain environment-specific runnable bindings") } if strings.HasSuffix(entry.Name(), ".secret.env") { - body, err := os.ReadFile(filepath.Join(layer, entry.Name())) + body, err := confined.ReadFile(entry.Name()) if err != nil { return err } @@ -88,52 +125,40 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env } } } - if info, err := os.Lstat(destination); err == nil && !info.IsDir() { - return fmt.Errorf("generated profile must be an ordinary directory") + return nil +} + +func importedProfileRetirements(confined *os.Root, target string) ([]string, error) { + if info, err := confined.Lstat(target); err == nil && !info.IsDir() { + return nil, fmt.Errorf("generated profile must be an ordinary directory") } else if err != nil && !os.IsNotExist(err) { - return err + return nil, err } - output := filepath.Join(destination, configurations.ProfileDerivationFile) var retired []string - previous, err := os.ReadDir(destination) + previous, err := os.ReadDir(filepath.Join(confined.Name(), target)) if err != nil && !os.IsNotExist(err) { - return err + return nil, err } for _, entry := range previous { if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { continue } - path := filepath.Join(destination, entry.Name()) + path := filepath.Join(target, entry.Name()) info, err := entry.Info() if err != nil || !info.Mode().IsRegular() { - return fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) + return nil, fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) } - body, err := os.ReadFile(path) + body, err := confined.ReadFile(filepath.Join(target, entry.Name())) if err != nil { - return err + return nil, err } owned := strings.HasPrefix(string(body), profileMarker) if !owned && entry.Name() != "runnable-bindings.env" { - return fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) + return nil, fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) } - if path != output && owned { + if entry.Name() != configurations.ProfileDerivationFile && owned { retired = append(retired, path) } } - if dryRun { - return nil - } - if err := os.MkdirAll(destination, 0o755); err != nil { - return err - } - body := profileMarker + env.ConfigurationSource + "\nderives-from: " + env.ConfigurationSource + "\n" - if err := shared.WriteFileAtomic(ctx, output, []byte(body), 0o644); err != nil { - return err - } - for _, path := range retired { - if err := os.Remove(path); err != nil { - return err - } - } - return nil + return retired, nil } diff --git a/cmd/environment/configuration_profile_test.go b/cmd/environment/configuration_profile_test.go index 132b62cd6..e388c5374 100644 --- a/cmd/environment/configuration_profile_test.go +++ b/cmd/environment/configuration_profile_test.go @@ -1,6 +1,7 @@ package environment import ( + "bytes" "os" "path/filepath" "strings" @@ -11,6 +12,46 @@ import ( "github.com/stretchr/testify/require" ) +func TestImportProductProfileIntoExistingPlacement(t *testing.T) { + root := profileSource(t) + workspace := "name: acme\nlayout: modules\nenvironments:\n- name: azure\n namespace: acme\n service-identity:\n services:\n model/model:\n principal: applied-client\n" + require.NoError(t, os.WriteFile(filepath.Join(root, "workspace.codefly.yaml"), []byte(workspace), 0o644)) + opts := importOptions{configurationSource: "hosted", configurationProfiles: []string{"azure", "local"}, dryRun: true} + doImport(t, root, opts) + require.Equal(t, workspace, readFile(t, filepath.Join(root, "workspace.codefly.yaml"))) + _, err := os.Stat(filepath.Join(root, "configurations", "azure")) + require.True(t, os.IsNotExist(err)) + opts.dryRun = false + doImport(t, root, opts) + env := loadWorkspace(t, root).FindEnvironment("azure") + require.Equal(t, "hosted", env.ConfigurationSource) + require.Equal(t, []string{"azure", "local"}, env.ConfigurationProfiles) + require.Contains(t, readFile(t, filepath.Join(root, "workspace.codefly.yaml")), "principal: applied-client") + profile := filepath.Join(root, "configurations", "azure", configurations.ProfileDerivationFile) + require.Contains(t, readFile(t, profile), "derives-from: hosted") + // A later placement refresh keeps and regenerates the product selection. + require.NoError(t, os.Remove(profile)) + doImport(t, root, importOptions{}) + require.Contains(t, readFile(t, profile), "derives-from: hosted") +} + +func TestImportProductProfileRefusesCoordinateConflict(t *testing.T) { + for _, declared := range []string{ + `"configuration-source":"other"`, + `"configuration-profile":"other"`, + `"configuration-profiles":["other"]`, + } { + t.Run(declared, func(t *testing.T) { + root := profileSource(t) + workspace := "name: acme\nlayout: modules\n" + require.NoError(t, os.WriteFile(filepath.Join(root, "workspace.codefly.yaml"), []byte(workspace), 0o644)) + opts := importOptions{dir: root, envName: "azure", configurationSource: "hosted", configurationProfiles: []string{"azure", "local"}, contractData: []byte(`{"schema":"codefly/coordinate/v1","environment":{"name":"azure",` + declared + `}}`), stdout: &bytes.Buffer{}} + require.Error(t, runImport(t.Context(), &opts)) + require.Equal(t, workspace, readFile(t, filepath.Join(root, "workspace.codefly.yaml"))) + }) + } +} + func profileSource(t *testing.T) string { t.Helper() root := t.TempDir() diff --git a/cmd/environment/import.go b/cmd/environment/import.go index e80424f34..b2f12a1d8 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -65,6 +65,8 @@ entries are preserved, comments included. Read from stdin with if err != nil { return err } + configurationSource, _ := cmd.Flags().GetString("configuration-source") + configurationProfiles, _ := cmd.Flags().GetStringSlice("configuration-profiles") dir, err := resources.FindUp[resources.Workspace](ctx) if err != nil || dir == nil { @@ -72,12 +74,14 @@ entries are preserved, comments included. Read from stdin with } opts := importOptions{ - dir: *dir, - envName: args[0], - contractData: data, - namespace: importNamespace, - namespaceSet: cmd.Flags().Changed("namespace"), - dryRun: importDryRun, + dir: *dir, + envName: args[0], + contractData: data, + namespace: importNamespace, + namespaceSet: cmd.Flags().Changed("namespace"), + dryRun: importDryRun, + configurationSource: configurationSource, + configurationProfiles: configurationProfiles, // UTC so the provenance timestamp does not vary by the operator's // machine timezone and churn the diff on re-import across a team. now: time.Now().UTC(), @@ -133,14 +137,16 @@ func readContract(stdin io.Reader, path string) ([]byte, error) { } type importOptions struct { - dir string - envName string - contractData []byte - namespace string - namespaceSet bool - dryRun bool - now time.Time - stdout io.Writer + dir string + envName string + contractData []byte + namespace string + namespaceSet bool + dryRun bool + now time.Time + stdout io.Writer + configurationSource string + configurationProfiles []string } // runImport parses the contract, merges its owned fields into the named @@ -164,6 +170,30 @@ func runImport(ctx context.Context, opts *importOptions) error { return err } declaredEnvironment := yamledit.MapValue(declaration, "environment") + // Product profiles are owned by the consuming workspace. A placement-only + // producer need not acquire product configuration to import its target. + if opts.configurationSource != "" { + if existing := contract.Environment.ConfigurationSource; existing != "" && existing != opts.configurationSource { + return fmt.Errorf("--configuration-source conflicts with the coordinate declaration") + } + contract.Environment.ConfigurationSource = opts.configurationSource + node, encodeErr := yamledit.Encode(opts.configurationSource) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-source", node) + } + if len(opts.configurationProfiles) > 0 { + if contract.Environment.ConfigurationProfile != "" || len(contract.Environment.ConfigurationProfiles) > 0 { + return fmt.Errorf("--configuration-profiles cannot replace coordinate-declared profiles") + } + contract.Environment.ConfigurationProfiles = opts.configurationProfiles + node, encodeErr := yamledit.Encode(opts.configurationProfiles) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-profiles", node) + } file := filepath.Join(opts.dir, resources.WorkspaceConfigurationName) original, err := os.ReadFile(file) @@ -220,10 +250,14 @@ func runImport(ctx context.Context, opts *importOptions) error { if err != nil { return err } - if _, err := environments.FromWorkspace(merged); err != nil { + if _, err = environments.FromWorkspace(merged); err != nil { return fmt.Errorf("merged environment declarations: %w", err) } - if err := materializeConfigurationProfile(ctx, opts.dir, env, opts.dryRun); err != nil { + selected, err := environments.FromRuntime(merged.FindEnvironment(opts.envName)) + if err != nil { + return err + } + if err := materializeConfigurationProfile(ctx, opts.dir, selected, opts.dryRun); err != nil { return err } if opts.dryRun { @@ -433,6 +467,8 @@ func stampProvenance(envNode *yaml.Node, contract *environments.CoordinateContra func init() { importCmd.Flags().String("coordinate-contract", "", "Path to a codefly/coordinate/v1 descriptor, or - for stdin") + importCmd.Flags().String("configuration-source", "", "Authored product profile inherited by the imported environment") + importCmd.Flags().StringSlice("configuration-profiles", nil, "Product configuration profile chain when absent from the coordinate declaration") importCmd.Flags().String("cell-contract", "", "Former spelling of --coordinate-contract") _ = importCmd.Flags().MarkDeprecated("cell-contract", "use --coordinate-contract") importCmd.Flags().StringVar(&importNamespace, "namespace", "", "Assert the namespace declared by the producer") diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 9cca565fc..8a48b2cb5 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2515,9 +2515,11 @@ codefly environment import --coordinate-contract Flags: ``` - --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin - --dry-run Print the unified diff of workspace.codefly.yaml and write nothing - --namespace string Assert the namespace declared by the producer + --configuration-profiles strings Product configuration profile chain when absent from the coordinate declaration + --configuration-source string Authored product profile inherited by the imported environment + --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin + --dry-run Print the unified diff of workspace.codefly.yaml and write nothing + --namespace string Assert the namespace declared by the producer ``` ## `codefly environment show` diff --git a/docs/commands.md b/docs/commands.md index e82bc4206..2e32f9f71 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2227,6 +2227,19 @@ outputs, preserving the environment's separately generated runnable bindings. Run `codefly generate runnable-bindings --env blue` after import so addresses are resolved for blue rather than copied from a sibling. +For a placement-only coordinate, the product may select its own source at import: + +```bash +codefly environment import azure --coordinate-contract coordinate.json \ + --configuration-source hosted-azure --configuration-profiles azure,local +``` + +These options use the same importer and profile inheritance. A source conflicting +with the coordinate is refused; an explicit profile chain cannot replace a chain +or single profile declared by the coordinate. Without a profile chain, the target +defaults to the environment name. Later placement-only imports preserve the +product selection and regenerate its inherited profile. + The cell inventory also carries `source.path` when publication uses a root other than `deployments`. Consumers must use that root to discover the published module trees. The local render directory remains a qualification tree; `publish` writes From d00219b105aca8f7fbbb0b2859dab16c51908b47 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 17:01:03 +0000 Subject: [PATCH 07/14] fix(environments): validate qualified secret services against their module --- pkg/environments/environment_test.go | 18 ++++++++++-------- pkg/environments/workspace.go | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/environments/environment_test.go b/pkg/environments/environment_test.go index 9e6461e4d..7d0836e8d 100644 --- a/pkg/environments/environment_test.go +++ b/pkg/environments/environment_test.go @@ -90,10 +90,11 @@ func serviceIdentityDeclaration(serviceName string) string { // service passes the graph check. func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accounts"), - "service-config": serviceConfigDeclaration("accounts"), - "qualified service-config": serviceConfigDeclaration("saas/accounts"), - "service-identity": serviceIdentityDeclaration("accounts"), + "service-secrets": serviceSecretsDeclaration("accounts"), + "qualified service-secrets": serviceSecretsDeclaration("saas/accounts"), + "service-config": serviceConfigDeclaration("accounts"), + "qualified service-config": serviceConfigDeclaration("saas/accounts"), + "service-identity": serviceIdentityDeclaration("accounts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() @@ -113,10 +114,11 @@ func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { // projection time; the graph check must reject it. func TestValidateEnvironmentsRejectsUnknownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accunts"), - "service-config": serviceConfigDeclaration("accunts"), - "qualified service-config": serviceConfigDeclaration("wrong/accounts"), - "service-identity": serviceIdentityDeclaration("accunts"), + "service-secrets": serviceSecretsDeclaration("accunts"), + "qualified service-secrets": serviceSecretsDeclaration("wrong/accounts"), + "service-config": serviceConfigDeclaration("accunts"), + "qualified service-config": serviceConfigDeclaration("wrong/accounts"), + "service-identity": serviceIdentityDeclaration("accunts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index 9cc782f9b..337b9e99e 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -226,7 +226,7 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) erro // A module-qualified service-identity key names exactly one // service of that module, or nothing (a typo that would leave // the workload on the default identity with no report). - if _, service, qualified := strings.Cut(name, "/"); qualified && (block == "service-identity" || block == "service-mesh" || block == "service-config") { + if _, service, qualified := strings.Cut(name, "/"); qualified { if !slices.Contains(byName[service], name) { return w.Wrap(fmt.Errorf("environment %q %s references unknown service %q", env.Name, block, name)) } From 0614c4316c05f8588d8cfaf9f6ac6b8e693c19ec Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 19:31:52 +0000 Subject: [PATCH 08/14] fix(signing): retain credential-exchange status without response contents The Google OAuth impersonation and STS token sources return status plus response body in an untyped error. Preserve only the recognized HTTP status so a refused signer credential is distinguishable from a KMS transport failure; never print the response body. Reuse the existing custody transport and add a sanitizer regression. CloudKMS package tests pass. --- pkg/delivery/signing/cloud_kms.go | 8 ++++++++ pkg/delivery/signing/cloud_kms_test.go | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/pkg/delivery/signing/cloud_kms.go b/pkg/delivery/signing/cloud_kms.go index 6e92ab6d8..7d3c6340c 100644 --- a/pkg/delivery/signing/cloud_kms.go +++ b/pkg/delivery/signing/cloud_kms.go @@ -44,6 +44,7 @@ const kmsScope = "https://www.googleapis.com/auth/cloudkms" const kmsVersionShape = "projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/" var kmsEnumName = regexp.MustCompile(`^[A-Z0-9_]{1,64}$`) +var googleCredentialStatus = regexp.MustCompile(`^oauth2/google(?:/externalaccount)?: status code ([1-5][0-9]{2}):`) // CloudKMS is the optional adapter the handbook decision admits behind the // same custody interface as Vault Transit: public metadata and an @@ -211,6 +212,13 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er if errors.As(err, &exchange) && exchange.Response != nil { return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %d)", exchange.Response.StatusCode) } + // Google's impersonation/STS token sources return an untyped error + // containing the response body. Retain only its known status prefix. + for cause := err; cause != nil; cause = errors.Unwrap(cause) { + if status := googleCredentialStatus.FindStringSubmatch(cause.Error()); status != nil { + return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %s)", status[1]) + } + } return fmt.Errorf("the Cloud KMS %s request failed", stage) } defer response.Body.Close() diff --git a/pkg/delivery/signing/cloud_kms_test.go b/pkg/delivery/signing/cloud_kms_test.go index d1f03d6df..c140fc05d 100644 --- a/pkg/delivery/signing/cloud_kms_test.go +++ b/pkg/delivery/signing/cloud_kms_test.go @@ -11,6 +11,7 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "errors" "fmt" "io" "net/http" @@ -257,6 +258,20 @@ func TestCloudKMSRefusalsNeverExposeToken(t *testing.T) { } } +type failedKMSCredential struct{ err error } + +func (c failedKMSCredential) Token() (*oauth2.Token, error) { return nil, c.err } + +func TestCloudKMSCredentialRefusalsKeepStatusWithoutResponseBody(t *testing.T) { + for _, prefix := range []string{"oauth2/google", "oauth2/google/externalaccount"} { + credential := failedKMSCredential{errors.New(prefix + ": status code 403: private-test-token")} + _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) + if err == nil || !strings.Contains(err.Error(), "HTTP 403") || strings.Contains(err.Error(), "private-test-token") { + t.Fatalf("unsafe or unhelpful credential refusal: %v", err) + } + } +} + func TestCloudKMSRejectsUnsafeConfiguration(t *testing.T) { for _, endpoint := range []string{"http://example.test", "http://192.0.2.1", "ftp://localhost", "https://private-test-token@example.test", "https://example.test?token=private-test-token", // The bearer token belongs to Google's hosts only. From f3e35b58b67e259608a60bc75a1af25515103f85 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 20:30:34 +0000 Subject: [PATCH 09/14] fix(signing): expose allowlisted credential exchange reasons --- pkg/delivery/signing/cloud_kms.go | 26 ++++++++++++++++++++++++-- pkg/delivery/signing/cloud_kms_test.go | 14 ++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/pkg/delivery/signing/cloud_kms.go b/pkg/delivery/signing/cloud_kms.go index 7d3c6340c..80675eced 100644 --- a/pkg/delivery/signing/cloud_kms.go +++ b/pkg/delivery/signing/cloud_kms.go @@ -210,13 +210,13 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er if err != nil { var exchange *oauth2.RetrieveError if errors.As(err, &exchange) && exchange.Response != nil { - return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %d)", exchange.Response.StatusCode) + return kmsCredentialRefusal(strconv.Itoa(exchange.Response.StatusCode), string(exchange.Body)) } // Google's impersonation/STS token sources return an untyped error // containing the response body. Retain only its known status prefix. for cause := err; cause != nil; cause = errors.Unwrap(cause) { if status := googleCredentialStatus.FindStringSubmatch(cause.Error()); status != nil { - return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %s)", status[1]) + return kmsCredentialRefusal(status[1], strings.TrimPrefix(cause.Error(), status[0])) } } return fmt.Errorf("the Cloud KMS %s request failed", stage) @@ -231,6 +231,28 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er return nil } +// Only named Google error reasons may leave a credential response; messages and +// metadata can echo credentials or request bodies. +func kmsCredentialRefusal(status, body string) error { + message := fmt.Sprintf("the Google credential was not exchanged for a Cloud KMS token (HTTP %s)", status) + var response struct { + Error struct { + Details []struct { + Reason string `json:"reason"` + } `json:"details"` + } `json:"error"` + } + if len(body) <= 65536 && json.Unmarshal([]byte(body), &response) == nil { + for _, detail := range response.Error.Details { + switch detail.Reason { + case "ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "SERVICE_DISABLED", "SECURITY_POLICY_VIOLATED": + return fmt.Errorf("%s: %s", message, detail.Reason) + } + } + } + return errors.New(message) +} + // kmsHost admits only Google's API hosts, where the ambient credential's // bearer token belongs, and loopback for a fake. func kmsHost(address string) bool { diff --git a/pkg/delivery/signing/cloud_kms_test.go b/pkg/delivery/signing/cloud_kms_test.go index c140fc05d..cab905ec1 100644 --- a/pkg/delivery/signing/cloud_kms_test.go +++ b/pkg/delivery/signing/cloud_kms_test.go @@ -272,6 +272,20 @@ func TestCloudKMSCredentialRefusalsKeepStatusWithoutResponseBody(t *testing.T) { } } +func TestCloudKMSCredentialReasonsAreAllowlisted(t *testing.T) { + for _, reason := range []string{"ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "private-test-token"} { + body := `{"error":{"message":"private-test-token","details":[{"reason":"` + reason + `"}]}}` + credential := failedKMSCredential{errors.New("oauth2/google: status code 403: " + body)} + _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) + if err == nil || strings.Contains(err.Error(), "private-test-token") || !strings.Contains(err.Error(), "HTTP 403") { + t.Fatalf("unsafe or unhelpful refusal: %v", err) + } + if reason != "private-test-token" && !strings.Contains(err.Error(), reason) { + t.Fatalf("known reason omitted: %v", err) + } + } +} + func TestCloudKMSRejectsUnsafeConfiguration(t *testing.T) { for _, endpoint := range []string{"http://example.test", "http://192.0.2.1", "ftp://localhost", "https://private-test-token@example.test", "https://example.test?token=private-test-token", // The bearer token belongs to Google's hosts only. From 03ee3053d6eeca477b1122f8f383709b23e42de2 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 20:43:01 +0000 Subject: [PATCH 10/14] fix(gitops): open promotion pull requests as drafts --- pkg/gitops/publish.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index 00edd3c6d..07ae76a26 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -2134,6 +2134,7 @@ func openOrUpdatePullRequest(ctx context.Context, prepared *preparedRepository, Head: github.Ptr(prepared.plan.PromotionBranch), Base: github.Ptr(prepared.plan.BaseBranch), Body: github.Ptr(body), + Draft: github.Ptr(true), }) if err != nil { return "", 0, fmt.Errorf("open promotion pull request: %w", err) From 9be4928dd7c02a9d4e2a3fd32841128c3247c057 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 21:11:44 +0000 Subject: [PATCH 11/14] fix(gitops): honor module-qualified secret mappings --- pkg/environments/environment.go | 33 +++++++++++++++++++++++++- pkg/gitops/configuration_projection.go | 4 ++-- pkg/gitops/configuration_templates.go | 2 +- pkg/gitops/secret_projection.go | 2 +- pkg/gitops/secret_projection_test.go | 29 ++++++++++++++++++++++ 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index a5c85d542..03db4161e 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -1108,6 +1108,37 @@ type SecretScope struct { Service string } +// MappingFor overlays a module-qualified declaration on the bare service mapping, +// matching ValuesFor without changing either declaration. +func (s *EnvironmentServiceSecrets) MappingFor(module, service string) EnvironmentServiceSecretMapping { + if s == nil { + return EnvironmentServiceSecretMapping{} + } + mapping := s.Services[service] + if module == "" { + return mapping + } + qualified := s.Services[module+"/"+service] + if len(qualified.RemoteKeys) != 0 { + mapping.RemoteKeys = make(map[string]EnvironmentSecretRemoteRef, len(mapping.RemoteKeys)+len(qualified.RemoteKeys)) + maps.Copy(mapping.RemoteKeys, s.Services[service].RemoteKeys) + maps.Copy(mapping.RemoteKeys, qualified.RemoteKeys) + } + if qualified.SecretStore != nil { + mapping.SecretStore = qualified.SecretStore + } + if qualified.Defaults != nil { + mapping.Defaults = qualified.Defaults + } + if qualified.Template != nil { + mapping.Template = qualified.Template + } + if qualified.RefreshInterval != "" { + mapping.RefreshInterval = qualified.RefreshInterval + } + return mapping +} + // RemoteRef locates one secret key of a service in the remote store. An explicit // per-service RemoteKeys entry wins; else the service's own Defaults template; // else the environment-wide Defaults template; else the "/" store @@ -1116,7 +1147,7 @@ type SecretScope struct { // fallback path, the same as an environment that declares nothing. func (s *EnvironmentServiceSecrets) RemoteRef(scope SecretScope, key string) EnvironmentSecretRemoteRef { if s != nil { - mapping := s.Services[scope.Service] + mapping := s.MappingFor(scope.Module, scope.Service) if remote, ok := mapping.RemoteKeys[key]; ok { return remote } diff --git a/pkg/gitops/configuration_projection.go b/pkg/gitops/configuration_projection.go index 97a7b0bbf..45e45e977 100644 --- a/pkg/gitops/configuration_projection.go +++ b/pkg/gitops/configuration_projection.go @@ -35,7 +35,7 @@ func projectConfigurationValuesInTree(ctx context.Context, tree *renderRoot, roo values := env.ServiceConfig.ValuesFor(module, service) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { - refs = env.ServiceSecrets.Services[service].RemoteKeys + refs = env.ServiceSecrets.MappingFor(module, service).RemoteKeys } if len(values) == 0 && len(refs) == 0 { return nil @@ -190,7 +190,7 @@ func validateProjectedConfigurationInTree(tree *renderRoot, root string, service values := env.ServiceConfig.ValuesFor(scope.Module, service.Name) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { - refs = env.ServiceSecrets.Services[service.Name].RemoteKeys + refs = env.ServiceSecrets.MappingFor(scope.Module, service.Name).RemoteKeys } identity, err := soleWorkloadIdentity(scope.Module, service.Name, consumedManagedServices(service, scope.Module, env), env) if err != nil { diff --git a/pkg/gitops/configuration_templates.go b/pkg/gitops/configuration_templates.go index cd79daa6b..feb3a0059 100644 --- a/pkg/gitops/configuration_templates.go +++ b/pkg/gitops/configuration_templates.go @@ -196,7 +196,7 @@ func producerPrimitiveRemote( // The producer's own projection takes its per-service override the same // way (serviceSecretProjection), so this is the store its ExternalSecret // actually reads from. - if override := secrets.Services[service].SecretStore; override != nil { + if override := secrets.MappingFor(module, service).SecretStore; override != nil { store = *override } } diff --git a/pkg/gitops/secret_projection.go b/pkg/gitops/secret_projection.go index 8476ba822..9d81e525b 100644 --- a/pkg/gitops/secret_projection.go +++ b/pkg/gitops/secret_projection.go @@ -140,7 +140,7 @@ func serviceSecretProjection(scope unitScope, service string, secrets *environme return nil, nil } namespace := scope.Namespace - mapping := secrets.Services[service] + mapping := secrets.MappingFor(scope.Module, service) // A service may resolve from a different store than the environment default, // mirroring EnvironmentManagedSecretReference's per-reference store; without // this the per-service secret-store declaration would load, validate, and then diff --git a/pkg/gitops/secret_projection_test.go b/pkg/gitops/secret_projection_test.go index c1188a65e..5a53cfd18 100644 --- a/pkg/gitops/secret_projection_test.go +++ b/pkg/gitops/secret_projection_test.go @@ -248,6 +248,35 @@ func TestServiceSecretProjectionDefaultsAndOverridesRemoteKeys(t *testing.T) { } } +func TestServiceSecretProjectionQualifiedMappingDoesNotLeakAcrossModules(t *testing.T) { + secrets := azureServiceSecrets() + secrets.Services["saas/accounts"] = environments.EnvironmentServiceSecretMapping{ + SecretStore: &environments.EnvironmentSecretStoreReference{Name: "signing", Kind: "ClusterSecretStore"}, + RemoteKeys: map[string]environments.EnvironmentSecretRemoteRef{ + "workos-client-secret": {Key: "accounts-signing", Property: "wrapped"}, + }, + Defaults: &environments.EnvironmentSecretRemoteRef{Key: "{module}-{service}", Property: "{key}"}, + RefreshInterval: "5m", + } + for _, module := range []string{"saas", "other", "saas"} { + projection, err := serviceSecretProjection(unitScope{Namespace: "product-" + module, Module: module}, + "accounts", secrets, []string{"workos-api-key", "workos-client-secret"}) + if err != nil { + t.Fatal(err) + } + if module == "saas" { + if projection.Spec.SecretStoreRef.Name != "signing" || projection.Spec.RefreshInterval != "5m" || + projection.Spec.Data[0].RemoteRef.Key != "saas-accounts" || + projection.Spec.Data[1].RemoteRef != (externalSecretRemote{Key: "accounts-signing", Property: "wrapped"}) { + t.Fatalf("qualified mapping lost: %+v", projection.Spec) + } + } else if projection.Spec.SecretStoreRef.Name != "azure-keyvault-prod" || + projection.Spec.Data[1].RemoteRef.Key != "workos/prod/client-secret" { + t.Fatalf("qualified mapping leaked to another module: %+v", projection.Spec) + } + } +} + func TestServiceSecretProjectionRendersNothingWithoutStoreOrKeys(t *testing.T) { if projection, err := serviceSecretProjection(unitScope{Namespace: "payments"}, "accounts", nil, []string{"api-key"}); err != nil || projection != nil { t.Fatalf("nil store rendered %v (err %v)", projection, err) From 862eb5bd07e7bb0a13152665a79f56ca3cd4b23a Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 21:12:22 +0000 Subject: [PATCH 12/14] refactor(secrets): reuse existing qualified mapping selection --- pkg/environments/environment.go | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index 03db4161e..1554fc13b 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -1108,35 +1108,16 @@ type SecretScope struct { Service string } -// MappingFor overlays a module-qualified declaration on the bare service mapping, -// matching ValuesFor without changing either declaration. +// MappingFor selects a module-qualified declaration before the unqualified fallback, +// matching service configuration and identity selection. func (s *EnvironmentServiceSecrets) MappingFor(module, service string) EnvironmentServiceSecretMapping { if s == nil { return EnvironmentServiceSecretMapping{} } - mapping := s.Services[service] - if module == "" { + if mapping, ok := s.Services[module+"/"+service]; module != "" && ok { return mapping } - qualified := s.Services[module+"/"+service] - if len(qualified.RemoteKeys) != 0 { - mapping.RemoteKeys = make(map[string]EnvironmentSecretRemoteRef, len(mapping.RemoteKeys)+len(qualified.RemoteKeys)) - maps.Copy(mapping.RemoteKeys, s.Services[service].RemoteKeys) - maps.Copy(mapping.RemoteKeys, qualified.RemoteKeys) - } - if qualified.SecretStore != nil { - mapping.SecretStore = qualified.SecretStore - } - if qualified.Defaults != nil { - mapping.Defaults = qualified.Defaults - } - if qualified.Template != nil { - mapping.Template = qualified.Template - } - if qualified.RefreshInterval != "" { - mapping.RefreshInterval = qualified.RefreshInterval - } - return mapping + return s.Services[service] } // RemoteRef locates one secret key of a service in the remote store. An explicit From 78ef8e38ba4342094448b7aa97f2d3200002e6ef Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 21:49:59 +0000 Subject: [PATCH 13/14] feat(gitops): derive cells from published module trees Reuse the existing cell derivation and completeness checks when assembling signed module publications for a direct deployment branch. Do not rebuild images or alter release signatures just to update the delivery source branch. --- cmd/deploy/gitops.go | 30 ++++++++++++++++++++++++++++-- docs/cli-reference.md | 15 +++++++++++++++ pkg/gitops/cell.go | 25 ++++++++++++++++++++++++- pkg/gitops/cell_test.go | 24 ++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index 1303e1849..b62ae80b5 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -33,6 +33,32 @@ var GitOpsCmd = &cobra.Command{ Short: "Render, publish, observe, and recover reviewed GitOps promotions", } +var gitOpsCellCmd = &cobra.Command{ + Use: "cell", + Short: "Derive the cell file from published module trees without rebuilding images", + Args: cobra.NoArgs, + RunE: func(_ *cobra.Command, _ []string) error { + ctx, done := common.NewContext() + defer done() + if err := common.ResolvePinnedModules(ctx); err != nil { + return err + } + workspace, err := common.LoadWorkspace(ctx) + if err != nil { + return err + } + env, err := orchestration.SelectEnvironment(workspace, gitOpsEnv) + if err != nil { + return err + } + cell, err := gitops.RenderPublishedCell(ctx, workspace, env) + if err == nil { + printCell(&cell) + } + return err + }, +} + var gitOpsRenderCmd = &cobra.Command{ Use: "render [module]", Short: "Render and validate a module-owned manifest tree", @@ -605,8 +631,8 @@ var ( ) func init() { - GitOpsCmd.AddCommand(gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) - for _, command := range []*cobra.Command{gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { + GitOpsCmd.AddCommand(gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) + for _, command := range []*cobra.Command{gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { command.Flags().StringVar(&gitOpsEnv, "env", "local", "Environment to promote") } // The delivery verbs — the ones that render a manifest tree from the diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 8a48b2cb5..7cdaff724 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2047,6 +2047,7 @@ codefly deploy gitops Subcommands: +- [`codefly deploy gitops cell`](#codefly-deploy-gitops-cell) - [`codefly deploy gitops observe`](#codefly-deploy-gitops-observe) - [`codefly deploy gitops plan`](#codefly-deploy-gitops-plan) - [`codefly deploy gitops publish`](#codefly-deploy-gitops-publish) @@ -2055,6 +2056,20 @@ Subcommands: - [`codefly deploy gitops rollback`](#codefly-deploy-gitops-rollback) - [`codefly deploy gitops snapshot`](#codefly-deploy-gitops-snapshot) +## `codefly deploy gitops cell` + +Derive the cell file from published module trees without rebuilding images + +``` +codefly deploy gitops cell [flags] +``` + +Flags: + +``` + --env string Environment to promote (default "local") +``` + ## `codefly deploy gitops observe` Verify Argo CD reconciled the reviewed Git revision and store evidence diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 19570da9d..305d329f8 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -98,16 +98,39 @@ func RenderCell(ctx context.Context, workspace *resources.Workspace, env *enviro return result, nil } +// RenderPublishedCell inventories the committed publication trees without rebuilding +// images or rewriting their signatures. Used when binding an assembled deployment branch. +func RenderPublishedCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (CellResult, error) { + if workspace == nil || env == nil { + return CellResult{}, errors.New("a cell is rendered for a workspace and an environment") + } + root, err := cellRoot(workspace, env) + if err != nil { + return CellResult{}, err + } + file, result, err := deriveCellFrom(ctx, workspace, env, filepath.Join(root, "modules")) + if err != nil { + return CellResult{}, err + } + return result, writeCell(&file, result.Path) +} + // deriveCell is RenderCell without the write: the cell file the rendered // trees and the composition describe, and the path it belongs at. func deriveCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (cellfile.File, CellResult, error) { + return deriveCellFrom(ctx, workspace, env, "") +} + +func deriveCellFrom(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, modulesDir string) (cellfile.File, CellResult, error) { if workspace == nil || env == nil { return cellfile.File{}, CellResult{}, errors.New("a cell is rendered for a workspace and an environment") } if env.Host == nil { return cellfile.File{}, CellResult{}, fmt.Errorf("environment %s declares no host, so it has no cell to describe", env.Name) } - modulesDir := filepath.Dir(moduleRenderDestination(workspace, "module")) + if modulesDir == "" { + modulesDir = filepath.Dir(moduleRenderDestination(workspace, "module")) + } entries, err := os.ReadDir(modulesDir) if errors.Is(err, fs.ErrNotExist) { return cellfile.File{}, CellResult{}, fmt.Errorf("no module has been rendered under %s", modulesDir) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 359858978..d4cda9101 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -210,6 +210,30 @@ func renderCellTreeWith(t *testing.T, options *RenderOptions, service, environme // container port the mesh authorizes and the Service port an edge route uses, // who the composition says calls them, the ingress into them, the bindings // the environment declares, and the egress a service is granted. +func TestPublishedCellUsesPublishedTreesAndRefusesMissingModules(t *testing.T) { + workspace := writeCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + env.Gitops = &environments.EnvironmentGitops{RepoURL: "https://github.com/example/product", Branch: "deploy", Path: "deployments/published"} + publication := filepath.Join(workspace.Dir(), env.Gitops.Path) + require.NoError(t, os.MkdirAll(publication, 0o755)) + require.NoError(t, os.Rename(filepath.Join(workspace.Dir(), "deployments/modules"), filepath.Join(publication, "modules"))) + result, err := RenderPublishedCell(context.Background(), workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing", "shop"}, result.Modules) + before, err := os.ReadFile(result.Path) + require.NoError(t, err) + _, err = RenderCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "no module has been rendered") + require.NoError(t, os.RemoveAll(filepath.Join(publication, "modules/billing"))) + _, err = RenderPublishedCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "billing") + after, err := os.ReadFile(result.Path) + require.NoError(t, err) + require.Equal(t, before, after, "incomplete publication must leave the last complete cell unchanged") +} + func TestCellFileInventoriesEveryRenderedWorkload(t *testing.T) { workspace := writeCellWorkspace(t) env := selectedEnvironment(t, workspace, "staging") From 35d619d481ebde19bcefef9e59e90c002698da6c Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Mon, 26 Oct 2026 23:01:21 +0000 Subject: [PATCH 14/14] fix(build): include read and traversal permissions in image identity --- pkg/orchestration/image_cache.go | 10 ++++------ pkg/orchestration/image_cache_test.go | 6 ++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/orchestration/image_cache.go b/pkg/orchestration/image_cache.go index 61d17cfa8..dadf722d1 100644 --- a/pkg/orchestration/image_cache.go +++ b/pkg/orchestration/image_cache.go @@ -643,17 +643,15 @@ func hashContextEntry(hasher hash.Hash, relative, path string, entry fs.DirEntry writeImageCacheRecord(hasher, "symlink", relative, filepath.ToSlash(target)) return nil case info.IsDir(): - writeImageCacheRecord(hasher, "directory", relative) + writeImageCacheRecord(hasher, "directory", relative, strconv.FormatUint(uint64(info.Mode().Perm()), 8)) return nil case !info.Mode().IsRegular(): writeImageCacheRecord(hasher, "special", relative, info.Mode().String()) return nil } - executable := "0" - if info.Mode().Perm()&0o111 != 0 { - executable = "1" - } - writeImageCacheRecord(hasher, "file", relative, executable, strconv.FormatInt(info.Size(), 10)) + // COPY preserves read and traversal permissions too: identical bytes can + // still be unreadable to the image's non-root user. + writeImageCacheRecord(hasher, "file", relative, strconv.FormatUint(uint64(info.Mode().Perm()), 8), strconv.FormatInt(info.Size(), 10)) file, err := os.Open(path) if err != nil { return err diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index c9fc3aca6..b9d6c9572 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -316,6 +316,12 @@ func TestEveryContextChangeMovesTheImageIdentity(t *testing.T) { "mode changed": func(t *testing.T, harness *imageCacheHarness) { require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code", "main.go"), 0o755)) }, + "file readability changed": func(t *testing.T, harness *imageCacheHarness) { + require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code", "main.go"), 0o600)) + }, + "directory traversal changed": func(t *testing.T, harness *imageCacheHarness) { + require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code"), 0o700)) + }, "file replaced by a symlink": func(t *testing.T, harness *imageCacheHarness) { path := filepath.Join(harness.serviceDir, "code", "main.go") require.NoError(t, os.Remove(path))