Merge pull request #4629 from KnVerey/configure_templateparser

fn framework: bugfix for schema providers + configurable template extensions

.golangci.yml

@@ -110,6 +110,19 @@ linters-settings:
   gomnd:
      ignored-functions:
      - ioutil.WriteFile
+  wrapcheck:
+    ignoreSigs:
+      # defaults
+      - .Errorf(
+      - errors.New(
+      - errors.Unwrap(
+      - .Wrap(
+      - .Wrapf(
+      - .WithMessage(
+      - .WithMessagef(
+      - .WithStack(
+      # from kyaml's errors package
+      - .WrapPrefixf(
 
 issues:
   new-from-rev: c94b5d8f2 # enables us to enforce a larger set of linters for new code than pass on existing code

kyaml/fn/framework/parser/parser.go

@@ -13,9 +13,9 @@ import (
 )
 
 type parser struct {
-	fs        iofs.FS
-	paths     []string
-	extension string
+	fs         iofs.FS
+	paths      []string
+	extensions []string
 }
 
 type contentProcessor func(content []byte, name string) error
@@ -51,8 +51,8 @@ func (l parser) readPath(path string, processContent contentProcessor) error {
 	}
 
 	// Path is a file -- check extension and read it
-	if !strings.HasSuffix(path, l.extension) {
-		return errors.Errorf("file %s did not have required extension %s", path, l.extension)
+	if err := checkExtension(path, l.extensions); err != nil {
+		return err
 	}
 	b, err := ioutil.ReadAll(f)
 	if err != nil {
@@ -61,6 +61,15 @@ func (l parser) readPath(path string, processContent contentProcessor) error {
 	return processContent(b, path)
 }
 
+func checkExtension(path string, extensions []string) error {
+	for _, ext := range extensions {
+		if strings.HasSuffix(path, ext) {
+			return nil
+		}
+	}
+	return errors.Errorf("file %s does not have any of permitted extensions %s", path, extensions)
+}
+
 func (l parser) readDir(dir iofs.ReadDirFile, dirname string, processContent contentProcessor) error {
 	entries, err := dir.ReadDir(0)
 	if err != nil {
@@ -68,7 +77,7 @@ func (l parser) readDir(dir iofs.ReadDirFile, dirname string, processContent con
 	}
 
 	for _, entry := range entries {
-		if entry.IsDir() || !strings.HasSuffix(entry.Name(), l.extension) {
+		if err := checkExtension(entry.Name(), l.extensions); err != nil || entry.IsDir() {
 			continue
 		}
 		// Note: using filepath.Join will break Windows, because io/fs.FS implementations require slashes on all OS.

kyaml/fn/framework/parser/schema.go

@@ -56,7 +56,7 @@ func SchemaStrings(data ...string) framework.SchemaParser {
 //		AdditionalSchemas: parser.SchemaFiles("path/to/crd-schemas", "path/to/special-schema.json),
 //	}
 func SchemaFiles(paths ...string) SchemaParser {
-	return SchemaParser{parser{paths: paths, extension: SchemaExtension}}
+	return SchemaParser{parser{paths: paths, extensions: []string{SchemaExtension}}}
 }
 
 // SchemaParser is a framework.SchemaParser that can parse files or directories containing openapi schemas.

kyaml/fn/framework/parser/schema_test.go

@@ -52,7 +52,7 @@ func TestSchemaFiles(t *testing.T) {
 		{
 			name:    "rejects non-.template.yaml files",
 			paths:   []string{"testdata/ignore.yaml"},
-			wantErr: "file testdata/ignore.yaml did not have required extension .json",
+			wantErr: "file testdata/ignore.yaml does not have any of permitted extensions [.json]",
 		},
 	}
 	for _, tt := range tests {

kyaml/fn/framework/parser/template.go

@@ -61,7 +61,7 @@ func TemplateStrings(data ...string) framework.TemplateParser {
 //		}},
 //   }
 func TemplateFiles(paths ...string) TemplateParser {
-	return TemplateParser{parser{paths: paths, extension: TemplateExtension}}
+	return TemplateParser{parser{paths: paths, extensions: []string{TemplateExtension}}}
 }
 
 // TemplateParser is a framework.TemplateParser that can parse files or directories containing Go templated YAML.
@@ -95,3 +95,9 @@ func (l TemplateParser) FromFS(fs iofs.FS) TemplateParser {
 	l.parser.fs = fs
 	return l
 }
+
+// WithExtensions allows you to replace the extension the parser accept on the input files.
+func (l TemplateParser) WithExtensions(ext ...string) TemplateParser {
+	l.parser.extensions = ext
+	return l
+}

kyaml/fn/framework/parser/template_test.go

@@ -81,7 +81,7 @@ func TestTemplateFiles(t *testing.T) {
 		{
 			name:    "rejects non-.template.yaml files",
 			paths:   []string{"testdata/ignore.yaml"},
-			wantErr: "file testdata/ignore.yaml did not have required extension .template.yaml",
+			wantErr: "file testdata/ignore.yaml does not have any of permitted extensions [.template.yaml]",
 		},
 	}
 	for _, tt := range tests {
@@ -114,6 +114,23 @@ func TestTemplateFiles(t *testing.T) {
 	}
 }
 
+func TestTemplateParser_WithExtensions(t *testing.T) {
+	p := parser.TemplateFiles("testdata").WithExtensions(".json")
+	templates, err := p.Parse()
+	require.NoError(t, err)
+	require.Len(t, templates, 2)
+
+	p = p.WithExtensions(".yaml")
+	templates, err = p.Parse()
+	require.NoError(t, err)
+	require.Len(t, templates, 3)
+
+	p = p.WithExtensions(".yaml", ".json")
+	templates, err = p.Parse()
+	require.NoError(t, err)
+	require.Len(t, templates, 5)
+}
+
 func TestTemplateStrings(t *testing.T) {
 	tests := []struct {
 		name     string

kyaml/fn/framework/processors.go

@@ -150,7 +150,7 @@ func LoadFunctionConfig(src *yaml.RNode, api interface{}) error {
 		if err != nil {
 			return errors.WrapPrefixf(err, "loading provided schema")
 		}
-		schemaValidationError = validate.AgainstSchema(schema, src, strfmt.Default)
+		schemaValidationError = errors.Wrap(validate.AgainstSchema(schema, src, strfmt.Default))
 		// don't return it yet--try to make it to custom validation stage to combine errors
 	}
 
@@ -166,14 +166,14 @@ func LoadFunctionConfig(src *yaml.RNode, api interface{}) error {
 
 	if d, ok := api.(Defaulter); ok {
 		if err := d.Default(); err != nil {
-			return err
+			return errors.Wrap(err)
 		}
 	}
 
 	if v, ok := api.(Validator); ok {
 		return combineErrors(schemaValidationError, v.Validate())
 	}
-	return nil
+	return schemaValidationError
 }
 
 func combineErrors(schemaErr, customErr error) error {

kyaml/fn/framework/processors_test.go

@@ -17,6 +17,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/fn/framework/frameworktestutil"
 	"sigs.k8s.io/kustomize/kyaml/fn/framework/parser"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
+	"sigs.k8s.io/kustomize/kyaml/resid"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 
 	"sigs.k8s.io/kustomize/kyaml/fn/framework"
@@ -700,6 +701,16 @@ func (e errorMergeTest) Validate() error {
 	return nil
 }
 
+func (a schemaProviderOnlyTest) Schema() (*spec.Schema, error) {
+	schema, err := framework.SchemaFromFunctionDefinition(resid.NewGvk("example.com", "v1alpha1", "JavaSpringBoot"), javaSpringBootDefinition)
+	return schema, errors.WrapPrefixf(err, "parsing JavaSpringBoot schema")
+}
+
+type schemaProviderOnlyTest struct {
+	Metadata Metadata                   `yaml:"metadata" json:"metadata"`
+	Spec     v1alpha1JavaSpringBootSpec `yaml:"spec" json:"spec"`
+}
+
 func TestLoadFunctionConfig(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -755,6 +766,19 @@ spec:
 			api: &errorMergeTest{},
 			wantErrMsgs: []string{
 				`validation failure list:
+spec.replicas in body should be less than or equal to 9`,
+			},
+		}, {
+			name: "schema provider only",
+			src: yaml.MustParse(`
+apiVersion: example.com/v1alpha1
+kind: JavaSpringBoot
+spec:
+  replicas: 11
+`),
+			api: &schemaProviderOnlyTest{},
+			wantErrMsgs: []string{
+				`validation failure list:
 spec.replicas in body should be less than or equal to 9`,
 			},
 		},
@@ -813,6 +837,7 @@ test: true
 				require.NoError(t, err)
 				require.Equal(t, tt.want, tt.api)
 			} else {
+				require.Error(t, err)
 				for _, msg := range tt.wantErrMsgs {
 					require.Contains(t, err.Error(), msg)
 				}