From 7b3c466ddb529af81d399b47ddb57d21f398bba0 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Fri, 31 Jul 2026 14:08:36 +0100 Subject: [PATCH 1/4] fixed stale fnconfig bug Signed-off-by: Oisin Johnston --- pkg/lib/kptops/helpers_test.go | 15 +++ pkg/lib/kptops/render_executor.go | 24 ++++- pkg/lib/kptops/render_executor_test.go | 131 +++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) diff --git a/pkg/lib/kptops/helpers_test.go b/pkg/lib/kptops/helpers_test.go index 26a58c92e1..6b4bfdce5d 100644 --- a/pkg/lib/kptops/helpers_test.go +++ b/pkg/lib/kptops/helpers_test.go @@ -37,6 +37,7 @@ import ( var testFunctions = map[string]framework.ResourceListProcessorFunc{ "ghcr.io/kptdev/krm-functions-catalog/set-labels:latest": setLabels, "ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest": setNamespace, + "ghcr.io/test/check-fnconfig-labels:latest": checkFnConfigLabels, } // runtime is a test-only FunctionRuntime that resolves functions from testFunctions. @@ -120,3 +121,17 @@ func validGVK(rn *kyaml.RNode, apiVersion, kind string) bool { } return meta.APIVersion == apiVersion && meta.Kind == kind } + +// checkFnConfigLabels is a test-only validator that verifies its functionConfig +// has a specific label. This is used to test that validators receive updated +// functionConfig from in-memory input after mutators have modified it. +func checkFnConfigLabels(rl *framework.ResourceList) error { + if rl.FunctionConfig == nil { + return errors.New("functionConfig is nil") + } + labels := rl.FunctionConfig.GetLabels() + if _, ok := labels["env"]; !ok { + return fmt.Errorf("functionConfig is stale - missing 'env' label that mutator should have added; labels: %v", labels) + } + return nil +} diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index 05d4e81556..3387ca5ed1 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -858,7 +858,7 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in if err != nil { return err } - var validator kio.Filter + var validator *fnruntime.FunctionRunner displayResourceCount := false if len(function.Selectors) > 0 || len(function.Exclusions) > 0 { displayResourceCount = true @@ -875,6 +875,28 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) return err } + + if function.ConfigPath != "" { + // functionConfigs are included in the function inputs during `render` + // and as a result, they can be mutated during the `render`. + // So functionConfigs needs be updated in the FunctionRunner instance + // before every run, same as for mutators. + for _, r := range input { + pkgPath, err := pkg.GetPkgPathAnnotation(r) + if err != nil { + return err + } + currPath, _, err := kioutil.GetFileAnnotations(r) + if err != nil { + return err + } + if pkgPath == pn.pkg.UniquePath.String() && currPath == function.ConfigPath { + validator.SetFnConfig(r) + break + } + } + } + if _, err = validator.Filter(cloneResources(selectedResources)); err != nil { hctx.validationSteps = append(hctx.validationSteps, captureStepResult(function, hctx.fnResults, resultCountBeforeExec, err)) return err diff --git a/pkg/lib/kptops/render_executor_test.go b/pkg/lib/kptops/render_executor_test.go index 56e310d863..88fc75f4c9 100644 --- a/pkg/lib/kptops/render_executor_test.go +++ b/pkg/lib/kptops/render_executor_test.go @@ -27,7 +27,9 @@ import ( fnruntime "github.com/kptdev/kpt/pkg/fn/runtime" "github.com/kptdev/kpt/pkg/kptfile/kptfileutil" "github.com/kptdev/kpt/pkg/lib/pkg" + "github.com/kptdev/kpt/pkg/lib/runneroptions" "github.com/kptdev/kpt/pkg/printer" + "github.com/kptdev/kpt/pkg/printer/fake" "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/kyaml/filesys" "sigs.k8s.io/kustomize/kyaml/fn/framework" @@ -1050,3 +1052,132 @@ metadata: }) } } + +func TestRunValidators_RefreshFnConfigFromInMemoryInput(t *testing.T) { + // This test verifies that when a mutator modifies a resource that is used as + // a validator's functionConfig (via configPath), the validator sees the + // updated in-memory version rather than the stale on-disk version. + // + // Setup: + // - A mutator (set-labels) adds "env: production" label to all resources + // - A validator (check-fnconfig-labels) uses configPath to reference a ConfigMap + // - The validator checks that its functionConfig has the "env" label + // + // Without the fix, the validator reads from disk (no labels) and fails. + // With the fix, the validator sees the mutated config (has env label) and passes. + + mockFS := filesys.MakeFsInMemory() + pkgPath := rootString + assert.NoError(t, mockFS.Mkdir(pkgPath)) + + // Kptfile with mutator that adds labels, and a validator with configPath + kptfileContent := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest + configMap: + env: production + validators: + - image: ghcr.io/test/check-fnconfig-labels:latest + configPath: validator-config.yaml +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "Kptfile"), []byte(kptfileContent))) + + // The validator config resource (initially has no labels - they will be added by the mutator) + validatorConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: validator-config + annotations: + config.kubernetes.io/local-config: "true" +data: + check: labels +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "validator-config.yaml"), []byte(validatorConfig))) + + // A regular resource in the package + deployment := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-app +spec: + replicas: 1 +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "deployment.yaml"), []byte(deployment))) + + r := &Renderer{ + PkgPath: pkgPath, + FileSystem: mockFS, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + ctx := fake.CtxWithDefaultPrinter() + _, err := r.Execute(ctx) + assert.NoError(t, err, "validator should pass because it receives the mutated functionConfig with 'env' label") + + // Verify the validator config was indeed mutated (written with labels) + res, err := mockFS.ReadFile(filepath.Join(pkgPath, "validator-config.yaml")) + assert.NoError(t, err) + assert.Contains(t, string(res), "env: production", "validator-config.yaml should have the label added by the mutator") +} + +func TestRunValidators_RefreshFnConfigFromInMemoryInput_BFS(t *testing.T) { + // Same test as above but with BFS rendering mode enabled + mockFS := filesys.MakeFsInMemory() + pkgPath := rootString + assert.NoError(t, mockFS.Mkdir(pkgPath)) + + kptfileContent := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + kpt.dev/bfs-rendering: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest + configMap: + env: production + validators: + - image: ghcr.io/test/check-fnconfig-labels:latest + configPath: validator-config.yaml +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "Kptfile"), []byte(kptfileContent))) + + validatorConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: validator-config + annotations: + config.kubernetes.io/local-config: "true" +data: + check: labels +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "validator-config.yaml"), []byte(validatorConfig))) + + deployment := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-app +spec: + replicas: 1 +` + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "deployment.yaml"), []byte(deployment))) + + r := &Renderer{ + PkgPath: pkgPath, + FileSystem: mockFS, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + ctx := fake.CtxWithDefaultPrinter() + _, err := r.Execute(ctx) + assert.NoError(t, err, "validator should pass in BFS mode because it receives the mutated functionConfig with 'env' label") +} From aa238417d62d2764d1d96e84399da5086e643a70 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Fri, 31 Jul 2026 14:18:38 +0100 Subject: [PATCH 2/4] fixed duplication Signed-off-by: Oisin Johnston --- pkg/lib/kptops/render_executor_test.go | 125 +++++++------------------ 1 file changed, 36 insertions(+), 89 deletions(-) diff --git a/pkg/lib/kptops/render_executor_test.go b/pkg/lib/kptops/render_executor_test.go index 88fc75f4c9..a9def45c19 100644 --- a/pkg/lib/kptops/render_executor_test.go +++ b/pkg/lib/kptops/render_executor_test.go @@ -1057,87 +1057,27 @@ func TestRunValidators_RefreshFnConfigFromInMemoryInput(t *testing.T) { // This test verifies that when a mutator modifies a resource that is used as // a validator's functionConfig (via configPath), the validator sees the // updated in-memory version rather than the stale on-disk version. - // - // Setup: - // - A mutator (set-labels) adds "env: production" label to all resources - // - A validator (check-fnconfig-labels) uses configPath to reference a ConfigMap - // - The validator checks that its functionConfig has the "env" label - // - // Without the fix, the validator reads from disk (no labels) and fails. - // With the fix, the validator sees the mutated config (has env label) and passes. - mockFS := filesys.MakeFsInMemory() - pkgPath := rootString - assert.NoError(t, mockFS.Mkdir(pkgPath)) - - // Kptfile with mutator that adds labels, and a validator with configPath - kptfileContent := `apiVersion: kpt.dev/v1 -kind: Kptfile -metadata: - name: test-pkg -pipeline: - mutators: - - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest - configMap: - env: production - validators: - - image: ghcr.io/test/check-fnconfig-labels:latest - configPath: validator-config.yaml -` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "Kptfile"), []byte(kptfileContent))) - - // The validator config resource (initially has no labels - they will be added by the mutator) - validatorConfig := `apiVersion: v1 -kind: ConfigMap -metadata: - name: validator-config - annotations: - config.kubernetes.io/local-config: "true" -data: - check: labels -` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "validator-config.yaml"), []byte(validatorConfig))) - - // A regular resource in the package - deployment := `apiVersion: apps/v1 -kind: Deployment -metadata: - name: my-app -spec: - replicas: 1 -` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "deployment.yaml"), []byte(deployment))) - - r := &Renderer{ - PkgPath: pkgPath, - FileSystem: mockFS, - Runtime: &runtime{}, + tests := []struct { + name string + renderBfs bool + }{ + {name: "DFS rendering", renderBfs: false}, + {name: "BFS rendering", renderBfs: true}, } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull - - ctx := fake.CtxWithDefaultPrinter() - _, err := r.Execute(ctx) - assert.NoError(t, err, "validator should pass because it receives the mutated functionConfig with 'env' label") - - // Verify the validator config was indeed mutated (written with labels) - res, err := mockFS.ReadFile(filepath.Join(pkgPath, "validator-config.yaml")) - assert.NoError(t, err) - assert.Contains(t, string(res), "env: production", "validator-config.yaml should have the label added by the mutator") -} -func TestRunValidators_RefreshFnConfigFromInMemoryInput_BFS(t *testing.T) { - // Same test as above but with BFS rendering mode enabled - mockFS := filesys.MakeFsInMemory() - pkgPath := rootString - assert.NoError(t, mockFS.Mkdir(pkgPath)) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mockFS := filesys.MakeFsInMemory() + pkgPath := rootString + assert.NoError(t, mockFS.Mkdir(pkgPath)) - kptfileContent := `apiVersion: kpt.dev/v1 + kptfileContent := fmt.Appendf(nil, `apiVersion: kpt.dev/v1 kind: Kptfile metadata: name: test-pkg annotations: - kpt.dev/bfs-rendering: "true" + kpt.dev/bfs-rendering: %q pipeline: mutators: - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest @@ -1146,10 +1086,10 @@ pipeline: validators: - image: ghcr.io/test/check-fnconfig-labels:latest configPath: validator-config.yaml -` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "Kptfile"), []byte(kptfileContent))) +`, fmt.Sprintf("%t", tc.renderBfs)) + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "Kptfile"), kptfileContent)) - validatorConfig := `apiVersion: v1 + validatorConfig := `apiVersion: v1 kind: ConfigMap metadata: name: validator-config @@ -1158,26 +1098,33 @@ metadata: data: check: labels ` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "validator-config.yaml"), []byte(validatorConfig))) + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "validator-config.yaml"), []byte(validatorConfig))) - deployment := `apiVersion: apps/v1 + deployment := `apiVersion: apps/v1 kind: Deployment metadata: name: my-app spec: replicas: 1 ` - assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "deployment.yaml"), []byte(deployment))) + assert.NoError(t, mockFS.WriteFile(filepath.Join(pkgPath, "deployment.yaml"), []byte(deployment))) - r := &Renderer{ - PkgPath: pkgPath, - FileSystem: mockFS, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + r := &Renderer{ + PkgPath: pkgPath, + FileSystem: mockFS, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + ctx := fake.CtxWithDefaultPrinter() + _, err := r.Execute(ctx) + assert.NoError(t, err, "validator should pass because it receives the mutated functionConfig with 'env' label") - ctx := fake.CtxWithDefaultPrinter() - _, err := r.Execute(ctx) - assert.NoError(t, err, "validator should pass in BFS mode because it receives the mutated functionConfig with 'env' label") + // Verify the validator config was indeed mutated (written with labels) + res, err := mockFS.ReadFile(filepath.Join(pkgPath, "validator-config.yaml")) + assert.NoError(t, err) + assert.Contains(t, string(res), "env: production", "validator-config.yaml should have the label added by the mutator") + }) + } } From ed80da8fd9c9f344b1a06d8738424f6893f0c05d Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Fri, 31 Jul 2026 14:34:20 +0100 Subject: [PATCH 3/4] fixed complexity issues Signed-off-by: Oisin Johnston --- pkg/lib/kptops/render_executor.go | 44 ++++++++++++++++++------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index 3387ca5ed1..cdeffd1591 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -876,25 +876,8 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in return err } - if function.ConfigPath != "" { - // functionConfigs are included in the function inputs during `render` - // and as a result, they can be mutated during the `render`. - // So functionConfigs needs be updated in the FunctionRunner instance - // before every run, same as for mutators. - for _, r := range input { - pkgPath, err := pkg.GetPkgPathAnnotation(r) - if err != nil { - return err - } - currPath, _, err := kioutil.GetFileAnnotations(r) - if err != nil { - return err - } - if pkgPath == pn.pkg.UniquePath.String() && currPath == function.ConfigPath { - validator.SetFnConfig(r) - break - } - } + if err := pn.refreshFnConfig(validator, input, function.ConfigPath); err != nil { + return err } if _, err = validator.Filter(cloneResources(selectedResources)); err != nil { @@ -914,6 +897,29 @@ func cloneResources(input []*yaml.RNode) (output []*yaml.RNode) { return } +// refreshFnConfig updates the runner's functionConfig from the in-memory input +// to pick up any mutations applied earlier in the pipeline. +func (pn *pkgNode) refreshFnConfig(runner *fnruntime.FunctionRunner, input []*yaml.RNode, configPath string) error { + if configPath == "" { + return nil + } + for _, r := range input { + pkgPath, err := pkg.GetPkgPathAnnotation(r) + if err != nil { + return err + } + currPath, _, err := kioutil.GetFileAnnotations(r) + if err != nil { + return err + } + if pkgPath == pn.pkg.UniquePath.String() && currPath == configPath { + runner.SetFnConfig(r) + break + } + } + return nil +} + // clearAnnotationsOnMutFailure removes annotations that are added during mutation when mutation fails. func clearAnnotationsOnMutFailure(input []*yaml.RNode) { annotations := []string{ From ab28c3d864b91e7e1979f86c7698976619e411ed Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Fri, 31 Jul 2026 14:42:30 +0100 Subject: [PATCH 4/4] fixed complexity fr this time Signed-off-by: Oisin Johnston --- pkg/lib/kptops/render_executor.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index cdeffd1591..953789cd29 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -859,10 +859,7 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in return err } var validator *fnruntime.FunctionRunner - displayResourceCount := false - if len(function.Selectors) > 0 || len(function.Exclusions) > 0 { - displayResourceCount = true - } + displayResourceCount := len(function.Selectors) > 0 || len(function.Exclusions) > 0 if function.Exec != "" && !hctx.runnerOptions.AllowExec { hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, errAllowedExecNotSpecified)) return errAllowedExecNotSpecified