Merge pull request #4758 from koba1t/feature/emit_a_warning_to_ignore_plugin

Emit a warning when using missing field in FunctionSpec

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 {

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 {

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")
 }

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])
 		}

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))
 	}
 }

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

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)
 			}