Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fn framework: bugfix for schema providers + configurable template extensions #4629

Merged
merged 3 commits into from May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions .golangci.yml
Expand Up @@ -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
Expand Down
21 changes: 15 additions & 6 deletions kyaml/fn/framework/parser/parser.go
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -61,14 +61,23 @@ 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 {
return err
}

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.
Expand Down
2 changes: 1 addition & 1 deletion kyaml/fn/framework/parser/schema.go
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion kyaml/fn/framework/parser/schema_test.go
Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion kyaml/fn/framework/parser/template.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
19 changes: 18 additions & 1 deletion kyaml/fn/framework/parser/template_test.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions kyaml/fn/framework/processors.go
Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down
25 changes: 25 additions & 0 deletions kyaml/fn/framework/processors_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`,
},
},
Expand Down Expand Up @@ -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)
}
Expand Down