From bcae65770a33bdadd8851d58b6817c6fef0ac02d Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Thu, 21 Apr 2022 18:28:13 -0400 Subject: [PATCH 1/3] Configurable extensions for template parser --- kyaml/fn/framework/parser/parser.go | 21 +++++++++++++++------ kyaml/fn/framework/parser/schema.go | 2 +- kyaml/fn/framework/parser/schema_test.go | 2 +- kyaml/fn/framework/parser/template.go | 8 +++++++- kyaml/fn/framework/parser/template_test.go | 19 ++++++++++++++++++- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/kyaml/fn/framework/parser/parser.go b/kyaml/fn/framework/parser/parser.go index 748964486e..a5bc7729c9 100644 --- a/kyaml/fn/framework/parser/parser.go +++ b/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. diff --git a/kyaml/fn/framework/parser/schema.go b/kyaml/fn/framework/parser/schema.go index 2160178a9b..d2c12840bd 100644 --- a/kyaml/fn/framework/parser/schema.go +++ b/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. diff --git a/kyaml/fn/framework/parser/schema_test.go b/kyaml/fn/framework/parser/schema_test.go index a51801a950..9d78cd4c79 100644 --- a/kyaml/fn/framework/parser/schema_test.go +++ b/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 { diff --git a/kyaml/fn/framework/parser/template.go b/kyaml/fn/framework/parser/template.go index 44a6db436c..b98381bd33 100644 --- a/kyaml/fn/framework/parser/template.go +++ b/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 +} diff --git a/kyaml/fn/framework/parser/template_test.go b/kyaml/fn/framework/parser/template_test.go index 1b0e14605b..bb0d60f076 100644 --- a/kyaml/fn/framework/parser/template_test.go +++ b/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 From 2e230b4d4b5df37cb5bd7a62c6372bdcafa894d5 Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Fri, 22 Apr 2022 17:19:49 -0400 Subject: [PATCH 2/3] Regression test and fix for APIs that satisfy ValidationSchemaProvider but not Validator --- kyaml/fn/framework/processors.go | 2 +- kyaml/fn/framework/processors_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/kyaml/fn/framework/processors.go b/kyaml/fn/framework/processors.go index 6dfc9a98e7..ed4367b379 100644 --- a/kyaml/fn/framework/processors.go +++ b/kyaml/fn/framework/processors.go @@ -173,7 +173,7 @@ func LoadFunctionConfig(src *yaml.RNode, api interface{}) error { if v, ok := api.(Validator); ok { return combineErrors(schemaValidationError, v.Validate()) } - return nil + return schemaValidationError } func combineErrors(schemaErr, customErr error) error { diff --git a/kyaml/fn/framework/processors_test.go b/kyaml/fn/framework/processors_test.go index da2964380a..cbf83837de 100644 --- a/kyaml/fn/framework/processors_test.go +++ b/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) } From 7a773a3a48eb59f540697c1fbe11c434fc7c9fbf Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Mon, 9 May 2022 16:40:48 -0400 Subject: [PATCH 3/3] Configure wrapcheck linter to recognize wrapping done by kyaml's errors package --- .golangci.yml | 13 +++++++++++++ kyaml/fn/framework/processors.go | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4b008d16d6..7a8af03765 100644 --- a/.golangci.yml +++ b/.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 diff --git a/kyaml/fn/framework/processors.go b/kyaml/fn/framework/processors.go index ed4367b379..7d2a9b4c9c 100644 --- a/kyaml/fn/framework/processors.go +++ b/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,7 +166,7 @@ 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) } }