diff --git a/api/kptfile/v1/types.go b/api/kptfile/v1/types.go index f42b6c52f3..d3938d0840 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,38 @@ 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"` +} + +// 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/api/kptfile/v1/validation.go b/api/kptfile/v1/validation.go index 6be7006a9f..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,28 +110,44 @@ func (f *Function) validate(fsys filesys.FileSystem, fnType string, idx int, pkg } } // TODO(droot): validate the exec + return nil +} - if len(f.ConfigMap) != 0 && f.ConfigPath != "" { +// 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++ + } + 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`", } } + return nil +} - 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(), - } +// 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 _, 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 @@ -154,6 +191,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/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`" 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..22a55a1d31 100644 --- a/pkg/fn/runtime/runner.go +++ b/pkg/fn/runtime/runner.go @@ -540,6 +540,54 @@ func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath kptfile 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.Matches(meta) { + matches = append(matches, r) + } + } + switch len(matches) { + case 0: + 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 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 4d28f76d69..979dfd5d74 100644 --- a/pkg/fn/runtime/runner_test.go +++ b/pkg/fn/runtime/runner_test.go @@ -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" in namespace "production" 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/configref_test.go b/pkg/lib/kptops/configref_test.go new file mode 100644 index 0000000000..2c85a38855 --- /dev/null +++ b/pkg/lib/kptops/configref_test.go @@ -0,0 +1,288 @@ +// 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 +` + +// 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 +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 +` + + ts := setupConfigRefTest(t, kptfile, fnConfig) + + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the deployment got the namespace set + res, err := ts.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 +` + + ts := setupConfigRefTest(t, kptfile, fnConfig) + + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the mutator ran (namespace set) and the validator didn't error + res, err := ts.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 +` + + ts := setupConfigRefTest(t, kptfile, "") + + _, err := ts.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 +` + + ts := setupConfigRefTest(t, kptfile, fnConfig) + + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + res, err := ts.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 +` + + ts := setupConfigRefTest(t, kptfile, fnConfig) + + _, err := ts.r.Execute(fake.CtxWithDefaultPrinter()) + require.NoError(t, err) + + // Verify the second mutator got the config and set the namespace + res, err := ts.fs.ReadFile("/pkg/resources.yaml") + 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/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 05d4e81556..8dff9abeaa 100644 --- a/pkg/lib/kptops/render_executor.go +++ b/pkg/lib/kptops/render_executor.go @@ -787,6 +787,19 @@ 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. + // 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 + } + mutator.SetFnConfig(resolved) + } + selectors := pl.Mutators[i].Selectors exclusions := pl.Mutators[i].Exclusions @@ -850,41 +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 kio.Filter - 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 _, 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 6bdf7e0b31..7ba5d7a59f 100644 --- a/pkg/lib/pkg/pkg.go +++ b/pkg/lib/pkg/pkg.go @@ -423,39 +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...) +} + +// 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 + } - // TODO: should we use go.uber.org/multierr instead? + 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 { +// 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("mutator", i, fn.Name, fn.Image, fn.ConfigPath)) + errs = append(errs, makeStepValidationErr(stepType, i, fn.Name, fn.Image, fn.ConfigPath)) } - } - 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(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 { @@ -467,6 +484,47 @@ func makeStepValidationErr(stepType string, i int, fnName, fnImage, configPath s } } +// 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) { + matchCount++ + } + } + + refDesc := ref.Kind + " " + fmt.Sprintf("%q", ref.Name) + if ref.APIVersion != "" { + refDesc = ref.APIVersion + "/" + refDesc + } + 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 + } +} + // 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 != "" {