diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index 9bfce13b..c03540ab 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -969,6 +969,9 @@ func (s *BuilderWrapper) DeployKustomize(ctx context.Context, req *builderv0.Dep } // A value too large for the environment reaches the workload as a file // in a mounted volume; the environment carries only its path. + // Normalize within each category after restricted-profile checks, matching + // the last-key-wins ConfigMap/Secret maps without erasing secret inputs. + configurations = resources.EffectiveEnvironmentVariables(configurations) fileCarriers := &FileCarrierDelivery{ConfigMap: map[string]string{}, Secret: map[string]string{}} if configurations, err = splitFileCarriers(configurations, resources.KubernetesFileCarrierMount, fileCarriers.ConfigMap); err != nil { return fail(err) @@ -980,7 +983,7 @@ func (s *BuilderWrapper) DeployKustomize(ctx context.Context, req *builderv0.Dep var secretMap EnvironmentMap var inlineSecrets []*resources.EnvironmentVariable if profile.InlineSecrets() { - secrets := append(secretValues, deploymentContext.Secrets...) + secrets := resources.EffectiveEnvironmentVariables(secretValues, deploymentContext.Secrets) if inlineSecrets, err = splitFileCarriers(secrets, resources.KubernetesSecretFileCarrierMount, fileCarriers.Secret); err != nil { return fail(err) } diff --git a/agents/services/file_carriers_test.go b/agents/services/file_carriers_test.go index 73fdd54e..07c59c30 100644 --- a/agents/services/file_carriers_test.go +++ b/agents/services/file_carriers_test.go @@ -141,6 +141,53 @@ func TestARenderWithoutLargeValuesMountsNothing(t *testing.T) { require.NotContains(t, string(deployment), "codefly-configuration-files") } +func TestRenderedEnvironmentCountsEffectiveBindings(t *testing.T) { + info := &basev0.ConfigurationInformation{Name: "runnable-bindings"} + for i := 0; i < 134; i++ { + info.ConfigurationValues = append(info.ConfigurationValues, &basev0.ConfigurationValue{Key: fmt.Sprintf("OP_%03d", i), Value: strings.Repeat("x", 2800)}) + } + configuration := &basev0.Configuration{Origin: resources.ConfigurationWorkspace, Infos: []*basev0.ConfigurationInformation{info}} + destination, response, err := renderWithConfiguration(t, builderv0.KubernetesOutputProfile_KUBERNETES_OUTPUT_PROFILE_RESTRICTED_PORTABLE_V1, configuration, configuration) + require.NoError(t, err) + require.Equal(t, builderv0.DeploymentStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + environment := readDocuments(t, filepath.Join(destination, "base", "config-map.yaml"))[0]["data"].(map[string]any) + for _, value := range info.ConfigurationValues { + require.Equal(t, value.Value, environment[resources.ConfigurationValueEnvironmentKey(configuration, info.Name, value)]) + } + _, err = os.Stat(filepath.Join(destination, "overlays", "test", "configuration-files.yaml")) + require.True(t, os.IsNotExist(err), "no new file delivery") +} + +func TestRenderedEnvironmentNormalizesBeforeFileSplit(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__SETTINGS__MODE" + large := &resources.EnvironmentVariable{Key: key, Value: strings.Repeat("x", resources.FileCarrierThreshold+1), File: true} + small := resources.Env(key, "last") + for _, fileWins := range []bool{false, true} { + t.Run(fmt.Sprint(fileWins), func(t *testing.T) { + destination, response, err := renderPrepared(t, builderv0.KubernetesOutputProfile_KUBERNETES_OUTPUT_PROFILE_RESTRICTED_PORTABLE_V1, func(_ context.Context, d *KustomizeDeploymentContext) error { + if fileWins { + d.AddConfigMap(small, large, large) + } else { + d.AddConfigMap(large, small) + } + return nil + }) + require.NoError(t, err) + require.Equal(t, builderv0.DeploymentStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + environment := readDocuments(t, filepath.Join(destination, "base", "config-map.yaml"))[0]["data"].(map[string]any) + if fileWins { + require.NotContains(t, environment, key) + require.Contains(t, environment, resources.FileCarrierKey(key)) + files := readDocuments(t, filepath.Join(destination, "overlays", "test", "configuration-files.yaml")) + require.Equal(t, large.Value, files[0]["data"].(map[string]any)[key]) + } else { + require.Equal(t, "last", environment[key]) + require.NotContains(t, environment, resources.FileCarrierKey(key)) + } + }) + } +} + // A local apply carries secret values; a large secret is a Secret volume, // readable by the pod's group and nobody else, never a ConfigMap entry. func TestALargeSecretIsDeliveredByASecretVolume(t *testing.T) { diff --git a/docs/runnable-binding-delivery.md b/docs/runnable-binding-delivery.md index 4b7edccf..4b0ac198 100644 --- a/docs/runnable-binding-delivery.md +++ b/docs/runnable-binding-delivery.md @@ -198,6 +198,18 @@ the value whichever carrier delivered it. Everything fails closed, and as early as the information exists: +Native and Nix preparation first keeps the last occurrence of each exact +environment key, matching `os/exec` precedence: process entries override +environment defaults. The complete winning `EnvironmentVariable`, including +`File` and `Secret`, survives before materialization and size accounting. +Repeated delivery of the same bindings therefore does not double their size. +Kubernetes rendering applies the same normalization within public and secret +categories, after restricted-profile admission and before file splitting, +matching its existing last-key-wins maps. Categories are not merged. Inline +and `CODEFLY__FILE__` keys remain distinct; normalization neither resolves nor +conceals conflicting delivery forms. It changes no catalog, threshold or size +limit, and a genuinely oversized effective environment still refuses. + - **Plan and render time.** `resources.CheckProcessEnvironment` refuses a process environment in which any one `KEY=VALUE` string reaches 128 KiB, or in which the Codefly-emitted carriers together exceed diff --git a/resources/effective_environment_test.go b/resources/effective_environment_test.go new file mode 100644 index 00000000..d6113306 --- /dev/null +++ b/resources/effective_environment_test.go @@ -0,0 +1,28 @@ +package resources_test + +import ( + "testing" + + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +func TestEffectiveEnvironmentPreservesLastCompleteVariable(t *testing.T) { + old := &resources.EnvironmentVariable{Key: "CODEFLY__VALUE", Value: "old", File: true, Secret: true} + other := resources.Env("OTHER", "unchanged") + winner := resources.Env(old.Key, "new") + alias := resources.Env(resources.FileCarrierKey(old.Key), "/separate") + base := []*resources.EnvironmentVariable{old, nil, other} + process := []*resources.EnvironmentVariable{winner, alias} + got := resources.EffectiveEnvironmentVariables(base, process) + require.Equal(t, []*resources.EnvironmentVariable{other, winner, alias}, got) + require.Same(t, winner, got[1]) + require.False(t, got[1].File) + require.False(t, got[1].Secret) + require.Equal(t, []*resources.EnvironmentVariable{old, nil, other}, base) + require.Equal(t, []*resources.EnvironmentVariable{winner, alias}, process) + got = resources.EffectiveEnvironmentVariables(process, base) + require.Same(t, old, got[1]) + require.True(t, got[1].File) + require.True(t, got[1].Secret) +} diff --git a/resources/environment_variables_manager.go b/resources/environment_variables_manager.go index c2d92cf7..2d9c0657 100644 --- a/resources/environment_variables_manager.go +++ b/resources/environment_variables_manager.go @@ -44,6 +44,31 @@ func EnvironmentVariableAsStrings(envs []*EnvironmentVariable) []string { return result } +// EffectiveEnvironmentVariables keeps the last occurrence of each exact key, +// matching process environment precedence. It preserves the complete winning +// variable, including File and Secret, and the order of surviving occurrences. +// Call before file expansion: inline keys and file-carrier keys are distinct +// and must not be collapsed into one another. Inputs are not modified. +func EffectiveEnvironmentVariables(sets ...[]*EnvironmentVariable) []*EnvironmentVariable { + var all []*EnvironmentVariable + last := map[string]int{} + for _, set := range sets { + for _, env := range set { + if env != nil { + last[env.Key] = len(all) + all = append(all, env) + } + } + } + out := make([]*EnvironmentVariable, 0, len(last)) + for index, env := range all { + if last[env.Key] == index { + out = append(out, env) + } + } + return out +} + type EnvironmentVariableManager struct { // Environment environment *basev0.Environment diff --git a/runners/base/carriers.go b/runners/base/carriers.go index 7b8f9734..30ffbd5f 100644 --- a/runners/base/carriers.go +++ b/runners/base/carriers.go @@ -22,16 +22,12 @@ type processCarriers struct { // value it cannot receive. The directory is created only when a value needs // it; release removes it once the process has exited. func prepareProcessCarriers(envs ...[]*resources.EnvironmentVariable) (*processCarriers, error) { - var all []*resources.EnvironmentVariable + // Environment defaults precede process overrides, just as in os/exec. + // Count and materialize only winners, not superseded duplicate entries. + all := resources.EffectiveEnvironmentVariables(envs...) file := false - for _, set := range envs { - for _, env := range set { - if env == nil { - continue - } - file = file || env.File - all = append(all, env) - } + for _, env := range all { + file = file || env.File } carriers := &processCarriers{} if file { diff --git a/runners/base/carriers_internal_test.go b/runners/base/carriers_internal_test.go new file mode 100644 index 00000000..82b238db --- /dev/null +++ b/runners/base/carriers_internal_test.go @@ -0,0 +1,79 @@ +package base + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +func TestProcessCarriersCount134EffectiveBindings(t *testing.T) { + var bindings []*resources.EnvironmentVariable + for i := 0; i < 134; i++ { + bindings = append(bindings, resources.Env(fmt.Sprintf("CODEFLY__WORKSPACE_CONFIGURATION__RUNNABLE_BINDINGS__OP_%03d", i), strings.Repeat("x", 2800))) + } + require.NoError(t, resources.CheckProcessEnvironment(bindings)) + require.ErrorIs(t, resources.CheckProcessEnvironment(append(append([]*resources.EnvironmentVariable{}, bindings...), bindings...)), resources.ErrEnvironmentLimit) + carriers, err := prepareProcessCarriers(bindings, bindings) + require.NoError(t, err) + t.Cleanup(carriers.release) + require.Empty(t, carriers.dir, "no spill or file transport change") + require.Equal(t, resources.EnvironmentVariableAsStrings(bindings), carriers.environ) + var distinct []*resources.EnvironmentVariable + for _, env := range bindings { + distinct = append(distinct, resources.Env(env.Key+"_OTHER", env.Value)) + } + _, err = prepareProcessCarriers(bindings, distinct) + require.ErrorIs(t, err, resources.ErrEnvironmentLimit, "genuinely oversized unique environment still refuses") +} + +func TestProcessCarrierOverridesPrecedeMaterialization(t *testing.T) { + key := "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__VAULT__VALUE" + file := &resources.EnvironmentVariable{Key: key, Value: strings.Repeat("s", resources.FileCarrierThreshold+1), File: true, Secret: true} + inline := resources.Env(key, "changed") + for _, test := range []struct { + name string + base, winner *resources.EnvironmentVariable + }{ + {"file to inline", file, inline}, + {"inline to file", inline, file}, + {"changed inline", resources.Env(key, "old"), inline}, + } { + t.Run(test.name, func(t *testing.T) { + carriers, err := prepareProcessCarriers([]*resources.EnvironmentVariable{test.base}, []*resources.EnvironmentVariable{test.winner}) + require.NoError(t, err) + t.Cleanup(carriers.release) + require.Len(t, carriers.environ, 1) + if !test.winner.File { + require.Empty(t, carriers.dir) + require.Equal(t, []string{key + "=changed"}, carriers.environ) + return + } + resolved, err := resources.ResolveFileCarriers(carriers.environ) + require.NoError(t, err) + require.Equal(t, test.winner.ValueAsString(), resolved[key]) + entries, err := os.ReadDir(carriers.dir) + require.NoError(t, err) + require.Len(t, entries, 1) + info, err := entries[0].Info() + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + }) + } +} + +func TestProcessCarrierDistinctAliasStillRefusesAtRead(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__GROUP__VALUE" + file := filepath.Join(t.TempDir(), "value") + require.NoError(t, os.WriteFile(file, []byte("from file"), 0o600)) + carriers, err := prepareProcessCarriers([]*resources.EnvironmentVariable{resources.Env(key, "inline")}, []*resources.EnvironmentVariable{resources.Env(resources.FileCarrierKey(key), file)}) + require.NoError(t, err) + t.Cleanup(carriers.release) + require.Len(t, carriers.environ, 2, "exact keys remain distinct") + _, err = resources.ResolveFileCarriers(carriers.environ) + require.ErrorIs(t, err, resources.ErrFileCarrier) +}