From 94f307fb2ac362d5f829a781e133a056eff1032c Mon Sep 17 00:00:00 2001 From: hasheddan Date: Sat, 3 Oct 2020 09:56:13 -0500 Subject: [PATCH 1/2] Rename DefaultParser to PackageParser DefaultParser is not descriptive of the Parser implementation's purpose. PackageParser indicates the this parser is meant for parsing packages. Signed-off-by: hasheddan --- pkg/parser/parser.go | 12 ++++++------ pkg/parser/parser_test.go | 17 +++++++++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index a1b4551a8..ea9b36097 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -63,15 +63,15 @@ type Parser interface { Parse(context.Context, io.ReadCloser) (*Package, error) } -// DefaultParser is the default Parser implementation. -type DefaultParser struct { +// PackageParser is a Parser implementation for parsing packages. +type PackageParser struct { metaScheme ObjectCreaterTyper objScheme ObjectCreaterTyper } -// New returns a new DefaultParser. -func New(meta, obj ObjectCreaterTyper) *DefaultParser { - return &DefaultParser{ +// New returns a new PackageParser. +func New(meta, obj ObjectCreaterTyper) *PackageParser { + return &PackageParser{ metaScheme: meta, objScheme: obj, } @@ -81,7 +81,7 @@ func New(meta, obj ObjectCreaterTyper) *DefaultParser { // decode objects recognized by the meta scheme, then attempts to decode objects // recognized by the object scheme. Objects not recognized by either scheme // return an error rather than being skipped. -func (p *DefaultParser) Parse(ctx context.Context, reader io.ReadCloser) (*Package, error) { //nolint:gocyclo +func (p *PackageParser) Parse(ctx context.Context, reader io.ReadCloser) (*Package, error) { //nolint:gocyclo pkg := NewPackage() if reader == nil { return pkg, nil diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go index 055df2290..50bc363b2 100644 --- a/pkg/parser/parser_test.go +++ b/pkg/parser/parser_test.go @@ -30,21 +30,26 @@ import ( "sigs.k8s.io/yaml" ) -var crdBytes = []byte(`apiVersion: apiextensions.k8s.io/v1beta1 +var _ Parser = &PackageParser{} + +var ( + crdBytes = []byte(`apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: name: test`) -var deployBytes = []byte(`apiVersion: apps/v1 + deployBytes = []byte(`apiVersion: apps/v1 kind: Deployment metadata: name: test`) + crd = &apiextensions.CustomResourceDefinition{} + _ = yaml.Unmarshal(crdBytes, crd) + deploy = &appsv1.Deployment{} + _ = yaml.Unmarshal(deployBytes, deploy) +) + func TestParser(t *testing.T) { - crd := &apiextensions.CustomResourceDefinition{} - _ = yaml.Unmarshal(crdBytes, crd) - deploy := &appsv1.Deployment{} - _ = yaml.Unmarshal(deployBytes, deploy) allBytes := bytes.Join([][]byte{crdBytes, deployBytes}, []byte("\n---\n")) fs := afero.NewMemMapFs() _ = afero.WriteFile(fs, "crd.yaml", crdBytes, 0o644) From 9918fd79070797066be15163ffb26f623860b7ea Mon Sep 17 00:00:00 2001 From: hasheddan Date: Sat, 3 Oct 2020 09:57:37 -0500 Subject: [PATCH 2/2] Move common linter functionality to parser pkg Downstream consumers may implement their own linter functions, but the core linter loop can be generalized, as well as helper functions such as Or. Signed-off-by: hasheddan --- pkg/parser/linter.go | 109 ++++++++++++++++++++++ pkg/parser/linter_test.go | 184 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+) create mode 100644 pkg/parser/linter.go create mode 100644 pkg/parser/linter_test.go diff --git a/pkg/parser/linter.go b/pkg/parser/linter.go new file mode 100644 index 000000000..ea2c27c0c --- /dev/null +++ b/pkg/parser/linter.go @@ -0,0 +1,109 @@ +/* +Copyright 2020 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parser + +import ( + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" +) + +const ( + errNilLinterFn = "linter function is nil" + + errOrFmt = "object did not pass either check: (%v), (%v)" +) + +// A Linter lints packages. +type Linter interface { + Lint(*Package) error +} + +// PackageLinterFn lints an entire package. If function applies a check for +// multiple objects, consider using an ObjectLinterFn. +type PackageLinterFn func(*Package) error + +// PackageLinterFns is a convenience function to pass multiple PackageLinterFn +// to a function that cannot accept variadic arguments. +func PackageLinterFns(fns ...PackageLinterFn) []PackageLinterFn { + return fns +} + +// ObjectLinterFn lints an object in a package. +type ObjectLinterFn func(runtime.Object) error + +// ObjectLinterFns is a convenience function to pass multiple ObjectLinterFn to +// a function that cannot accept variadic arguments. +func ObjectLinterFns(fns ...ObjectLinterFn) []ObjectLinterFn { + return fns +} + +// PackageLinter lints packages by applying package and object linter functions +// to it. +type PackageLinter struct { + pre []PackageLinterFn + perMeta []ObjectLinterFn + perObject []ObjectLinterFn +} + +// NewPackageLinter creates a new PackageLinter. +func NewPackageLinter(pre []PackageLinterFn, perMeta, perObject []ObjectLinterFn) *PackageLinter { + return &PackageLinter{ + pre: pre, + perMeta: perMeta, + perObject: perObject, + } +} + +// Lint executes all linter functions against a package. +func (l *PackageLinter) Lint(pkg *Package) error { + for _, fn := range l.pre { + if err := fn(pkg); err != nil { + return err + } + } + for _, o := range pkg.GetMeta() { + for _, fn := range l.perMeta { + if err := fn(o); err != nil { + return err + } + } + } + for _, o := range pkg.GetObjects() { + for _, fn := range l.perObject { + if err := fn(o); err != nil { + return err + } + } + } + return nil +} + +// Or checks that at least one of the passed linter functions does not return an +// error. +func Or(a, b ObjectLinterFn) ObjectLinterFn { + return func(o runtime.Object) error { + if a == nil || b == nil { + return errors.New(errNilLinterFn) + } + aErr := a(o) + bErr := b(o) + if aErr == nil || bErr == nil { + return nil + } + return errors.Errorf(errOrFmt, aErr, bErr) + } +} diff --git a/pkg/parser/linter_test.go b/pkg/parser/linter_test.go new file mode 100644 index 000000000..d929b4dc0 --- /dev/null +++ b/pkg/parser/linter_test.go @@ -0,0 +1,184 @@ +/* +Copyright 2020 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parser + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/crossplane/crossplane-runtime/pkg/test" +) + +var _ Linter = &PackageLinter{} + +var ( + errBoom = errors.New("boom") + + pkgPass = func(pkg *Package) error { + return nil + } + pkgFail = func(pkg *Package) error { + return errBoom + } + objPass = func(o runtime.Object) error { + return nil + } + objFail = func(o runtime.Object) error { + return errBoom + } +) + +func TestLinter(t *testing.T) { + type args struct { + linter Linter + pkg *Package + } + + cases := map[string]struct { + reason string + args args + err error + }{ + "SuccessfulNoOp": { + reason: "Passing no checks should always be successful.", + args: args{ + linter: NewPackageLinter(nil, nil, nil), + pkg: NewPackage(), + }, + }, + "SuccessfulNoObjects": { + reason: "Passing object linters on empty package should always be successful.", + args: args{ + linter: NewPackageLinter(nil, ObjectLinterFns(objFail), ObjectLinterFns(objFail)), + // Object linters do not run if a package has no objects. + pkg: NewPackage(), + }, + }, + "SuccessfulWithChecks": { + reason: "Passing checks for a valid package should always be successful.", + args: args{ + linter: NewPackageLinter(PackageLinterFns(pkgPass), ObjectLinterFns(objPass), ObjectLinterFns(Or(objPass, objFail))), + pkg: &Package{ + meta: []runtime.Object{deploy}, + objects: []runtime.Object{crd}, + }, + }, + }, + "ErrorPackageLint": { + reason: "Passing package linters for an invalid package should always fail.", + args: args{ + linter: NewPackageLinter(PackageLinterFns(pkgFail), ObjectLinterFns(objPass), ObjectLinterFns(objPass)), + pkg: &Package{ + meta: []runtime.Object{deploy}, + objects: []runtime.Object{crd}, + }, + }, + err: errBoom, + }, + "ErrorMetaLint": { + reason: "Passing meta linters for a package with invalid meta should always fail.", + args: args{ + linter: NewPackageLinter(PackageLinterFns(pkgPass), ObjectLinterFns(objFail), ObjectLinterFns(objPass)), + pkg: &Package{ + meta: []runtime.Object{deploy}, + objects: []runtime.Object{crd}, + }, + }, + err: errBoom, + }, + "ErrorObjectLint": { + reason: "Passing object linters for a package with invalid objects should always fail.", + args: args{ + linter: NewPackageLinter(PackageLinterFns(pkgPass), ObjectLinterFns(objPass), ObjectLinterFns(Or(objFail, objFail))), + pkg: &Package{ + meta: []runtime.Object{deploy}, + objects: []runtime.Object{crd}, + }, + }, + err: errors.Errorf(errOrFmt, errBoom, errBoom), + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := tc.args.linter.Lint(tc.args.pkg) + + if diff := cmp.Diff(tc.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nl.Lint(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +} + +var _ ObjectLinterFn = Or(nil, nil) + +func TestOr(t *testing.T) { + type args struct { + one ObjectLinterFn + two ObjectLinterFn + } + + cases := map[string]struct { + reason string + args args + err error + }{ + "SuccessfulBothPass": { + reason: "Passing two successful linters should never return error.", + args: args{ + one: objPass, + two: objPass, + }, + }, + "SuccessfulOnePass": { + reason: "Passing one successful linters should never return error.", + args: args{ + one: objPass, + two: objFail, + }, + }, + "ErrNeitherPass": { + reason: "Passing two unsuccessful linters should always return error.", + args: args{ + one: objFail, + two: objFail, + }, + err: errors.Errorf(errOrFmt, errBoom, errBoom), + }, + "ErrNilLinter": { + reason: "Passing a nil linter will should always return error.", + args: args{ + one: nil, + two: objPass, + }, + err: errors.New(errNilLinterFn), + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := Or(tc.args.one, tc.args.two)(crd) + + if diff := cmp.Diff(tc.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nOr(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +}