diff --git a/api/internal/plugins/fnplugin/fnplugin.go b/api/internal/plugins/fnplugin/fnplugin.go index 84bc0ac05..bf9d794d8 100644 --- a/api/internal/plugins/fnplugin/fnplugin.go +++ b/api/internal/plugins/fnplugin/fnplugin.go @@ -51,13 +51,16 @@ func resourceToRNode(res *resource.Resource) (*yaml.RNode, error) { } // GetFunctionSpec return function spec is there is. Otherwise return nil -func GetFunctionSpec(res *resource.Resource) *runtimeutil.FunctionSpec { +func GetFunctionSpec(res *resource.Resource) (*runtimeutil.FunctionSpec, error) { rnode, err := resourceToRNode(res) if err != nil { - return nil + return nil, fmt.Errorf("could not convert resource to RNode: %w", err) } - - return runtimeutil.GetFunctionSpec(rnode) + functionSpec, err := runtimeutil.GetFunctionSpec(rnode) + if err != nil { + return nil, fmt.Errorf("failed to get FunctionSpec: %w", err) + } + return functionSpec, nil } func toStorageMounts(mounts []string) []runtimeutil.StorageMount { diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index 82cb93965..26441ed51 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -58,11 +58,11 @@ func (l *Loader) LoadGenerators( for _, res := range rm.Resources() { g, err := l.LoadGenerator(ldr, v, res) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to load generator: %w", err) } generatorOrigin, err := resource.OriginFromCustomPlugin(res) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get origin from CustomPlugin: %w", err) } result = append(result, &resmap.GeneratorWithProperties{Generator: g, Origin: generatorOrigin}) } @@ -130,15 +130,19 @@ func (l *Loader) AbsolutePluginPath(id resid.ResId) (string, error) { // absPluginHome is the home of kustomize Exec and Go plugins. // Kustomize plugin configuration files are k8s-style objects // containing the fields 'apiVersion' and 'kind', e.g. -// apiVersion: apps/v1 -// kind: Deployment +// +// apiVersion: apps/v1 +// kind: Deployment +// // kustomize reads plugin configuration data from a file path // specified in the 'generators:' or 'transformers:' field of a // kustomization file. For Exec and Go plugins, kustomize // uses this data to both locate the plugin and configure it. // Each Exec or Go plugin (its code, its tests, its supporting data // files, etc.) must be housed in its own directory at -// ${absPluginHome}/${pluginApiVersion}/LOWERCASE(${pluginKind}) +// +// ${absPluginHome}/${pluginApiVersion}/LOWERCASE(${pluginKind}) +// // where // - ${absPluginHome} is an absolute path, defined below. // - ${pluginApiVersion} is taken from the plugin config file. @@ -226,7 +230,10 @@ func (l *Loader) makeBuiltinPlugin(r resid.Gvk) (resmap.Configurable, error) { } func (l *Loader) loadPlugin(res *resource.Resource) (resmap.Configurable, error) { - spec := fnplugin.GetFunctionSpec(res) + spec, err := fnplugin.GetFunctionSpec(res) + if err != nil { + return nil, fmt.Errorf("loader: %w", err) + } if spec != nil { // validation check that function mounts are under the current kustomization directory for _, mount := range spec.Container.StorageMounts { diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index 173d9e4b1..a4be95d55 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -737,7 +737,7 @@ generators: fSys, tmpDir.String()) assert.Error(t, err) - assert.Contains(t, err.Error(), "loading generator plugins: plugin RenderHelmChart."+ + assert.Contains(t, err.Error(), "loading generator plugins: failed to load generator: plugin RenderHelmChart."+ "v1alpha1.[noGrp]/demo.[noNs] with mount path '/tmp/dir' is not permitted; mount paths must"+ " be relative to the current kustomization directory") } @@ -770,7 +770,7 @@ generators: fSys, tmpDir.String()) assert.Error(t, err) - assert.Contains(t, err.Error(), "loading generator plugins: plugin RenderHelmChart."+ + assert.Contains(t, err.Error(), "loading generator plugins: failed to load generator: plugin RenderHelmChart."+ "v1alpha1.[noGrp]/demo.[noNs] with mount path './tmp/../../dir' is not permitted; mount paths must "+ "be under the current kustomization directory") } diff --git a/kyaml/fn/runtime/runtimeutil/functiontypes.go b/kyaml/fn/runtime/runtimeutil/functiontypes.go index 39cb24195..de3ee9049 100644 --- a/kyaml/fn/runtime/runtimeutil/functiontypes.go +++ b/kyaml/fn/runtime/runtimeutil/functiontypes.go @@ -10,6 +10,7 @@ import ( "strings" "sigs.k8s.io/kustomize/kyaml/yaml" + k8syaml "sigs.k8s.io/yaml" ) const ( @@ -200,50 +201,54 @@ func (s *StorageMount) String() string { // // The FunctionSpec is read from the resource metadata.annotation // "config.kubernetes.io/function" -func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { +func GetFunctionSpec(n *yaml.RNode) (*FunctionSpec, error) { meta, err := n.GetMeta() if err != nil { - return nil + return nil, nil } - if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { - return fn + + fn, err := getFunctionSpecFromAnnotation(n, meta) + if err != nil { + return nil, err + } + if fn != nil { + return fn, nil } // legacy function specification for backwards compatibility container := meta.Annotations["config.kubernetes.io/container"] if container != "" { - return &FunctionSpec{Container: ContainerSpec{Image: container}} + return &FunctionSpec{Container: ContainerSpec{Image: container}}, nil } - return nil + return nil, nil } // getFunctionSpecFromAnnotation parses the config function from an annotation // if it is found -func getFunctionSpecFromAnnotation(n *yaml.RNode, meta yaml.ResourceMeta) *FunctionSpec { +func getFunctionSpecFromAnnotation(n *yaml.RNode, meta yaml.ResourceMeta) (*FunctionSpec, error) { var fs FunctionSpec for _, s := range functionAnnotationKeys { fn := meta.Annotations[s] if fn != "" { - err := yaml.Unmarshal([]byte(fn), &fs) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) + if err := k8syaml.UnmarshalStrict([]byte(fn), &fs); err != nil { + return nil, fmt.Errorf("%s unmarshal error: %w", s, err) } - return &fs + return &fs, nil } } n, err := n.Pipe(yaml.Lookup("metadata", "configFn")) if err != nil || yaml.IsMissingOrNull(n) { - return nil + return nil, nil } s, err := n.String() if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) + fmt.Fprintf(os.Stderr, "configFn parse error: %v\n", err) + return nil, fmt.Errorf("configFn parse error: %w", err) } - err = yaml.Unmarshal([]byte(s), &fs) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) + if err := k8syaml.UnmarshalStrict([]byte(s), &fs); err != nil { + return nil, fmt.Errorf("%s unmarshal error: %w", "configFn", err) } - return &fs + return &fs, nil } func StringToStorageMount(s string) StorageMount { @@ -288,7 +293,11 @@ type IsReconcilerFilter struct { func (c *IsReconcilerFilter) Filter(inputs []*yaml.RNode) ([]*yaml.RNode, error) { var out []*yaml.RNode for i := range inputs { - isFnResource := GetFunctionSpec(inputs[i]) != nil + functionSpec, err := GetFunctionSpec(inputs[i]) + if err != nil { + return nil, err + } + isFnResource := functionSpec != nil if isFnResource && !c.ExcludeReconcilers { out = append(out, inputs[i]) } diff --git a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go index aff7c6239..3ed839d84 100644 --- a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go +++ b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go @@ -58,7 +58,7 @@ func TestFunctionFilter_Filter(t *testing.T) { // verify that resources emitted from the function have a file path defaulted // if none already exists { - name: "default_file_path_annotation", + name: "default file path annotation", run: testRun{ output: ` apiVersion: config.kubernetes.io/v1 @@ -99,7 +99,7 @@ metadata: // verify that resources emitted from the function do not have a file path defaulted // if one already exists { - name: "no_default_file_path_annotation", + name: "no default file path annotation", run: testRun{ output: ` apiVersion: config.kubernetes.io/v1 @@ -142,7 +142,7 @@ metadata: // verify the FunctionFilter correctly writes the inputs and reads the outputs // of Run { - name: "write_read", + name: "write read", run: testRun{ output: ` apiVersion: config.kubernetes.io/v1 @@ -200,7 +200,7 @@ metadata: // verify that the results file is written // { - name: "write_results_file", + name: "write results file", run: testRun{ output: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -277,7 +277,7 @@ metadata: // verify that the results file is written for functions that exist non-0 // and the FunctionFilter returns the error { - name: "write_results_file_function_exit_non_0", + name: "write results file function exit non 0", expectedError: "failed", run: testRun{ err: fmt.Errorf("failed"), @@ -356,7 +356,7 @@ metadata: // verify that if deferFailure is set, the results file is written and the // exit error is saved, but the FunctionFilter does not return an error. { - name: "write_results_defer_failure", + name: "write results defer failure", instance: FunctionFilter{DeferFailure: true}, expectedSavedError: "failed", run: testRun{ @@ -434,7 +434,7 @@ metadata: }, { - name: "write_results_bad_results_file", + name: "write results bad results file", expectedError: "open /not/real/file:", noMakeResultsFile: true, run: testRun{ @@ -484,7 +484,7 @@ metadata: // resources not provided to the function should still appear in the FunctionFilter // output { - name: "scope_resources_by_directory", + name: "scope resources by directory", run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -575,7 +575,7 @@ metadata: // verify functions without file path annotation are not scoped to functions { - name: "scope_resources_by_directory_resources_missing_path", + name: "scope resources by directory resources missing path", run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -662,7 +662,7 @@ metadata: // verify the functions can see all resources if global scope is set { - name: "scope_resources_global", + name: "scope resources global", instance: FunctionFilter{GlobalScope: true}, run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 @@ -767,7 +767,7 @@ metadata: }, { - name: "scope_no_resources", + name: "scope no resources", run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -837,7 +837,7 @@ metadata: }, { - name: "scope_functions_dir", + name: "scope functions dir", run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -928,7 +928,7 @@ metadata: }, { - name: "copy_comments", + name: "copy comments", run: testRun{ expectedInput: `apiVersion: config.kubernetes.io/v1 kind: ResourceList @@ -1242,7 +1242,7 @@ container: }, { - name: "path", + name: "path with uncorrect position", resource: ` apiVersion: v1beta1 kind: Example @@ -1253,15 +1253,11 @@ metadata: container: image: foo:v1.0.0 `, - // path should be erased - expectedFn: ` -container: - image: foo:v1.0.0 -`, + missingFn: true, }, { - name: "network", + name: "network with uncorrect position", resource: ` apiVersion: v1beta1 kind: Example @@ -1272,15 +1268,25 @@ metadata: container: image: foo:v1.0.0 `, - // network should be erased - expectedFn: ` -container: - image: foo:v1.0.0 + missingFn: true, + }, + { + name: "typo in function config", + resource: ` +apiVersion: v1beta1 +kind: Example +metadata: + annotations: + config.kubernetes.io/function: |- + containeer: + image: foo:v1.0.0 `, + missingFn: true, }, // legacy fn style - {name: "legacy fn meta", + { + name: "legacy fn meta", resource: ` apiVersion: v1beta1 kind: Example @@ -1296,7 +1302,8 @@ container: }, // no fn - {name: "no fn", + { + name: "no fn", resource: ` apiVersion: v1beta1 kind: Example @@ -1313,21 +1320,21 @@ metadata: tt := tests[i] t.Run(tt.name, func(t *testing.T) { resource := yaml.MustParse(tt.resource) - fn := GetFunctionSpec(resource) + fn, err := GetFunctionSpec(resource) if tt.missingFn { - if !assert.Nil(t, fn) { - t.FailNow() - } - } else { - b, err := yaml.Marshal(fn) - if !assert.NoError(t, err) { - t.FailNow() - } - if !assert.Equal(t, - strings.TrimSpace(tt.expectedFn), - strings.TrimSpace(string(b))) { + if err == nil && !assert.Nil(t, fn) { t.FailNow() } + return + } + b, err := yaml.Marshal(fn) + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.Equal(t, + strings.TrimSpace(tt.expectedFn), + strings.TrimSpace(string(b))) { + t.FailNow() } }) } @@ -1392,9 +1399,12 @@ metadata: for _, tc := range tests { cfg, err := yaml.Parse(tc.input) if !assert.NoError(t, err) { - return + t.FailNow() + } + fn, err := GetFunctionSpec(cfg) + if !assert.NoError(t, err) { + t.FailNow() } - fn := GetFunctionSpec(cfg) assert.Equal(t, tc.required, fn.Container.Network) } } @@ -1530,9 +1540,12 @@ metadata: for _, tc := range tests { cfg, err := yaml.Parse(tc.input) if !assert.NoError(t, err) { - return + t.FailNow() + } + fn, err := GetFunctionSpec(cfg) + if !assert.NoError(t, err) { + t.FailNow() } - fn := GetFunctionSpec(cfg) assert.Equal(t, tc.expected, *NewContainerEnvFromStringSlice(fn.Container.Env)) } } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index c855144d7..6b36d7d3a 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -308,7 +308,10 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) ( var fltrs []kio.Filter for i := range fns { api := fns[i] - spec := runtimeutil.GetFunctionSpec(api) + spec, err := runtimeutil.GetFunctionSpec(api) + if err != nil { + return nil, fmt.Errorf("failed to get FunctionSpec: %w", err) + } if spec == nil { // resource doesn't have function spec continue diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index ebdb56984..0ea0779df 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -314,6 +314,24 @@ metadata: }, out: []string{"gcr.io/example.com/image:v1.0.0 deferFailure: true"}, }, + { + name: "parse_failure", + in: []f{ + { + path: filepath.Join("foo", "bar.yaml"), + value: ` +apiVersion: example.com/v1alpha1 +kind: ExampleFunction +metadata: + annotations: + config.kubernetes.io/function: | + containeeer: + image: gcr.io/example.com/image:v1.0.0 +`, + }, + }, + error: "config.kubernetes.io/function unmarshal error: error unmarshaling JSON: while decoding JSON: json: unknown field \"containeeer\"", + }, {name: "disable containers", in: []f{ @@ -807,7 +825,10 @@ metadata: var images []string for _, n := range packageBuff.Nodes { - spec := runtimeutil.GetFunctionSpec(n) + spec, err := runtimeutil.GetFunctionSpec(n) + if !assert.NoError(t, err) { + t.FailNow() + } images = append(images, spec.Container.Image) }