From 934329ef2a7d85695ab0064c0f36c752849bbdaf Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Fri, 24 Jul 2026 15:24:23 +0100 Subject: [PATCH 1/7] allow config references by resource name Signed-off-by: Oisin Johnston --- api/kptfile/v1/types.go | 19 +++ api/kptfile/v1/validation.go | 37 ++++- api/kptfile/v1/validation_test.go | 167 ++++++++++++++++++++++ api/kptfile/v1/zz_generated.deepcopy.go | 20 +++ api/schema/v1/group_version.go | 10 +- go.mod | 2 +- go.sum | 2 - pkg/fn/runtime/runner.go | 61 +++++++- pkg/fn/runtime/runner_test.go | 178 +++++++++++++++++++++++- pkg/lib/kptops/render_executor.go | 13 +- pkg/lib/pkg/pkg.go | 44 ++++++ 11 files changed, 539 insertions(+), 14 deletions(-) diff --git a/api/kptfile/v1/types.go b/api/kptfile/v1/types.go index f42b6c52f3..264a253bcb 100644 --- a/api/kptfile/v1/types.go +++ b/api/kptfile/v1/types.go @@ -346,6 +346,11 @@ type Function struct { // `ConfigMap` is a convenient way to specify a function config of kind ConfigMap. ConfigMap map[string]string `yaml:"configMap,omitempty" json:"configMap,omitempty"` + // `ConfigRef` references an existing resource in the package as the function config. + // The resource is identified by apiVersion, kind, and name. This is mutually exclusive + // with `configPath` and `configMap`. + ConfigRef *ResourceReference `yaml:"configRef,omitempty" json:"configRef,omitempty"` + // `Name` is used to uniquely identify the function declaration // this is primarily used for merging function declaration with upstream counterparts Name string `yaml:"name,omitempty" json:"name,omitempty"` @@ -363,6 +368,20 @@ type Function struct { Exclusions []Selector `yaml:"exclude,omitempty" json:"exclude,omitempty"` } +// ResourceReference identifies a resource within the package by its API identity. +type ResourceReference struct { + // APIVersion of the referenced resource (e.g. "v1"). Optional; when omitted, + // matches resources regardless of apiVersion. + APIVersion string `yaml:"apiVersion,omitempty" json:"apiVersion,omitempty"` + // Kind of the referenced resource (e.g. "ConfigMap"). Required. + Kind string `yaml:"kind" json:"kind"` + // Name of the referenced resource. Required. + Name string `yaml:"name" json:"name"` + // Namespace of the referenced resource. Optional; when omitted, matches + // resources regardless of namespace. + Namespace string `yaml:"namespace,omitempty" json:"namespace,omitempty"` +} + // Selector specifies the selection criteria // please update IsEmpty method if more properties are added type Selector struct { diff --git a/api/kptfile/v1/validation.go b/api/kptfile/v1/validation.go index 6be7006a9f..9369e1f668 100644 --- a/api/kptfile/v1/validation.go +++ b/api/kptfile/v1/validation.go @@ -90,10 +90,26 @@ func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkg } // TODO(droot): validate the exec - if len(f.ConfigMap) != 0 && f.ConfigPath != "" { + configSources := 0 + if len(f.ConfigMap) != 0 { + configSources++ + } + if f.ConfigPath != "" { + configSources++ + } + if f.ConfigRef != nil { + configSources++ + } + if configSources > 1 { return &ValidateError{ Field: fmt.Sprintf("pipeline.%s[%d]", fnType, idx), - Reason: "functionConfig must not specify both `configMap` and `configPath` at the same time", + Reason: "functionConfig must specify at most one of `configMap`, `configPath`, or `configRef`", + } + } + + if f.ConfigRef != nil { + if err := f.ConfigRef.validate(fnType, idx); err != nil { + return err } } @@ -154,6 +170,23 @@ func validateFnConfigPathSyntax(p string) error { return nil } +// validate checks that the ResourceReference has the required fields set. +func (r *ResourceReference) validate(fnType string, idx int) error { + if r.Kind == "" { + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configRef.kind", fnType, idx), + Reason: "configRef must specify `kind`", + } + } + if r.Name == "" { + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configRef.name", fnType, idx), + Reason: "configRef must specify `name`", + } + } + return nil +} + // GetValidatedFnConfigFromPath validates the functionConfig at the path specified by // the package path (pkgPath) and configPath, returning the functionConfig as an // RNode if the validation is successful. diff --git a/api/kptfile/v1/validation_test.go b/api/kptfile/v1/validation_test.go index 3d5458188d..121f50ee67 100644 --- a/api/kptfile/v1/validation_test.go +++ b/api/kptfile/v1/validation_test.go @@ -526,3 +526,170 @@ metadata: }) } } + +func TestConfigRefValidation(t *testing.T) { + type input struct { + name string + kptfile KptFile + valid bool + } + + cases := []input{ + { + name: "configRef: valid with kind and name", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + }, + }, + }, + }, + }, + valid: true, + }, + { + name: "configRef: valid with all fields", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + APIVersion: "v1", + Kind: "ConfigMap", + Name: "my-config", + Namespace: "default", + }, + }, + }, + }, + }, + valid: true, + }, + { + name: "configRef: missing kind", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Name: "my-config", + }, + }, + }, + }, + }, + valid: false, + }, + { + name: "configRef: missing name", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + }, + }, + }, + }, + }, + valid: false, + }, + { + name: "configRef: mutual exclusivity with configMap", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + }, + ConfigMap: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + valid: false, + }, + { + name: "configRef: mutual exclusivity with configPath", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + }, + ConfigPath: "./config.yaml", + }, + }, + }, + }, + valid: false, + }, + { + name: "configRef: mutual exclusivity all three", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Mutators: []Function{ + { + Image: "set-namespace", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + }, + ConfigPath: "./config.yaml", + ConfigMap: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + valid: false, + }, + { + name: "configRef: valid in validators", + kptfile: KptFile{ + Pipeline: &Pipeline{ + Validators: []Function{ + { + Image: "gatekeeper", + ConfigRef: &ResourceReference{ + Kind: "ConfigMap", + Name: "policy-config", + }, + }, + }, + }, + }, + valid: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.kptfile.Validate(filesys.FileSystemOrOnDisk{}, "") + if c.valid && err != nil { + t.Fatalf("kptfile should be valid, %s", err) + } + if !c.valid && err == nil { + t.Fatal("kptfile should not be valid") + } + }) + } +} diff --git a/api/kptfile/v1/zz_generated.deepcopy.go b/api/kptfile/v1/zz_generated.deepcopy.go index 01631de11e..d7225bc304 100644 --- a/api/kptfile/v1/zz_generated.deepcopy.go +++ b/api/kptfile/v1/zz_generated.deepcopy.go @@ -47,6 +47,11 @@ func (in *Function) DeepCopyInto(out *Function) { (*out)[key] = val } } + if in.ConfigRef != nil { + in, out := &in.ConfigRef, &out.ConfigRef + *out = new(ResourceReference) + **out = **in + } if in.Selectors != nil { in, out := &in.Selectors, &out.Selectors *out = make([]Selector, len(*in)) @@ -350,6 +355,21 @@ func (in *RenderStatus) DeepCopy() *RenderStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResourceReference) DeepCopyInto(out *ResourceReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResourceReference. +func (in *ResourceReference) DeepCopy() *ResourceReference { + if in == nil { + return nil + } + out := new(ResourceReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Selector) DeepCopyInto(out *Selector) { *out = *in diff --git a/api/schema/v1/group_version.go b/api/schema/v1/group_version.go index 50e7144a4c..1a81066cd9 100644 --- a/api/schema/v1/group_version.go +++ b/api/schema/v1/group_version.go @@ -80,19 +80,19 @@ func (gr GroupResource) String() string { } func ParseGroupKind(gk string) GroupKind { - i := strings.Index(gk, ".") - if i == -1 { + before, after, ok := strings.Cut(gk, ".") + if !ok { return GroupKind{Kind: gk} } - return GroupKind{Group: gk[i+1:], Kind: gk[:i]} + return GroupKind{Group: after, Kind: before} } // ParseGroupResource turns "resource.group" string into a GroupResource struct. Empty strings are allowed // for each field. func ParseGroupResource(gr string) GroupResource { - if i := strings.Index(gr, "."); i >= 0 { - return GroupResource{Group: gr[i+1:], Resource: gr[:i]} + if before, after, ok := strings.Cut(gr, "."); ok { + return GroupResource{Group: after, Resource: before} } return GroupResource{Resource: gr} } diff --git a/go.mod b/go.mod index a8321b9fdf..0d60a15c84 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/kptdev/kpt go 1.26.3 -//replace github.com/kptdev/kpt/api => ./api +replace github.com/kptdev/kpt/api => ./api require ( github.com/Masterminds/semver/v3 v3.5.0 diff --git a/go.sum b/go.sum index 2f5b7ef0d7..72446dda49 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,6 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= -github.com/kptdev/kpt/api v0.0.2 h1:0AWWhw8/LapkPwDwImtJXUDx81U9nW+joYIzCl3S/68= -github.com/kptdev/kpt/api v0.0.2/go.mod h1:D/WM1LJ/HvHt3cHxDs2mF6mXkA37BbD1nrcsAVWgbfE= github.com/kptdev/krm-functions-sdk/go/fn v1.1.1 h1:F/tdu0FSWSnLaAV+AC8CVS0YGpUwkVnAs+O5NHPRmQU= github.com/kptdev/krm-functions-sdk/go/fn v1.1.1/go.mod h1:rPrLdh02mfqq5PjZRrMr5mhRA6dkjKm7O4bm5VZJWOg= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= diff --git a/pkg/fn/runtime/runner.go b/pkg/fn/runtime/runner.go index d1a035de9d..0629624239 100644 --- a/pkg/fn/runtime/runner.go +++ b/pkg/fn/runtime/runner.go @@ -51,8 +51,13 @@ func NewRunner( fnResults *fnresultv1.ResultList, opts runneroptions.RunnerOptions, runtime fn.FunctionRuntime, + resources ...[]*yaml.RNode, ) (*FunctionRunner, error) { - config, err := newFnConfig(fsys, f, pkgPath) + var inputResources []*yaml.RNode + if len(resources) > 0 { + inputResources = resources[0] + } + config, err := newFnConfig(fsys, f, pkgPath, inputResources) if err != nil { return nil, err } @@ -505,7 +510,7 @@ func enforcePathInvariants(nodes []*yaml.RNode) error { return nil } -func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfilev1.UniquePath) (*yaml.RNode, error) { +func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfilev1.UniquePath, resources []*yaml.RNode) (*yaml.RNode, error) { const op errors.Op = "fn.readConfig" fn := errors.Fn(f.Image) @@ -535,11 +540,63 @@ func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfile return nil, errors.E(op, fn, err) } return configNode, nil + case f.ConfigRef != nil: + if resources == nil { + // Resources not available yet (e.g. during mutator chain construction). + // Config will be resolved from the input list before execution. + return nil, nil + } + node, err := ResolveConfigRef(f.ConfigRef, pkgPath, resources) + if err != nil { + return nil, errors.E(op, fn, err) + } + return node, nil } // no need to return ConfigMap if no config given return nil, nil } +// ResolveConfigRef searches the resource list for a resource matching the given +// ResourceReference. It returns the matching resource or an error if no match +// or multiple matches are found. +func ResolveConfigRef(ref *kptfilev1.ResourceReference, pkgPath kptfilev1.UniquePath, resources []*yaml.RNode) (*yaml.RNode, error) { + var matches []*yaml.RNode + for _, r := range resources { + meta, err := r.GetMeta() + if err != nil { + continue + } + // If pkgPath is set, only match resources belonging to this package. + if pkgPath != "" { + resPkgPath, _ := pkg.GetPkgPathAnnotation(r) + if resPkgPath != "" && resPkgPath != string(pkgPath) { + continue + } + } + if ref.APIVersion != "" && meta.APIVersion != ref.APIVersion { + continue + } + if meta.Kind != ref.Kind { + continue + } + if meta.Name != ref.Name { + continue + } + if ref.Namespace != "" && meta.Namespace != ref.Namespace { + continue + } + matches = append(matches, r) + } + switch len(matches) { + case 0: + return nil, fmt.Errorf("configRef: resource %s/%s %q not found in package", ref.APIVersion, ref.Kind, ref.Name) + case 1: + return matches[0], nil + default: + return nil, fmt.Errorf("configRef: resource %s/%s %q matched %d resources, must match exactly one", ref.APIVersion, ref.Kind, ref.Name, len(matches)) + } +} + // hasTagOrDigest reports whether the image reference contains an explicit tag or digest. func hasTagOrDigest(image string) bool { if strings.Contains(image, "@") { diff --git a/pkg/fn/runtime/runner_test.go b/pkg/fn/runtime/runner_test.go index 4d28f76d69..b92c3e9326 100644 --- a/pkg/fn/runtime/runner_test.go +++ b/pkg/fn/runtime/runner_test.go @@ -91,7 +91,7 @@ data: {foo: bar} c.fn.ConfigPath = path.Base(tmp.Name()) } fsys := filesys.MakeFsOnDisk() - cn, err := newFnConfig(fsys, &c.fn, kptfilev1.UniquePath(os.TempDir())) + cn, err := newFnConfig(fsys, &c.fn, kptfilev1.UniquePath(os.TempDir()), nil) assert.NoError(t, err, "unexpected error") actual, err := cn.String() assert.NoError(t, err, "unexpected error") @@ -847,3 +847,179 @@ items: }) } } + +func TestResolveConfigRef(t *testing.T) { + // Helper to create a resource RNode from YAML. + makeResource := func(t *testing.T, y string) *yaml.RNode { + t.Helper() + node, err := yaml.Parse(y) + require.NoError(t, err) + return node + } + + resources := []*yaml.RNode{ + makeResource(t, ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-config +data: + namespace: production +`), + makeResource(t, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-app + namespace: default +spec: + replicas: 3 +`), + makeResource(t, ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: other-config + namespace: staging +data: + key: value +`), + } + + tests := []struct { + name string + ref *kptfilev1.ResourceReference + expectName string + expectErr string + }{ + { + name: "match by kind and name", + ref: &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + }, + expectName: "my-config", + }, + { + name: "match by apiVersion, kind, and name", + ref: &kptfilev1.ResourceReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "my-app", + }, + expectName: "my-app", + }, + { + name: "match with namespace filter", + ref: &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "other-config", + Namespace: "staging", + }, + expectName: "other-config", + }, + { + name: "no match - wrong name", + ref: &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "nonexistent", + }, + expectErr: `configRef: resource /ConfigMap "nonexistent" not found in package`, + }, + { + name: "no match - wrong kind", + ref: &kptfilev1.ResourceReference{ + Kind: "Secret", + Name: "my-config", + }, + expectErr: `configRef: resource /Secret "my-config" not found in package`, + }, + { + name: "no match - apiVersion mismatch", + ref: &kptfilev1.ResourceReference{ + APIVersion: "v2", + Kind: "ConfigMap", + Name: "my-config", + }, + expectErr: `configRef: resource v2/ConfigMap "my-config" not found in package`, + }, + { + name: "no match - namespace mismatch", + ref: &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "other-config", + Namespace: "production", + }, + expectErr: `configRef: resource /ConfigMap "other-config" not found in package`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result, err := ResolveConfigRef(tc.ref, "", resources) + if tc.expectErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectErr) + return + } + require.NoError(t, err) + meta, err := result.GetMeta() + require.NoError(t, err) + assert.Equal(t, tc.expectName, meta.Name) + }) + } +} + +func TestResolveConfigRef_MultipleMatches(t *testing.T) { + makeResource := func(t *testing.T, y string) *yaml.RNode { + t.Helper() + node, err := yaml.Parse(y) + require.NoError(t, err) + return node + } + + // Two ConfigMaps with the same name but different namespaces + resources := []*yaml.RNode{ + makeResource(t, ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-config + namespace: ns-a +data: + key: a +`), + makeResource(t, ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-config + namespace: ns-b +data: + key: b +`), + } + + t.Run("ambiguous match without namespace", func(t *testing.T) { + ref := &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + } + _, err := ResolveConfigRef(ref, "", resources) + require.Error(t, err) + assert.Contains(t, err.Error(), "matched 2 resources") + }) + + t.Run("disambiguate with namespace", func(t *testing.T) { + ref := &kptfilev1.ResourceReference{ + Kind: "ConfigMap", + Name: "my-config", + Namespace: "ns-b", + } + result, err := ResolveConfigRef(ref, "", resources) + require.NoError(t, err) + meta, err := result.GetMeta() + require.NoError(t, err) + assert.Equal(t, "ns-b", meta.Namespace) + }) +} diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index 05d4e81556..c9b458b9c4 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -787,6 +787,17 @@ func (pn *pkgNode) runMutators(ctx context.Context, hctx *hydrationContext, inpu } } + if pl.Mutators[i].ConfigRef != nil { + // Resolve configRef from the current input resources. Like configPath, + // the referenced resource may have been mutated by earlier pipeline steps, + // so we resolve it fresh before each execution. + resolved, err := fnruntime.ResolveConfigRef(pl.Mutators[i].ConfigRef, pn.pkg.UniquePath, input) + if err != nil { + return nil, err + } + mutator.SetFnConfig(resolved) + } + selectors := pl.Mutators[i].Selectors exclusions := pl.Mutators[i].Exclusions @@ -870,7 +881,7 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in opts := hctx.runnerOptions opts.SetPkgPathAnnotation = true opts.DisplayResourceCount = displayResourceCount - validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime) + validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime, input) if err != nil { hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) return err diff --git a/pkg/lib/pkg/pkg.go b/pkg/lib/pkg/pkg.go index 6bdf7e0b31..b19077b492 100644 --- a/pkg/lib/pkg/pkg.go +++ b/pkg/lib/pkg/pkg.go @@ -449,11 +449,21 @@ func (p *Pkg) ValidatePipeline() error { if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) { errs = append(errs, makeStepValidationErr("mutator", i, fn.Name, fn.Image, fn.ConfigPath)) } + if fn.ConfigRef != nil { + if err := validateConfigRefExists("mutators", i, fn, resources); err != nil { + errs = append(errs, err) + } + } } for i, fn := range pl.Validators { if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) { errs = append(errs, makeStepValidationErr("validator", i, fn.Name, fn.Image, fn.ConfigPath)) } + if fn.ConfigRef != nil { + if err := validateConfigRefExists("validators", i, fn, resources); err != nil { + errs = append(errs, err) + } + } } return stderrors.Join(errs...) } @@ -467,6 +477,40 @@ func makeStepValidationErr(stepType string, i int, fnName, fnImage, configPath s } } +// validateConfigRefExists checks that the resource referenced by configRef exists +// among the given resources. +func validateConfigRefExists(fnType string, i int, fn kptfilev1.Function, resources []*yaml.RNode) error { + ref := fn.ConfigRef + for _, r := range resources { + meta, err := r.GetMeta() + if err != nil { + continue + } + if ref.APIVersion != "" && meta.APIVersion != ref.APIVersion { + continue + } + if meta.Kind != ref.Kind { + continue + } + if meta.Name != ref.Name { + continue + } + if ref.Namespace != "" && meta.Namespace != ref.Namespace { + continue + } + return nil // found + } + refDesc := ref.Kind + "/" + ref.Name + if ref.APIVersion != "" { + refDesc = ref.APIVersion + "/" + refDesc + } + return &kptfilev1.ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configRef", fnType, i), + Value: refDesc, + Reason: fmt.Sprintf("referenced resource %q not found in package for function %q", refDesc, PipelineStepNameOrImage(fn.Name, fn.Image)), + } +} + // PipelineStepNameOrImage returns the name if it's not empty or extracts the base name of the image func PipelineStepNameOrImage(name, image string) string { if name != "" { From 074c67bbdaa07d50219049e29d97af6d199b21af Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Mon, 27 Jul 2026 15:27:48 +0100 Subject: [PATCH 2/7] tracked e2e tests Signed-off-by: Oisin Johnston --- pkg/lib/kptops/configref_test.go | 280 +++++++++++++++++++++++++++++++ 1 file changed, 280 insertions(+) create mode 100644 pkg/lib/kptops/configref_test.go diff --git a/pkg/lib/kptops/configref_test.go b/pkg/lib/kptops/configref_test.go new file mode 100644 index 0000000000..f6b425c1c3 --- /dev/null +++ b/pkg/lib/kptops/configref_test.go @@ -0,0 +1,280 @@ +// Copyright 2026 The kpt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kptops + +import ( + "testing" + + "github.com/kptdev/kpt/pkg/lib/runneroptions" + "github.com/kptdev/kpt/pkg/printer/fake" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +const testDeploymentResource = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-app +spec: + replicas: 3 +` + +func TestRenderWithConfigRef_Mutator(t *testing.T) { + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configRef: + kind: ConfigMap + name: ns-config +` + + fnConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: ns-config + annotations: + config.kubernetes.io/local-config: "true" +data: + namespace: production +` + + resources := testDeploymentResource + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + _, err := r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the deployment got the namespace set + res, err := fs.ReadFile("/pkg/resources.yaml") + require.NoError(t, err) + assert.Contains(t, string(res), "namespace: production") +} + +func TestRenderWithConfigRef_Validator(t *testing.T) { + // Validators run on a copy but shouldn't error if configRef resolves + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configMap: + namespace: staging + validators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest + configRef: + kind: ConfigMap + name: label-config +` + + fnConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: label-config + annotations: + config.kubernetes.io/local-config: "true" +data: + env: staging +` + + resources := testDeploymentResource + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + _, err := r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the mutator ran (namespace set) and the validator didn't error + res, err := fs.ReadFile("/pkg/resources.yaml") + require.NoError(t, err) + assert.Contains(t, string(res), "namespace: staging") +} + +func TestRenderWithConfigRef_NotFound(t *testing.T) { + // configRef pointing to a nonexistent resource should fail + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configRef: + kind: ConfigMap + name: does-not-exist +` + + resources := testDeploymentResource + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + _, err := r.Execute(fake.CtxWithDefaultPrinter()) + require.Error(t, err) + assert.Contains(t, err.Error(), "does-not-exist") +} + +func TestRenderWithConfigRef_APIVersionFilter(t *testing.T) { + // configRef with apiVersion should match only resources with that apiVersion + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configRef: + apiVersion: v1 + kind: ConfigMap + name: ns-config +` + + fnConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: ns-config + annotations: + config.kubernetes.io/local-config: "true" +data: + namespace: filtered +` + + resources := testDeploymentResource + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + _, err := r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + res, err := fs.ReadFile("/pkg/resources.yaml") + require.NoError(t, err) + assert.Contains(t, string(res), "namespace: filtered") +} + +func TestRenderWithConfigRef_MutatorChain(t *testing.T) { + // Test that a configRef resource can be mutated by an earlier pipeline step + // and the later step sees the updated config. + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-labels:latest + configMap: + tier: backend + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configRef: + kind: ConfigMap + name: ns-config +` + + // This ConfigMap will be passed through the first mutator (set-labels), + // which will add a label to it, then used as config for the second mutator. + fnConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: ns-config +data: + namespace: chained +` + + resources := testDeploymentResource + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + _, err := r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the second mutator got the config and set the namespace + res, err := fs.ReadFile("/pkg/resources.yaml") + require.NoError(t, err) + assert.Contains(t, string(res), "namespace: chained") +} From 9b59269e024923f2cc6d584fd93786d4c0c66b20 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Tue, 28 Jul 2026 09:53:24 +0100 Subject: [PATCH 3/7] cleaned stuff up a bit Signed-off-by: Oisin Johnston --- api/kptfile/v1/types.go | 18 ++++++++++++++++ pkg/fn/runtime/runner.go | 34 ++++--------------------------- pkg/fn/runtime/runner_test.go | 10 ++++----- pkg/lib/kptops/hook_executor.go | 3 +++ pkg/lib/kptops/render_executor.go | 14 +++++++++++-- pkg/lib/pkg/pkg.go | 14 ++----------- 6 files changed, 44 insertions(+), 49 deletions(-) diff --git a/api/kptfile/v1/types.go b/api/kptfile/v1/types.go index 264a253bcb..d3938d0840 100644 --- a/api/kptfile/v1/types.go +++ b/api/kptfile/v1/types.go @@ -382,6 +382,24 @@ type ResourceReference struct { Namespace string `yaml:"namespace,omitempty" json:"namespace,omitempty"` } +// Matches reports whether the given resource metadata satisfies this reference. +// Optional fields (APIVersion, Namespace) only filter when non-empty. +func (r *ResourceReference) Matches(meta yaml.ResourceMeta) bool { + if r.APIVersion != "" && meta.APIVersion != r.APIVersion { + return false + } + if meta.Kind != r.Kind { + return false + } + if meta.Name != r.Name { + return false + } + if r.Namespace != "" && meta.Namespace != r.Namespace { + return false + } + return true +} + // Selector specifies the selection criteria // please update IsEmpty method if more properties are added type Selector struct { diff --git a/pkg/fn/runtime/runner.go b/pkg/fn/runtime/runner.go index 0629624239..cf44607c6f 100644 --- a/pkg/fn/runtime/runner.go +++ b/pkg/fn/runtime/runner.go @@ -51,13 +51,8 @@ func NewRunner( fnResults *fnresultv1.ResultList, opts runneroptions.RunnerOptions, runtime fn.FunctionRuntime, - resources ...[]*yaml.RNode, ) (*FunctionRunner, error) { - var inputResources []*yaml.RNode - if len(resources) > 0 { - inputResources = resources[0] - } - config, err := newFnConfig(fsys, f, pkgPath, inputResources) + config, err := newFnConfig(fsys, f, pkgPath) if err != nil { return nil, err } @@ -510,7 +505,7 @@ func enforcePathInvariants(nodes []*yaml.RNode) error { return nil } -func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfilev1.UniquePath, resources []*yaml.RNode) (*yaml.RNode, error) { +func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfilev1.UniquePath) (*yaml.RNode, error) { const op errors.Op = "fn.readConfig" fn := errors.Fn(f.Image) @@ -540,17 +535,6 @@ func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfile return nil, errors.E(op, fn, err) } return configNode, nil - case f.ConfigRef != nil: - if resources == nil { - // Resources not available yet (e.g. during mutator chain construction). - // Config will be resolved from the input list before execution. - return nil, nil - } - node, err := ResolveConfigRef(f.ConfigRef, pkgPath, resources) - if err != nil { - return nil, errors.E(op, fn, err) - } - return node, nil } // no need to return ConfigMap if no config given return nil, nil @@ -573,19 +557,9 @@ func ResolveConfigRef(ref *kptfilev1.ResourceReference, pkgPath kptfilev1.Unique continue } } - if ref.APIVersion != "" && meta.APIVersion != ref.APIVersion { - continue - } - if meta.Kind != ref.Kind { - continue - } - if meta.Name != ref.Name { - continue - } - if ref.Namespace != "" && meta.Namespace != ref.Namespace { - continue + if ref.Matches(meta) { + matches = append(matches, r) } - matches = append(matches, r) } switch len(matches) { case 0: diff --git a/pkg/fn/runtime/runner_test.go b/pkg/fn/runtime/runner_test.go index b92c3e9326..d9f72e3b5e 100644 --- a/pkg/fn/runtime/runner_test.go +++ b/pkg/fn/runtime/runner_test.go @@ -91,7 +91,7 @@ data: {foo: bar} c.fn.ConfigPath = path.Base(tmp.Name()) } fsys := filesys.MakeFsOnDisk() - cn, err := newFnConfig(fsys, &c.fn, kptfilev1.UniquePath(os.TempDir()), nil) + cn, err := newFnConfig(fsys, &c.fn, kptfilev1.UniquePath(os.TempDir())) assert.NoError(t, err, "unexpected error") actual, err := cn.String() assert.NoError(t, err, "unexpected error") @@ -887,10 +887,10 @@ data: } tests := []struct { - name string - ref *kptfilev1.ResourceReference - expectName string - expectErr string + name string + ref *kptfilev1.ResourceReference + expectName string + expectErr string }{ { name: "match by kind and name", diff --git a/pkg/lib/kptops/hook_executor.go b/pkg/lib/kptops/hook_executor.go index 71a45afd0a..e3e5a8a7ea 100644 --- a/pkg/lib/kptops/hook_executor.go +++ b/pkg/lib/kptops/hook_executor.go @@ -83,6 +83,9 @@ func (e *Executor) fnChain(ctx context.Context, fns []kptfilev1.Function) ([]kio var err error var runner kio.Filter fn := fns[i] + if fn.ConfigRef != nil { + return nil, fmt.Errorf("configRef is not supported in lifecycle hooks (function %q)", fn.Image) + } if fn.Exec != "" && !e.RunnerOptions.AllowExec { return nil, ErrAllowedExecNotSpecified } diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index c9b458b9c4..e712f8d304 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -791,6 +791,8 @@ func (pn *pkgNode) runMutators(ctx context.Context, hctx *hydrationContext, inpu // Resolve configRef from the current input resources. Like configPath, // the referenced resource may have been mutated by earlier pipeline steps, // so we resolve it fresh before each execution. + // Note: the resource remains in the input list (same as configPath behaviour) + // so that mutations to it are preserved across pipeline steps. resolved, err := fnruntime.ResolveConfigRef(pl.Mutators[i].ConfigRef, pn.pkg.UniquePath, input) if err != nil { return nil, err @@ -869,7 +871,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 @@ -881,11 +883,19 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in opts := hctx.runnerOptions opts.SetPkgPathAnnotation = true opts.DisplayResourceCount = displayResourceCount - validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime, input) + validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime) if err != nil { hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) return err } + if function.ConfigRef != nil { + resolved, err := fnruntime.ResolveConfigRef(function.ConfigRef, pn.pkg.UniquePath, input) + if err != nil { + hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) + return err + } + validator.SetFnConfig(resolved) + } 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/pkg/pkg.go b/pkg/lib/pkg/pkg.go index b19077b492..71d8b9ceb9 100644 --- a/pkg/lib/pkg/pkg.go +++ b/pkg/lib/pkg/pkg.go @@ -486,19 +486,9 @@ func validateConfigRefExists(fnType string, i int, fn kptfilev1.Function, resour if err != nil { continue } - if ref.APIVersion != "" && meta.APIVersion != ref.APIVersion { - continue - } - if meta.Kind != ref.Kind { - continue - } - if meta.Name != ref.Name { - continue - } - if ref.Namespace != "" && meta.Namespace != ref.Namespace { - continue + if ref.Matches(meta) { + return nil // found } - return nil // found } refDesc := ref.Kind + "/" + ref.Name if ref.APIVersion != "" { From f265ac6acfe76f5ed8470ff113ad93ac259884a1 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Tue, 28 Jul 2026 10:10:24 +0100 Subject: [PATCH 4/7] fixed sonarqube complaints Signed-off-by: Oisin Johnston --- pkg/lib/kptops/configref_test.go | 129 +++++++++++-------------------- 1 file changed, 46 insertions(+), 83 deletions(-) diff --git a/pkg/lib/kptops/configref_test.go b/pkg/lib/kptops/configref_test.go index f6b425c1c3..a815294a8b 100644 --- a/pkg/lib/kptops/configref_test.go +++ b/pkg/lib/kptops/configref_test.go @@ -32,6 +32,38 @@ spec: replicas: 3 ` +// configRefTestSetup holds the filesystem and renderer for configRef tests. +type configRefTestSetup struct { + fs filesys.FileSystem + r Renderer +} + +// setupConfigRefTest creates an in-memory filesystem populated with the given +// Kptfile, optional function config, and the standard test deployment resource, +// then returns a ready-to-use Renderer. Pass an empty string for fnConfig to +// skip writing fn-config.yaml. +func setupConfigRefTest(t *testing.T, kptfile, fnConfig string) configRefTestSetup { + t.Helper() + + fs := filesys.MakeFsInMemory() + require.NoError(t, fs.MkdirAll("/pkg")) + require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) + if fnConfig != "" { + require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) + } + require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(testDeploymentResource))) + + r := Renderer{ + PkgPath: "/pkg", + FileSystem: fs, + Runtime: &runtime{}, + } + r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) + r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + + return configRefTestSetup{fs: fs, r: r} +} + func TestRenderWithConfigRef_Mutator(t *testing.T) { kptfile := `apiVersion: kpt.dev/v1 kind: Kptfile @@ -57,27 +89,13 @@ data: namespace: production ` - resources := testDeploymentResource + ts := setupConfigRefTest(t, kptfile, fnConfig) - fs := filesys.MakeFsInMemory() - require.NoError(t, fs.MkdirAll("/pkg")) - require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) - require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) - require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) - - r := Renderer{ - PkgPath: "/pkg", - FileSystem: fs, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull - - _, err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) require.NoError(t, err) // Verify the deployment got the namespace set - res, err := fs.ReadFile("/pkg/resources.yaml") + res, err := ts.fs.ReadFile("/pkg/resources.yaml") require.NoError(t, err) assert.Contains(t, string(res), "namespace: production") } @@ -112,27 +130,13 @@ data: env: staging ` - resources := testDeploymentResource - - fs := filesys.MakeFsInMemory() - require.NoError(t, fs.MkdirAll("/pkg")) - require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) - require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) - require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) - - r := Renderer{ - PkgPath: "/pkg", - FileSystem: fs, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + ts := setupConfigRefTest(t, kptfile, fnConfig) - _, err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) require.NoError(t, err) // Verify the mutator ran (namespace set) and the validator didn't error - res, err := fs.ReadFile("/pkg/resources.yaml") + res, err := ts.fs.ReadFile("/pkg/resources.yaml") require.NoError(t, err) assert.Contains(t, string(res), "namespace: staging") } @@ -153,22 +157,9 @@ pipeline: name: does-not-exist ` - resources := testDeploymentResource - - fs := filesys.MakeFsInMemory() - require.NoError(t, fs.MkdirAll("/pkg")) - require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) - require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) - - r := Renderer{ - PkgPath: "/pkg", - FileSystem: fs, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + ts := setupConfigRefTest(t, kptfile, "") - _, err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) require.Error(t, err) assert.Contains(t, err.Error(), "does-not-exist") } @@ -200,26 +191,12 @@ data: namespace: filtered ` - resources := testDeploymentResource + ts := setupConfigRefTest(t, kptfile, fnConfig) - fs := filesys.MakeFsInMemory() - require.NoError(t, fs.MkdirAll("/pkg")) - require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) - require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) - require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) - - r := Renderer{ - PkgPath: "/pkg", - FileSystem: fs, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull - - _, err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) require.NoError(t, err) - res, err := fs.ReadFile("/pkg/resources.yaml") + res, err := ts.fs.ReadFile("/pkg/resources.yaml") require.NoError(t, err) assert.Contains(t, string(res), "namespace: filtered") } @@ -254,27 +231,13 @@ data: namespace: chained ` - resources := testDeploymentResource - - fs := filesys.MakeFsInMemory() - require.NoError(t, fs.MkdirAll("/pkg")) - require.NoError(t, fs.WriteFile("/pkg/Kptfile", []byte(kptfile))) - require.NoError(t, fs.WriteFile("/pkg/fn-config.yaml", []byte(fnConfig))) - require.NoError(t, fs.WriteFile("/pkg/resources.yaml", []byte(resources))) - - r := Renderer{ - PkgPath: "/pkg", - FileSystem: fs, - Runtime: &runtime{}, - } - r.RunnerOptions.InitDefaults(runneroptions.GHCRImagePrefix) - r.RunnerOptions.ImagePullPolicy = runneroptions.IfNotPresentPull + ts := setupConfigRefTest(t, kptfile, fnConfig) - _, err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) require.NoError(t, err) // Verify the second mutator got the config and set the namespace - res, err := fs.ReadFile("/pkg/resources.yaml") + res, err := ts.fs.ReadFile("/pkg/resources.yaml") require.NoError(t, err) assert.Contains(t, string(res), "namespace: chained") } From 2b6d417e1674d625c5d3d42e086edf6c3f1a159d Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Tue, 28 Jul 2026 14:30:31 +0100 Subject: [PATCH 5/7] fixed docker test Signed-off-by: Oisin Johnston --- e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml index 860538b659..b19c287080 100644 --- a/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml +++ b/e2e/testdata/fn-render/multiple-fnconfig/.expected/config.yaml @@ -13,4 +13,4 @@ # limitations under the License. exitCode: 1 -stdErr: 'functionConfig must not specify both `configMap` and `configPath` at the same time' +stdErr: "Kptfile is invalid:\nField: `pipeline.mutators[0]`\nReason: functionConfig must specify at most one of `configMap`, `configPath`, or `configRef`" From 8d0d901c3f9150540b7144f2ea74454224a0e757 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Wed, 29 Jul 2026 16:40:22 +0100 Subject: [PATCH 6/7] applies copilot comments Signed-off-by: Oisin Johnston --- pkg/fn/runtime/runner.go | 21 +++++++++++++-- pkg/fn/runtime/runner_test.go | 6 ++--- pkg/lib/kptops/configref_test.go | 45 ++++++++++++++++++++++++++++++++ pkg/lib/pkg/pkg.go | 33 +++++++++++++++++------ 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/pkg/fn/runtime/runner.go b/pkg/fn/runtime/runner.go index cf44607c6f..22a55a1d31 100644 --- a/pkg/fn/runtime/runner.go +++ b/pkg/fn/runtime/runner.go @@ -563,14 +563,31 @@ func ResolveConfigRef(ref *kptfilev1.ResourceReference, pkgPath kptfilev1.Unique } switch len(matches) { case 0: - return nil, fmt.Errorf("configRef: resource %s/%s %q not found in package", ref.APIVersion, ref.Kind, ref.Name) + return nil, fmt.Errorf("configRef: resource %s not found in package", formatResourceRef(ref)) case 1: return matches[0], nil default: - return nil, fmt.Errorf("configRef: resource %s/%s %q matched %d resources, must match exactly one", ref.APIVersion, ref.Kind, ref.Name, len(matches)) + return nil, fmt.Errorf("configRef: resource %s matched %d resources, must match exactly one", formatResourceRef(ref), len(matches)) } } +// formatResourceRef produces a human-readable representation of a resource +// reference for error messages. It omits empty fields to avoid confusing +// output like "/ConfigMap". +func formatResourceRef(ref *kptfilev1.ResourceReference) string { + var s string + if ref.APIVersion != "" { + s = ref.APIVersion + "/" + ref.Kind + } else { + s = ref.Kind + } + s += " " + fmt.Sprintf("%q", ref.Name) + if ref.Namespace != "" { + s += " in namespace " + fmt.Sprintf("%q", ref.Namespace) + } + return s +} + // hasTagOrDigest reports whether the image reference contains an explicit tag or digest. func hasTagOrDigest(image string) bool { if strings.Contains(image, "@") { diff --git a/pkg/fn/runtime/runner_test.go b/pkg/fn/runtime/runner_test.go index d9f72e3b5e..979dfd5d74 100644 --- a/pkg/fn/runtime/runner_test.go +++ b/pkg/fn/runtime/runner_test.go @@ -924,7 +924,7 @@ data: Kind: "ConfigMap", Name: "nonexistent", }, - expectErr: `configRef: resource /ConfigMap "nonexistent" not found in package`, + expectErr: `configRef: resource ConfigMap "nonexistent" not found in package`, }, { name: "no match - wrong kind", @@ -932,7 +932,7 @@ data: Kind: "Secret", Name: "my-config", }, - expectErr: `configRef: resource /Secret "my-config" not found in package`, + expectErr: `configRef: resource Secret "my-config" not found in package`, }, { name: "no match - apiVersion mismatch", @@ -950,7 +950,7 @@ data: Name: "other-config", Namespace: "production", }, - expectErr: `configRef: resource /ConfigMap "other-config" not found in package`, + expectErr: `configRef: resource ConfigMap "other-config" in namespace "production" not found in package`, }, } diff --git a/pkg/lib/kptops/configref_test.go b/pkg/lib/kptops/configref_test.go index a815294a8b..2c85a38855 100644 --- a/pkg/lib/kptops/configref_test.go +++ b/pkg/lib/kptops/configref_test.go @@ -241,3 +241,48 @@ data: require.NoError(t, err) assert.Contains(t, string(res), "namespace: chained") } + +func TestRenderWithConfigRef_AmbiguousMatch(t *testing.T) { + // configRef that matches multiple resources should fail at validation + kptfile := `apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: test-pkg + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:latest + configRef: + kind: ConfigMap + name: ns-config +` + + // Two ConfigMaps with the same name but different namespaces + fnConfig := `apiVersion: v1 +kind: ConfigMap +metadata: + name: ns-config + namespace: ns-a + annotations: + config.kubernetes.io/local-config: "true" +data: + namespace: production +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ns-config + namespace: ns-b + annotations: + config.kubernetes.io/local-config: "true" +data: + namespace: staging +` + + ts := setupConfigRefTest(t, kptfile, fnConfig) + + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) + require.Error(t, err) + assert.Contains(t, err.Error(), "matched 2 resources") +} diff --git a/pkg/lib/pkg/pkg.go b/pkg/lib/pkg/pkg.go index 71d8b9ceb9..5c53dcdace 100644 --- a/pkg/lib/pkg/pkg.go +++ b/pkg/lib/pkg/pkg.go @@ -477,27 +477,44 @@ func makeStepValidationErr(stepType string, i int, fnName, fnImage, configPath s } } -// validateConfigRefExists checks that the resource referenced by configRef exists -// among the given resources. +// validateConfigRefExists checks that the resource referenced by configRef +// resolves to exactly one resource among the given resources. func validateConfigRefExists(fnType string, i int, fn kptfilev1.Function, resources []*yaml.RNode) error { ref := fn.ConfigRef + matchCount := 0 for _, r := range resources { meta, err := r.GetMeta() if err != nil { continue } if ref.Matches(meta) { - return nil // found + matchCount++ } } - refDesc := ref.Kind + "/" + ref.Name + + refDesc := ref.Kind + " " + fmt.Sprintf("%q", ref.Name) if ref.APIVersion != "" { refDesc = ref.APIVersion + "/" + refDesc } - return &kptfilev1.ValidateError{ - Field: fmt.Sprintf("pipeline.%s[%d].configRef", fnType, i), - Value: refDesc, - Reason: fmt.Sprintf("referenced resource %q not found in package for function %q", refDesc, PipelineStepNameOrImage(fn.Name, fn.Image)), + if ref.Namespace != "" { + refDesc += " in namespace " + fmt.Sprintf("%q", ref.Namespace) + } + + switch { + case matchCount == 0: + return &kptfilev1.ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configRef", fnType, i), + Value: refDesc, + Reason: fmt.Sprintf("referenced resource %s not found in package for function %q", refDesc, PipelineStepNameOrImage(fn.Name, fn.Image)), + } + case matchCount > 1: + return &kptfilev1.ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configRef", fnType, i), + Value: refDesc, + Reason: fmt.Sprintf("referenced resource %s matched %d resources, must match exactly one; use namespace to disambiguate", refDesc, matchCount), + } + default: + return nil } } From 59c533454eb0fd8cb4fd51df21efe7cbcdefd116 Mon Sep 17 00:00:00 2001 From: Oisin Johnston Date: Thu, 30 Jul 2026 16:36:08 +0100 Subject: [PATCH 7/7] fixed complexity issues Signed-off-by: Oisin Johnston --- api/kptfile/v1/validation.go | 61 ++++++++++++++++-------- pkg/lib/kptops/render_executor.go | 79 ++++++++++++++++++------------- pkg/lib/pkg/pkg.go | 47 ++++++++++-------- 3 files changed, 114 insertions(+), 73 deletions(-) diff --git a/api/kptfile/v1/validation.go b/api/kptfile/v1/validation.go index 9369e1f668..b0c34bda92 100644 --- a/api/kptfile/v1/validation.go +++ b/api/kptfile/v1/validation.go @@ -66,6 +66,28 @@ func (p *Pipeline) validate(fsys filesys.FileSystem, pkgPath UniquePath) error { } func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkgPath UniquePath) error { + if err := f.validateExecutor(fnType, idx); err != nil { + return err + } + if err := f.validateConfigSources(fnType, idx); err != nil { + return err + } + if f.ConfigRef != nil { + if err := f.ConfigRef.validate(fnType, idx); err != nil { + return err + } + } + if f.ConfigPath != "" { + if err := f.validateConfigPath(fsys, fnType, idx, pkgPath); err != nil { + return err + } + } + return nil +} + +// validateExecutor checks that exactly one of Image or Exec is specified and that +// the image reference is syntactically valid. +func (f *Function) validateExecutor(fnType string, idx int) error { if f.Image == "" && f.Exec == "" { return &ValidateError{ Field: fmt.Sprintf("pipeline.%s[%d]", fnType, idx), @@ -79,8 +101,7 @@ func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkg } } if f.Image != "" { - err := ValidateFunctionImageURL(f.Image) - if err != nil { + if err := ValidateFunctionImageURL(f.Image); err != nil { return &ValidateError{ Field: fmt.Sprintf("pipeline.%s[%d].image", fnType, idx), Value: f.Image, @@ -89,7 +110,11 @@ func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkg } } // TODO(droot): validate the exec + return nil +} +// validateConfigSources ensures at most one of configMap, configPath, or configRef is set. +func (f *Function) validateConfigSources(fnType string, idx int) error { configSources := 0 if len(f.ConfigMap) != 0 { configSources++ @@ -106,27 +131,23 @@ func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkg Reason: "functionConfig must specify at most one of `configMap`, `configPath`, or `configRef`", } } + return nil +} - if f.ConfigRef != nil { - if err := f.ConfigRef.validate(fnType, idx); err != nil { - return err +// validateConfigPath validates the configPath syntax and verifies the referenced file exists. +func (f *Function) validateConfigPath(fsys filesys.FileSystem, fnType string, idx int, pkgPath UniquePath) error { + if err := validateFnConfigPathSyntax(f.ConfigPath); err != nil { + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx), + Value: f.ConfigPath, + Reason: err.Error(), } } - - if f.ConfigPath != "" { - if err := validateFnConfigPathSyntax(f.ConfigPath); err != nil { - return &ValidateError{ - Field: fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx), - Value: f.ConfigPath, - Reason: err.Error(), - } - } - if _, err := GetValidatedFnConfigFromPath(fsys, pkgPath, f.ConfigPath); err != nil { - return &ValidateError{ - Field: fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx), - Value: f.ConfigPath, - Reason: err.Error(), - } + if _, err := GetValidatedFnConfigFromPath(fsys, pkgPath, f.ConfigPath); err != nil { + return &ValidateError{ + Field: fmt.Sprintf("pipeline.%s[%d].configPath", fnType, idx), + Value: f.ConfigPath, + Reason: err.Error(), } } return nil diff --git a/pkg/lib/kptops/render_executor.go b/pkg/lib/kptops/render_executor.go index e712f8d304..8dff9abeaa 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -863,49 +863,62 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in } for i := range pl.Validators { - function := pl.Validators[i] - resultCountBeforeExec := len(hctx.fnResults.Items) - // validators are run on a copy of mutated resources to ensure - // resources are not mutated. - selectedResources, err := fnruntime.SelectInput(input, function.Selectors, function.Exclusions, &fnruntime.SelectionContext{RootPackagePath: hctx.root.pkg.UniquePath}) - if err != nil { + if err := pn.runSingleValidator(ctx, hctx, input, pl.Validators[i]); err != nil { return err } - var validator *fnruntime.FunctionRunner - displayResourceCount := false - if len(function.Selectors) > 0 || len(function.Exclusions) > 0 { - displayResourceCount = true - } - if function.Exec != "" && !hctx.runnerOptions.AllowExec { - hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, errAllowedExecNotSpecified)) - return errAllowedExecNotSpecified - } - opts := hctx.runnerOptions - opts.SetPkgPathAnnotation = true - opts.DisplayResourceCount = displayResourceCount - validator, err = fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime) + } + return nil +} + +// runSingleValidator executes a single validator function against the input resources. +func (pn *pkgNode) runSingleValidator(ctx context.Context, hctx *hydrationContext, input []*yaml.RNode, function kptfilev1.Function) error { + resultCountBeforeExec := len(hctx.fnResults.Items) + + // validators are run on a copy of mutated resources to ensure + // resources are not mutated. + selectedResources, err := fnruntime.SelectInput(input, function.Selectors, function.Exclusions, &fnruntime.SelectionContext{RootPackagePath: hctx.root.pkg.UniquePath}) + if err != nil { + return err + } + + if function.Exec != "" && !hctx.runnerOptions.AllowExec { + hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, errAllowedExecNotSpecified)) + return errAllowedExecNotSpecified + } + + validator, err := pn.createValidatorRunner(ctx, hctx, function) + if err != nil { + hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) + return err + } + + if function.ConfigRef != nil { + resolved, err := fnruntime.ResolveConfigRef(function.ConfigRef, pn.pkg.UniquePath, input) if err != nil { hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) return err } - if function.ConfigRef != nil { - resolved, err := fnruntime.ResolveConfigRef(function.ConfigRef, pn.pkg.UniquePath, input) - if err != nil { - hctx.validationSteps = append(hctx.validationSteps, preExecFailureStep(function, err)) - return err - } - validator.SetFnConfig(resolved) - } - if _, err = validator.Filter(cloneResources(selectedResources)); err != nil { - hctx.validationSteps = append(hctx.validationSteps, captureStepResult(function, hctx.fnResults, resultCountBeforeExec, err)) - return err - } - hctx.executedFunctionCnt++ - hctx.validationSteps = append(hctx.validationSteps, captureStepResult(function, hctx.fnResults, resultCountBeforeExec, nil)) + validator.SetFnConfig(resolved) + } + + if _, err = validator.Filter(cloneResources(selectedResources)); err != nil { + hctx.validationSteps = append(hctx.validationSteps, captureStepResult(function, hctx.fnResults, resultCountBeforeExec, err)) + return err } + hctx.executedFunctionCnt++ + hctx.validationSteps = append(hctx.validationSteps, captureStepResult(function, hctx.fnResults, resultCountBeforeExec, nil)) return nil } +// createValidatorRunner constructs a FunctionRunner for the given validator function. +func (pn *pkgNode) createValidatorRunner(ctx context.Context, hctx *hydrationContext, function kptfilev1.Function) (*fnruntime.FunctionRunner, error) { + displayResourceCount := len(function.Selectors) > 0 || len(function.Exclusions) > 0 + opts := hctx.runnerOptions + opts.SetPkgPathAnnotation = true + opts.DisplayResourceCount = displayResourceCount + return fnruntime.NewRunner(ctx, hctx.fileSystem, &function, pn.pkg.UniquePath, hctx.fnResults, opts, hctx.runtime) +} + func cloneResources(input []*yaml.RNode) (output []*yaml.RNode) { for _, resource := range input { output = append(output, resource.Copy()) diff --git a/pkg/lib/pkg/pkg.go b/pkg/lib/pkg/pkg.go index 5c53dcdace..7ba5d7a59f 100644 --- a/pkg/lib/pkg/pkg.go +++ b/pkg/lib/pkg/pkg.go @@ -423,49 +423,56 @@ func (p *Pkg) ValidatePipeline() error { return nil } - // read all resources including function pipeline. - resources, err := p.LocalResources() + resourcesByPath, resources, err := p.buildResourcePathSet() if err != nil { return err } - resourcesByPath := sets.String{} + var errs []error + errs = append(errs, validatePipelineFunctions("mutators", pl.Mutators, resourcesByPath, resources)...) + errs = append(errs, validatePipelineFunctions("validators", pl.Validators, resourcesByPath, resources)...) + return stderrors.Join(errs...) +} - // TODO: should we use go.uber.org/multierr instead? +// buildResourcePathSet reads all local resources and returns a set of their file paths +// along with the resources themselves. +func (p *Pkg) buildResourcePathSet() (sets.String, []*yaml.RNode, error) { + resources, err := p.LocalResources() + if err != nil { + return nil, nil, err + } + + resourcesByPath := sets.String{} var errs []error for _, r := range resources { rPath, _, err := kioutil.GetFileAnnotations(r) if err != nil { errs = append(errs, fmt.Errorf("resource %q missing path annotation err: %w", r.GetName(), err)) + continue } resourcesByPath.Insert(filepath.Clean(rPath)) } - if len(errs) > 0 { - return stderrors.Join(errs...) + return nil, nil, stderrors.Join(errs...) } + return resourcesByPath, resources, nil +} - for i, fn := range pl.Mutators { - if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) { - errs = append(errs, makeStepValidationErr("mutator", i, fn.Name, fn.Image, fn.ConfigPath)) - } - if fn.ConfigRef != nil { - if err := validateConfigRefExists("mutators", i, fn, resources); err != nil { - errs = append(errs, err) - } - } - } - for i, fn := range pl.Validators { +// validatePipelineFunctions validates configPath and configRef for a slice of pipeline functions. +func validatePipelineFunctions(fnType string, fns []kptfilev1.Function, resourcesByPath sets.String, resources []*yaml.RNode) []error { + var errs []error + stepType := strings.TrimSuffix(fnType, "s") // "mutators" -> "mutator" + for i, fn := range fns { if fn.ConfigPath != "" && !resourcesByPath.Has(filepath.Clean(fn.ConfigPath)) { - errs = append(errs, makeStepValidationErr("validator", i, fn.Name, fn.Image, fn.ConfigPath)) + errs = append(errs, makeStepValidationErr(stepType, i, fn.Name, fn.Image, fn.ConfigPath)) } if fn.ConfigRef != nil { - if err := validateConfigRefExists("validators", i, fn, resources); err != nil { + if err := validateConfigRefExists(fnType, i, fn, resources); err != nil { errs = append(errs, err) } } } - return stderrors.Join(errs...) + return errs } func makeStepValidationErr(stepType string, i int, fnName, fnImage, configPath string) *kptfilev1.ValidateError {