From 57b5fe56ce19b10da3805a3f01ccace6fecf35bf Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 30 Sep 2021 16:54:25 +0100 Subject: [PATCH 1/4] Add crd-linter tool --- cmd/crd-linter/README.md | 51 ++++++++++++ cmd/crd-linter/evaluator/evaluate.go | 60 ++++++++++++++ cmd/crd-linter/evaluator/types.go | 93 +++++++++++++++++++++ cmd/crd-linter/exceptions/exceptions.go | 106 ++++++++++++++++++++++++ cmd/crd-linter/linters/interface.go | 32 +++++++ cmd/crd-linter/linters/linters.go | 67 +++++++++++++++ cmd/crd-linter/linters/util.go | 62 ++++++++++++++ cmd/crd-linter/main.go | 98 ++++++++++++++++++++++ cmd/crd-linter/reader/directory.go | 93 +++++++++++++++++++++ cmd/crd-linter/reader/scheme.go | 60 ++++++++++++++ cmd/crd-linter/reader/scheme_test.go | 105 +++++++++++++++++++++++ 11 files changed, 827 insertions(+) create mode 100644 cmd/crd-linter/README.md create mode 100644 cmd/crd-linter/evaluator/evaluate.go create mode 100644 cmd/crd-linter/evaluator/types.go create mode 100644 cmd/crd-linter/exceptions/exceptions.go create mode 100644 cmd/crd-linter/linters/interface.go create mode 100644 cmd/crd-linter/linters/linters.go create mode 100644 cmd/crd-linter/linters/util.go create mode 100644 cmd/crd-linter/main.go create mode 100644 cmd/crd-linter/reader/directory.go create mode 100644 cmd/crd-linter/reader/scheme.go create mode 100644 cmd/crd-linter/reader/scheme_test.go diff --git a/cmd/crd-linter/README.md b/cmd/crd-linter/README.md new file mode 100644 index 000000000..e5ccaee54 --- /dev/null +++ b/cmd/crd-linter/README.md @@ -0,0 +1,51 @@ +# Linter for Kubernetes CustomResourceDefinition resources + +The crd-linter encompasses a number of linters that can be run against +Kubernetes CustomResourceDefinition (CRD) objects to help ensure best-practices +are followed when CRDs are authored. + +## Usage + +``` +$ crd-linter --help +Usage of crd-linter: + --exceptions-file string Path to a list of exceptions for linter failures + --linters strings Full list of linters to run against discovered CustomResourceDefinitions (default [SchemaProvided,PreserveUnknownFields,MaxLengthStrings,MaxItemsArrays]) + --output-exceptions If true, an exception list file will be written to the file denoted by '--exceptions-file' + --path string Path to recursively search for CustomResourceDefinition objects +``` + +The crd-linter must be pointed at a directory containing YAML files with CRD +object(s) defined in them using the `--path` flag. + +For example: + +``` +crd-linter --path ./path/to/crds/ +``` + +### Linters + +The following linters are supported: + +| Name | Description | +|-----------------------|---------------------------------------------------------------------| +| MaxLengthStrings | Ensures that all 'string' type fields have a `maxLength` option set | +| MaxItemsArrays | Ensures that all 'array' type fields have a `maxItems` option set | +| SchemaProvided | Ensures that all versions of all CRDs provide an OpenAPI schema | +| PreserveUnknownFields | Ensures that no fields within a CRD schema permit unknown fields | + +### Exceptions + +An exceptions file may be provided which will permit certain rule violations to +be skipped. This is useful when gradually improving/fixing issues within a +repository of CRDs, whilst not permitting new violations. + +You can generate an exceptions file by setting `--output-exceptions=true` +whilst also providing a `--exceptions-file`. +This will output all linter failures to the named exceptions file, so that +future runs of the linter will skip these failures. + +Use this in combination with the `--linters` flag to generate an exceptions +file that only contains a subset of linter failures (e.g. only the nosiest or +least problematic failures). diff --git a/cmd/crd-linter/evaluator/evaluate.go b/cmd/crd-linter/evaluator/evaluate.go new file mode 100644 index 000000000..f34fac82b --- /dev/null +++ b/cmd/crd-linter/evaluator/evaluate.go @@ -0,0 +1,60 @@ +/* +Copyright 2021 The Kubernetes 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 evaluator + +import ( + "sigs.k8s.io/controller-tools/cmd/crd-linter/exceptions" + "sigs.k8s.io/controller-tools/cmd/crd-linter/linters" + "sigs.k8s.io/controller-tools/cmd/crd-linter/reader" +) + +func Evaluate(list linters.LinterList, eval []reader.CRDToEvaluate, excepted *exceptions.ExceptionList) ResultsList { + results := make([]Results, len(eval)) + for i, e := range eval { + results[i] = Results{ + Evaluated: e, + } + for _, l := range list { + errs := l.Execute(e.CustomResourceDefinition) + if len(errs) == 0 { + continue + } + + // If no exceptions are provided, skip the filtering logic + if excepted == nil { + results[i].Violations = append(results[i].Violations, ViolationList{ + Linter: l, + Violations: errs, + }) + continue + } + + // Check results against the exception list + var filteredErrs []string + for _, err := range errs { + if !excepted.IsExcepted(e.OriginalFilename, e.CustomResourceDefinition.Name, l.Name(), err) { + filteredErrs = append(filteredErrs, err) + } + } + results[i].Violations = append(results[i].Violations, ViolationList{ + Linter: l, + Violations: filteredErrs, + }) + } + } + return results +} diff --git a/cmd/crd-linter/evaluator/types.go b/cmd/crd-linter/evaluator/types.go new file mode 100644 index 000000000..33b3b28cd --- /dev/null +++ b/cmd/crd-linter/evaluator/types.go @@ -0,0 +1,93 @@ +/* +Copyright 2021 The Kubernetes 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 evaluator + +import ( + "sort" + + "sigs.k8s.io/controller-tools/cmd/crd-linter/exceptions" + "sigs.k8s.io/controller-tools/cmd/crd-linter/linters" + "sigs.k8s.io/controller-tools/cmd/crd-linter/reader" +) + +type Results struct { + Evaluated reader.CRDToEvaluate + + Violations []ViolationList +} + +func (r *Results) HasViolations() bool { + if len(r.Violations) == 0 { + return false + } + for _, v := range r.Violations { + if len(v.Violations) > 0 { + return true + } + } + return false +} + +func (r *Results) ViolatedLinters() []ViolationList { + var list []ViolationList + for _, v := range r.Violations { + if len(v.Violations) == 0 { + continue + } + + list = append(list, v) + } + return list +} + +type ResultsList []Results + +func (results ResultsList) ToExceptionList() *exceptions.ExceptionList { + list := exceptions.New() + for _, r := range results { + if !r.HasViolations() { + continue + } + + for _, l := range r.Violations { + for _, v := range l.Violations { + list.Add(r.Evaluated.OriginalFilename, r.Evaluated.CustomResourceDefinition.Name, l.Linter.Name(), v) + } + } + } + sort.SliceStable(list.Exceptions, func(i, j int) bool { + return list.Exceptions[i].String() < list.Exceptions[j].String() + }) + return list +} + +func (results ResultsList) HasViolations() bool { + for _, r := range results { + if r.HasViolations() { + return true + } + } + return false +} + +type ViolationList struct { + // The Linter that triggered this set of violations + Linter linters.Linter + + // An ordered list of violations triggered by this linter + Violations []string +} diff --git a/cmd/crd-linter/exceptions/exceptions.go b/cmd/crd-linter/exceptions/exceptions.go new file mode 100644 index 000000000..ea666493f --- /dev/null +++ b/cmd/crd-linter/exceptions/exceptions.go @@ -0,0 +1,106 @@ +/* +Copyright 2021 The Kubernetes 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 exceptions + +import ( + "fmt" + "io/ioutil" + "strings" +) + +type ExceptionList struct { + Exceptions []Exception +} + +type Exception struct { + filename, crdName, linter, fieldRef string +} + +func (e Exception) String() string { + return fmt.Sprintf("%s|%s|%s|%s", e.filename, e.crdName, e.linter, e.fieldRef) +} + +func (l *ExceptionList) IsExcepted(filename, crdName, linter, fieldRef string) bool { + toFind := Exception{ + filename: filename, + crdName: crdName, + linter: linter, + fieldRef: fieldRef, + } + for _, e := range l.Exceptions { + if e == toFind { + return true + } + } + return false +} + +func (l *ExceptionList) Add(filename, crdName, linter, fieldRef string) { + if l.IsExcepted(filename, crdName, linter, fieldRef) { + return + } + l.Exceptions = append(l.Exceptions, Exception{ + filename: filename, + crdName: crdName, + linter: linter, + fieldRef: fieldRef, + }) +} + +func (l *ExceptionList) String() string { + buf := &strings.Builder{} + for _, e := range l.Exceptions { + buf.WriteString(fmt.Sprintf("%s\n", e.String())) + } + return buf.String() +} + +func (l *ExceptionList) Size() int { + return len(l.Exceptions) +} + +func (l *ExceptionList) WriteToFile(path string) error { + return ioutil.WriteFile(path, []byte(l.String()), 0644) +} + +func New() *ExceptionList { + return &ExceptionList{Exceptions: make([]Exception, 0)} +} + +// LoadFromFile will load a pipe (|) separated list of Exceptions from the +// given file. +func LoadFromFile(path string) (*ExceptionList, error) { + data, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + + list := New() + entries := strings.Split(string(data), "\n") + for lineNo, e := range entries { + // skip empty lines + if len(e) == 0 { + continue + } + e := strings.Split(e, "|") + if len(e) != 4 { + return nil, fmt.Errorf("invalid Exception entry at line %d", lineNo+1) + } + list.Add(e[0], e[1], e[2], e[3]) + } + return list, nil +} diff --git a/cmd/crd-linter/linters/interface.go b/cmd/crd-linter/linters/interface.go new file mode 100644 index 000000000..cec1eabcf --- /dev/null +++ b/cmd/crd-linter/linters/interface.go @@ -0,0 +1,32 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + +// A Linter evaluates a CRD resource and provides information about policy violations. +type Linter interface { + // Name returns the name of this linter, used during report output + Name() string + + // Execute runs the linter against the given custom resource definition + Execute(*v1.CustomResourceDefinition) []string + + // Description prints human-readable, actionable information and references about + // what the linter does. + Description() string +} diff --git a/cmd/crd-linter/linters/linters.go b/cmd/crd-linter/linters/linters.go new file mode 100644 index 000000000..0e2c4cacc --- /dev/null +++ b/cmd/crd-linter/linters/linters.go @@ -0,0 +1,67 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" +) + +var allLinters = []Linter{ + // Add new linters to this list +} + +var namedLinters = make(map[string]Linter) + +func init() { + for _, linter := range allLinters { + namedLinters[linter.Name()] = linter + } +} + +// All returns all registered linters +func All() LinterList { + linters := make([]Linter, len(allLinters)) + copy(linters, allLinters) + return linters +} + +// Named returns a list of Linters with the given names, or an error if any +// names passed to it are not recognised +func Named(names []string) (LinterList, error) { + var linters []Linter + for _, n := range names { + if l, ok := namedLinters[n]; ok { + linters = append(linters, l) + continue + } + + return nil, fmt.Errorf("unrecognised linter: %s", n) + } + return linters, nil +} + +// LinterList is a list of linters, used to provide methods that work across a whole list +type LinterList []Linter + +// Names calls Name() on each Linter in the LinterList +func (list LinterList) Names() []string { + var names []string + for _, l := range list { + names = append(names, l.Name()) + } + return names +} diff --git a/cmd/crd-linter/linters/util.go b/cmd/crd-linter/linters/util.go new file mode 100644 index 000000000..247196fd2 --- /dev/null +++ b/cmd/crd-linter/linters/util.go @@ -0,0 +1,62 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +func recurseAllSchemas(versions []v1.CustomResourceDefinitionVersion, fn func(props v1.JSONSchemaProps, path string) []string) []string { + var errs []string + for i, vers := range versions { + // Skip versions that do not specify a schema + if vers.Schema == nil || vers.Schema.OpenAPIV3Schema == nil { + continue + } + + errs = append(errs, recurseAllProps(*vers.Schema.OpenAPIV3Schema, fmt.Sprintf("spec.versions[%d].schema.openAPIV3Schema", i), fn)...) + } + return errs +} + +func recurseAllProps(props v1.JSONSchemaProps, path string, fn func(props v1.JSONSchemaProps, path string) []string) []string { + var errs []string + errs = append(errs, fn(props, path)...) + for i, val := range props.AnyOf { + errs = append(errs, recurseAllProps(val, fmt.Sprintf("%s.anyOf[%d]", path, i), fn)...) + } + for i, val := range props.OneOf { + errs = append(errs, recurseAllProps(val, fmt.Sprintf("%s.oneOf[%d]", path, i), fn)...) + } + for i, val := range props.AllOf { + errs = append(errs, recurseAllProps(val, fmt.Sprintf("%s.allOf[%d]", path, i), fn)...) + } + for name, val := range props.Properties { + errs = append(errs, recurseAllProps(val, fmt.Sprintf("%s.properties.%s", path, name), fn)...) + } + if props.Items != nil { + if props.Items.Schema != nil { + errs = append(errs, recurseAllProps(*props.Items.Schema, fmt.Sprintf("%s.items", path), fn)...) + } + for i, v := range props.Items.JSONSchemas { + errs = append(errs, recurseAllProps(v, fmt.Sprintf("%s.items[%d]", path, i), fn)...) + } + } + return errs +} diff --git a/cmd/crd-linter/main.go b/cmd/crd-linter/main.go new file mode 100644 index 000000000..da08ea56a --- /dev/null +++ b/cmd/crd-linter/main.go @@ -0,0 +1,98 @@ +package main + +import ( + "log" + "os" + "strings" + + flag "github.com/spf13/pflag" + + "sigs.k8s.io/controller-tools/cmd/crd-linter/evaluator" + "sigs.k8s.io/controller-tools/cmd/crd-linter/exceptions" + "sigs.k8s.io/controller-tools/cmd/crd-linter/linters" + "sigs.k8s.io/controller-tools/cmd/crd-linter/reader" +) + +var ( + enabledLinters []string + + path string + + exceptionsFile string + outputExceptions bool + + errOnViolations bool +) + +func init() { + flag.StringSliceVar(&enabledLinters, "linters", linters.All().Names(), "Full list of linters to run against discovered CustomResourceDefinitions") + flag.StringVar(&path, "path", "", "Path to recursively search for CustomResourceDefinition objects") + flag.StringVar(&exceptionsFile, "exceptions-file", "", "Path to a list of exceptions for linter failures") + flag.BoolVar(&outputExceptions, "output-exceptions", false, "If true, an exception list file will be written to the file denoted by '--exceptions-file'") + flag.BoolVar(&errOnViolations, "error-on-violations", true, "If true, the linter will exit with a non-zero exit code if there are any unexpected violations") +} + +func main() { + flag.Parse() + + linters, err := linters.Named(enabledLinters) + if err != nil { + log.Fatalf("Failed to build linter list: %s", err.Error()) + } + + log.Printf("Enabled linters: %s", strings.Join(linters.Names(), ", ")) + + log.Printf("Searching path %q for CustomResourceDefinition objects", path) + crds, err := reader.DiscoverAllCRDs(path) + if err != nil { + log.Fatalf("Error reading CRDs from disk: %v", err) + } + + if outputExceptions { + results := evaluator.Evaluate(linters, crds, nil) + if exceptionsFile == "" { + log.Fatalf("--exceptions-file must be specified if --output-exceptions=true") + } + + e := results.ToExceptionList() + if err := e.WriteToFile(exceptionsFile); err != nil { + log.Fatalf("Failed to write exceptions file: %v", err) + } + return + } + + var excepted *exceptions.ExceptionList + if exceptionsFile != "" { + excepted, err = exceptions.LoadFromFile(exceptionsFile) + if err != nil { + log.Fatalf("Failed loading exceptions file %q: %v", exceptionsFile, err) + } + + log.Printf("Loaded %d exceptions from file %q", excepted.Size(), exceptionsFile) + } + + results := evaluator.Evaluate(linters, crds, excepted) + for _, r := range results { + if !r.HasViolations() { + continue + } + + log.Printf("CRD %q in file %q has linter failures:", r.Evaluated.CustomResourceDefinition.Name, r.Evaluated.OriginalFilename) + for _, list := range r.ViolatedLinters() { + log.Println() + log.Printf("\tLinter %q failed:", list.Linter.Name()) + for _, err := range list.Violations { + log.Printf("\t\t%s", err) + } + log.Println() + log.Printf("\tDescription: %s", list.Linter.Description()) + log.Println() + } + } + + if errOnViolations && results.HasViolations() { + log.Println() + log.Printf("Unexpected violations found!") + os.Exit(1) + } +} diff --git a/cmd/crd-linter/reader/directory.go b/cmd/crd-linter/reader/directory.go new file mode 100644 index 000000000..567ef984a --- /dev/null +++ b/cmd/crd-linter/reader/directory.go @@ -0,0 +1,93 @@ +/* +Copyright 2021 The Kubernetes 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 reader + +import ( + "bytes" + "io/fs" + "io/ioutil" + "log" + "path/filepath" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type accumulator struct { + crds []CRDToEvaluate +} + +type CRDToEvaluate struct { + // OriginalFilename is the name of the file that the CRD was read from + OriginalFilename string + + // OriginalAPIVersion is the API version of the resource as it is on disk + OriginalGroupVersionKind schema.GroupVersionKind + + // CustomResourceDefinition is the apiextensions.k8s.io/v1 representation of the CRD + CustomResourceDefinition *v1.CustomResourceDefinition +} + +func DiscoverAllCRDs(path string) ([]CRDToEvaluate, error) { + a := &accumulator{} + if err := filepath.WalkDir(path, a.processDirectory); err != nil { + return nil, err + } + return a.list(), nil +} + +func (a *accumulator) processDirectory(path string, d fs.DirEntry, err error) error { + if err != nil { + log.Printf("Error reading path %q: %v", path, err) + return nil + } + + if d.IsDir() { + return nil + } + + if filepath.Ext(path) != ".yaml" && filepath.Ext(path) != ".yml" { + log.Printf("Skipping file with non-yaml extension %q: %s", filepath.Ext(path), path) + return nil + } + + data, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + dataChunks := bytes.Split(data, []byte("---\n")) + for _, data := range dataChunks { + obj, gvk, err := DecodeCustomResourceDefinition(data, crdV1GVK.GroupVersion()) + if err != nil { + log.Printf("Failed to decode file %q: %v", path, err) + return nil + } + + a.crds = append(a.crds, CRDToEvaluate{ + OriginalFilename: path, + OriginalGroupVersionKind: *gvk, + CustomResourceDefinition: obj.(*v1.CustomResourceDefinition), + }) + } + + return nil +} + +func (a *accumulator) list() []CRDToEvaluate { + return a.crds +} diff --git a/cmd/crd-linter/reader/scheme.go b/cmd/crd-linter/reader/scheme.go new file mode 100644 index 000000000..f8774f21d --- /dev/null +++ b/cmd/crd-linter/reader/scheme.go @@ -0,0 +1,60 @@ +/* +Copyright 2021 The Kubernetes 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 reader + +import ( + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + apiextensionsinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/apimachinery/pkg/runtime/serializer/versioning" +) + +var ( + // defaultScheme is used to decode CustomResourceDefinition objects into + // the required 'v1' API version. + defaultScheme = runtime.NewScheme() + + // codec is used to decode CRD resources into the desired GVK + codec runtime.Serializer + + crdV1GVK = schema.GroupVersionKind{Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition"} + crdV1beta1GVK = schema.GroupVersionKind{Group: "apiextensions.k8s.io", Version: "v1beta1", Kind: "CustomResourceDefinition"} +) + +func init() { + apiextensionsinstall.Install(defaultScheme) + metav1.AddToGroupVersion(defaultScheme, schema.GroupVersion{Version: "v1"}) + + serializer := json.NewSerializerWithOptions(json.DefaultMetaFactory, defaultScheme, defaultScheme, json.SerializerOptions{Yaml: true}) + codec = versioning.NewCodec(serializer, serializer, defaultScheme, defaultScheme, defaultScheme, nil, runtime.InternalGroupVersioner, runtime.InternalGroupVersioner, "") +} + +func DecodeCustomResourceDefinition(data []byte, targetGV schema.GroupVersion) (runtime.Object, *schema.GroupVersionKind, error) { + internalObj := &apiextensions.CustomResourceDefinition{} + _, gvk, err := codec.Decode(data, nil, internalObj) + if err != nil { + return nil, nil, err + } + obj, err := defaultScheme.ConvertToVersion(internalObj, targetGV) + if err != nil { + return nil, nil, err + } + return obj, gvk, nil +} diff --git a/cmd/crd-linter/reader/scheme_test.go b/cmd/crd-linter/reader/scheme_test.go new file mode 100644 index 000000000..daf8b339b --- /dev/null +++ b/cmd/crd-linter/reader/scheme_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2021 The Kubernetes 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 reader + +import ( + "reflect" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" +) + +func TestDecodeCustomResourceDefinitionV1Beta1(t *testing.T) { + v1beta1Bytes := []byte(`apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: test-resource`) + + obj, gvk, err := DecodeCustomResourceDefinition(v1beta1Bytes, crdV1GVK.GroupVersion()) + if err != nil { + t.Fatalf("Error decoding bytes: %v", err) + } + if !reflect.DeepEqual(*gvk, crdV1beta1GVK) { + t.Fatalf("Unexpected GVK: %v", *gvk) + } + + _, ok := obj.(*v1.CustomResourceDefinition) + if !ok { + t.Fatalf("Invalid object type: %T", obj) + } +} + +func TestDecodeCustomResourceDefinitionV1(t *testing.T) { + v1beta1Bytes := []byte(`apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: test-resource`) + + obj, gvk, err := DecodeCustomResourceDefinition(v1beta1Bytes, crdV1GVK.GroupVersion()) + if err != nil { + t.Fatalf("Error decoding bytes: %v", err) + } + if !reflect.DeepEqual(*gvk, crdV1GVK) { + t.Fatalf("Unexpected GVK: %v", *gvk) + } + + _, ok := obj.(*v1.CustomResourceDefinition) + if !ok { + t.Fatalf("Invalid object type: %T", obj) + } +} + +func TestDecodeCustomResourceDefinitionV1ToV1beta1(t *testing.T) { + v1beta1Bytes := []byte(`apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: test-resource`) + + obj, gvk, err := DecodeCustomResourceDefinition(v1beta1Bytes, crdV1beta1GVK.GroupVersion()) + if err != nil { + t.Fatalf("Error decoding bytes: %v", err) + } + if !reflect.DeepEqual(*gvk, crdV1GVK) { + t.Fatalf("Unexpected GVK: %v", *gvk) + } + + _, ok := obj.(*v1beta1.CustomResourceDefinition) + if !ok { + t.Fatalf("Invalid object type: %T", obj) + } +} + +func TestDecodeCustomResourceDefinitionV1beta1ToV1beta1(t *testing.T) { + v1beta1Bytes := []byte(`apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: test-resource`) + + obj, gvk, err := DecodeCustomResourceDefinition(v1beta1Bytes, crdV1beta1GVK.GroupVersion()) + if err != nil { + t.Fatalf("Error decoding bytes: %v", err) + } + if !reflect.DeepEqual(*gvk, crdV1beta1GVK) { + t.Fatalf("Unexpected GVK: %v", *gvk) + } + + _, ok := obj.(*v1beta1.CustomResourceDefinition) + if !ok { + t.Fatalf("Invalid object type: %T", obj) + } +} From f4adb3c3c5a2292890c634fd1dbc4d9ac2600dff Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 30 Sep 2021 16:58:05 +0100 Subject: [PATCH 2/4] crd-linter: add 4 initial linters --- cmd/crd-linter/linters/linters.go | 5 +- cmd/crd-linter/linters/max_items_arrays.go | 48 ++++++++ .../linters/max_items_arrays_test.go | 72 ++++++++++++ cmd/crd-linter/linters/max_length_strings.go | 48 ++++++++ .../linters/max_length_strings_test.go | 81 +++++++++++++ .../linters/preserve_unknown_fields.go | 52 +++++++++ .../linters/preserve_unknown_fields_test.go | 108 ++++++++++++++++++ cmd/crd-linter/linters/schema_provided.go | 48 ++++++++ .../linters/schema_provided_test.go | 63 ++++++++++ go.mod | 1 + 10 files changed, 525 insertions(+), 1 deletion(-) create mode 100644 cmd/crd-linter/linters/max_items_arrays.go create mode 100644 cmd/crd-linter/linters/max_items_arrays_test.go create mode 100644 cmd/crd-linter/linters/max_length_strings.go create mode 100644 cmd/crd-linter/linters/max_length_strings_test.go create mode 100644 cmd/crd-linter/linters/preserve_unknown_fields.go create mode 100644 cmd/crd-linter/linters/preserve_unknown_fields_test.go create mode 100644 cmd/crd-linter/linters/schema_provided.go create mode 100644 cmd/crd-linter/linters/schema_provided_test.go diff --git a/cmd/crd-linter/linters/linters.go b/cmd/crd-linter/linters/linters.go index 0e2c4cacc..818aff48d 100644 --- a/cmd/crd-linter/linters/linters.go +++ b/cmd/crd-linter/linters/linters.go @@ -21,7 +21,10 @@ import ( ) var allLinters = []Linter{ - // Add new linters to this list + SchemaProvided{}, + PreserveUnknownFields{}, + MaxLengthStrings{}, + MaxItemsArrays{}, } var namedLinters = make(map[string]Linter) diff --git a/cmd/crd-linter/linters/max_items_arrays.go b/cmd/crd-linter/linters/max_items_arrays.go new file mode 100644 index 000000000..e316683ae --- /dev/null +++ b/cmd/crd-linter/linters/max_items_arrays.go @@ -0,0 +1,48 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// MaxItemsArrays checks all array type fields on a CRD to ensure they have +// a 'maxItems' specified. +type MaxItemsArrays struct{} + +func (m MaxItemsArrays) Name() string { + return "MaxItemsArrays" +} + +func (m MaxItemsArrays) Execute(crd *v1.CustomResourceDefinition) []string { + return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { + if props.Type == "array" && props.MaxItems == nil { + return []string{fmt.Sprintf("%s.maxItems is not specified on a field of type 'array'", path)} + } + return nil + }) +} + +func (m MaxItemsArrays) Description() string { + return "All 'array' typed fields should have a 'maxItems' specified, even if arbitrarily high, to ensure you do " + + "not accidentally store more data that originally intended. This allows the apiserver to be sure you do not " + + "store too much data in the apiserver, which can potentially lead to instability." +} + +var _ Linter = MaxItemsArrays{} diff --git a/cmd/crd-linter/linters/max_items_arrays_test.go b/cmd/crd-linter/linters/max_items_arrays_test.go new file mode 100644 index 000000000..8d5470ce8 --- /dev/null +++ b/cmd/crd-linter/linters/max_items_arrays_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "reflect" + "sort" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/diff" +) + +func TestMaxItemsArrays_Execute(t *testing.T) { + crd := &v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: true, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "test-string-field": {Type: "string"}, + "test-field-without-max-items": {Type: "array"}, + "any-of-field": {AnyOf: []v1.JSONSchemaProps{ + { + Type: "string", + }, + { + Type: "integer", + }, + { + Type: "array", + }, + }}, + }, + }, + }, + }, + }, + }, + } + + expectedErrors := []string{ + "spec.versions[0].schema.openAPIV3Schema.properties.any-of-field.anyOf[2].maxItems is not specified on a field of type 'array'", + "spec.versions[0].schema.openAPIV3Schema.properties.test-field-without-max-items.maxItems is not specified on a field of type 'array'", + } + + fieldEvaluator := MaxItemsArrays{} + errs := fieldEvaluator.Execute(crd) + // sort strings to allow for consistent comparison + sort.Strings(expectedErrors) + sort.Strings(errs) + + if !reflect.DeepEqual(errs, expectedErrors) { + t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) + } +} diff --git a/cmd/crd-linter/linters/max_length_strings.go b/cmd/crd-linter/linters/max_length_strings.go new file mode 100644 index 000000000..0d9d0e6d6 --- /dev/null +++ b/cmd/crd-linter/linters/max_length_strings.go @@ -0,0 +1,48 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// MaxLengthStrings checks all string type fields on a CRD to ensure they have +// a 'maxLength' specified. +type MaxLengthStrings struct{} + +func (m MaxLengthStrings) Name() string { + return "MaxLengthStrings" +} + +func (m MaxLengthStrings) Execute(crd *v1.CustomResourceDefinition) []string { + return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { + if props.Type == "string" && props.MaxLength == nil { + return []string{fmt.Sprintf("%s.maxLength is not specified on a field of type 'string'", path)} + } + return nil + }) +} + +func (m MaxLengthStrings) Description() string { + return "All 'string' typed fields should have a 'maxLength' specified, even if arbitrarily high, to ensure you do " + + "not accidentally store more data that originally intended. This allows the apiserver to be sure you do not " + + "store too much data in the apiserver, which can potentially lead to instability." +} + +var _ Linter = MaxLengthStrings{} diff --git a/cmd/crd-linter/linters/max_length_strings_test.go b/cmd/crd-linter/linters/max_length_strings_test.go new file mode 100644 index 000000000..2b4ac5e41 --- /dev/null +++ b/cmd/crd-linter/linters/max_length_strings_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "reflect" + "sort" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/diff" +) + +func TestMaxLengthString_Execute(t *testing.T) { + oneVal := int64(1) + crd := &v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: true, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "test-field-with-max-length": {Type: "string", MaxLength: &oneVal}, + "test-field-without-max-length": {Type: "string"}, + "any-of-field": {AnyOf: []v1.JSONSchemaProps{ + { + Type: "string", + }, + { + Type: "integer", + }, + }}, + "test-array-fields": { + Type: "array", + Items: &v1.JSONSchemaPropsOrArray{ + Schema: &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "string-field": {Type: "string"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + expectedErrors := []string{ + "spec.versions[0].schema.openAPIV3Schema.properties.any-of-field.anyOf[0].maxLength is not specified on a field of type 'string'", + "spec.versions[0].schema.openAPIV3Schema.properties.test-array-fields.items.properties.string-field.maxLength is not specified on a field of type 'string'", + "spec.versions[0].schema.openAPIV3Schema.properties.test-field-without-max-length.maxLength is not specified on a field of type 'string'", + } + + fieldEvaluator := MaxLengthStrings{} + errs := fieldEvaluator.Execute(crd) + // sort strings to allow for consistent comparison + sort.Strings(expectedErrors) + sort.Strings(errs) + + if !reflect.DeepEqual(errs, expectedErrors) { + t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) + } +} diff --git a/cmd/crd-linter/linters/preserve_unknown_fields.go b/cmd/crd-linter/linters/preserve_unknown_fields.go new file mode 100644 index 000000000..52e5e17be --- /dev/null +++ b/cmd/crd-linter/linters/preserve_unknown_fields.go @@ -0,0 +1,52 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// PreserveUnknownFields checks if a CRD contains a 'preserveUnknownFields' marker +// either at the top level, or on any specific sub-field of the resource. +type PreserveUnknownFields struct{} + +var _ Linter = PreserveUnknownFields{} + +func (p PreserveUnknownFields) Name() string { + return "PreserveUnknownFields" +} + +func (p PreserveUnknownFields) Description() string { + return "Fields should avoid using 'preserveUnknownFields' as it means any data can be persisted into the Kubernetes apiserver " + + "without any guards on the size and type of data. Setting this to true is no longer permitted in the 'v1' API version of " + + "CustomResourceDefinitions, and as such, it should not be used: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-pruning" +} + +func (p PreserveUnknownFields) Execute(crd *v1.CustomResourceDefinition) []string { + var errs []string + if crd.Spec.PreserveUnknownFields { + errs = append(errs, "spec.preserveUnknownFields is set to 'true'") + } + return append(errs, recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { + if props.XPreserveUnknownFields != nil && *props.XPreserveUnknownFields == true { + return []string{fmt.Sprintf("%s.x-kubernetes-preserve-unknown-fields is set to 'true'", path)} + } + return nil + })...) +} diff --git a/cmd/crd-linter/linters/preserve_unknown_fields_test.go b/cmd/crd-linter/linters/preserve_unknown_fields_test.go new file mode 100644 index 000000000..9ab55de6a --- /dev/null +++ b/cmd/crd-linter/linters/preserve_unknown_fields_test.go @@ -0,0 +1,108 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "reflect" + "sort" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/diff" +) + +func TestPreserveUnknownFields_Execute_WithPreserveUnknownFields(t *testing.T) { + trueVal := true + falseVal := false + crd := &v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: true, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "test-field-with-preserve-fields": {XPreserveUnknownFields: &trueVal}, + "test-field-with-false-preserve-fields": {XPreserveUnknownFields: &falseVal}, + "test-field-with-nil-preserve-fields": {XPreserveUnknownFields: nil}, + "test-field-with-nested-preserve-fields": { + Properties: map[string]v1.JSONSchemaProps{ + "nested-field": {XPreserveUnknownFields: &trueVal}, + }, + }, + "array-type-field": { + Type: "array", + Items: &v1.JSONSchemaPropsOrArray{ + Schema: &v1.JSONSchemaProps{ + XPreserveUnknownFields: &trueVal, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + expectedErrors := []string{ + "spec.preserveUnknownFields is set to 'true'", + "spec.versions[0].schema.openAPIV3Schema.properties.array-type-field.items.x-kubernetes-preserve-unknown-fields is set to 'true'", + "spec.versions[0].schema.openAPIV3Schema.properties.test-field-with-nested-preserve-fields.properties.nested-field.x-kubernetes-preserve-unknown-fields is set to 'true'", + "spec.versions[0].schema.openAPIV3Schema.properties.test-field-with-preserve-fields.x-kubernetes-preserve-unknown-fields is set to 'true'", + } + + fieldEvaluator := PreserveUnknownFields{} + errs := fieldEvaluator.Execute(crd) + // sort strings to allow for consistent comparison + sort.Strings(expectedErrors) + sort.Strings(errs) + + if !reflect.DeepEqual(errs, expectedErrors) { + t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) + } +} + +func TestPreserveUnknownFields_Execute_WithNoPreserveUnknownFields(t *testing.T) { + falseVal := false + crd := &v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: false, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "test-field-with-false-preserve-fields": {XPreserveUnknownFields: &falseVal}, + }, + }, + }, + }, + }, + }, + } + + fieldEvaluator := PreserveUnknownFields{} + errs := fieldEvaluator.Execute(crd) + // sort strings to allow for consistent comparison + sort.Strings(errs) + + if errs != nil { + t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, nil)) + } +} diff --git a/cmd/crd-linter/linters/schema_provided.go b/cmd/crd-linter/linters/schema_provided.go new file mode 100644 index 000000000..a3c568564 --- /dev/null +++ b/cmd/crd-linter/linters/schema_provided.go @@ -0,0 +1,48 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "fmt" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// SchemaProvided verifies that all API versions provide a schema +type SchemaProvided struct{} + +var _ Linter = SchemaProvided{} + +func (p SchemaProvided) Name() string { + return "SchemaProvided" +} + +func (p SchemaProvided) Description() string { + return "Not providing a schema is no longer possible with v1 CRDs, and means many API server features such as " + + "server-side apply are not possible. Additionally, custom resources contents cannot be validated and arbitrary " + + "data can be stored in objects, leading to potential API server instability." +} + +func (p SchemaProvided) Execute(crd *v1.CustomResourceDefinition) []string { + var errs []string + for i, vers := range crd.Spec.Versions { + if vers.Schema == nil || vers.Schema.OpenAPIV3Schema == nil { + errs = append(errs, fmt.Sprintf("spec.versions[%d] (%s) does not provide a schema", i, vers.Name)) + } + } + return errs +} diff --git a/cmd/crd-linter/linters/schema_provided_test.go b/cmd/crd-linter/linters/schema_provided_test.go new file mode 100644 index 000000000..6c962b3cb --- /dev/null +++ b/cmd/crd-linter/linters/schema_provided_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2021 The Kubernetes 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 linters + +import ( + "reflect" + "sort" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/diff" +) + +func TestSchemaProvided_Execute_WithNoSchema(t *testing.T) { + crd := &v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: true, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: nil, + }, + { + Name: "v1alpha2", + Schema: &v1.CustomResourceValidation{OpenAPIV3Schema: nil}, + }, + { + Name: "v1alpha3", + Schema: &v1.CustomResourceValidation{OpenAPIV3Schema: &v1.JSONSchemaProps{}}, + }, + }, + }, + } + + expectedErrors := []string{ + "spec.versions[0] (v1alpha1) does not provide a schema", + "spec.versions[1] (v1alpha2) does not provide a schema", + } + + fieldEvaluator := SchemaProvided{} + errs := fieldEvaluator.Execute(crd) + // sort strings to allow for consistent comparison + sort.Strings(expectedErrors) + sort.Strings(errs) + + if !reflect.DeepEqual(errs, expectedErrors) { + t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) + } +} diff --git a/go.mod b/go.mod index 8647f5442..4877a3b85 100644 --- a/go.mod +++ b/go.mod @@ -15,5 +15,6 @@ require ( k8s.io/api v0.22.2 k8s.io/apiextensions-apiserver v0.22.2 k8s.io/apimachinery v0.22.2 + k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a sigs.k8s.io/yaml v1.2.0 ) From 70c7eb3bc610d3ac08c936e58fb2f80a4b488c47 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Fri, 1 Oct 2021 09:42:02 +0100 Subject: [PATCH 3/4] Fix golangci-lint failures by aliasing imports --- cmd/crd-linter/linters/interface.go | 2 +- cmd/crd-linter/linters/max_items_arrays.go | 2 +- cmd/crd-linter/linters/max_items_arrays_test.go | 2 +- cmd/crd-linter/linters/max_length_strings.go | 2 +- cmd/crd-linter/linters/max_length_strings_test.go | 2 +- cmd/crd-linter/linters/preserve_unknown_fields.go | 2 +- cmd/crd-linter/linters/preserve_unknown_fields_test.go | 2 +- cmd/crd-linter/linters/schema_provided.go | 2 +- cmd/crd-linter/linters/schema_provided_test.go | 2 +- cmd/crd-linter/linters/util.go | 2 +- cmd/crd-linter/reader/directory.go | 2 +- cmd/crd-linter/reader/scheme_test.go | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/crd-linter/linters/interface.go b/cmd/crd-linter/linters/interface.go index cec1eabcf..845e0d7e9 100644 --- a/cmd/crd-linter/linters/interface.go +++ b/cmd/crd-linter/linters/interface.go @@ -16,7 +16,7 @@ limitations under the License. package linters -import "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +import v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" // A Linter evaluates a CRD resource and provides information about policy violations. type Linter interface { diff --git a/cmd/crd-linter/linters/max_items_arrays.go b/cmd/crd-linter/linters/max_items_arrays.go index e316683ae..fb1963a7d 100644 --- a/cmd/crd-linter/linters/max_items_arrays.go +++ b/cmd/crd-linter/linters/max_items_arrays.go @@ -19,7 +19,7 @@ package linters import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) // MaxItemsArrays checks all array type fields on a CRD to ensure they have diff --git a/cmd/crd-linter/linters/max_items_arrays_test.go b/cmd/crd-linter/linters/max_items_arrays_test.go index 8d5470ce8..ce55c9281 100644 --- a/cmd/crd-linter/linters/max_items_arrays_test.go +++ b/cmd/crd-linter/linters/max_items_arrays_test.go @@ -21,7 +21,7 @@ import ( "sort" "testing" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/utils/diff" ) diff --git a/cmd/crd-linter/linters/max_length_strings.go b/cmd/crd-linter/linters/max_length_strings.go index 0d9d0e6d6..a33038be6 100644 --- a/cmd/crd-linter/linters/max_length_strings.go +++ b/cmd/crd-linter/linters/max_length_strings.go @@ -19,7 +19,7 @@ package linters import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) // MaxLengthStrings checks all string type fields on a CRD to ensure they have diff --git a/cmd/crd-linter/linters/max_length_strings_test.go b/cmd/crd-linter/linters/max_length_strings_test.go index 2b4ac5e41..6ef5ce5b9 100644 --- a/cmd/crd-linter/linters/max_length_strings_test.go +++ b/cmd/crd-linter/linters/max_length_strings_test.go @@ -21,7 +21,7 @@ import ( "sort" "testing" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/utils/diff" ) diff --git a/cmd/crd-linter/linters/preserve_unknown_fields.go b/cmd/crd-linter/linters/preserve_unknown_fields.go index 52e5e17be..b19d8e57d 100644 --- a/cmd/crd-linter/linters/preserve_unknown_fields.go +++ b/cmd/crd-linter/linters/preserve_unknown_fields.go @@ -19,7 +19,7 @@ package linters import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) // PreserveUnknownFields checks if a CRD contains a 'preserveUnknownFields' marker diff --git a/cmd/crd-linter/linters/preserve_unknown_fields_test.go b/cmd/crd-linter/linters/preserve_unknown_fields_test.go index 9ab55de6a..836433596 100644 --- a/cmd/crd-linter/linters/preserve_unknown_fields_test.go +++ b/cmd/crd-linter/linters/preserve_unknown_fields_test.go @@ -21,7 +21,7 @@ import ( "sort" "testing" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/utils/diff" ) diff --git a/cmd/crd-linter/linters/schema_provided.go b/cmd/crd-linter/linters/schema_provided.go index a3c568564..b88610c59 100644 --- a/cmd/crd-linter/linters/schema_provided.go +++ b/cmd/crd-linter/linters/schema_provided.go @@ -19,7 +19,7 @@ package linters import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) // SchemaProvided verifies that all API versions provide a schema diff --git a/cmd/crd-linter/linters/schema_provided_test.go b/cmd/crd-linter/linters/schema_provided_test.go index 6c962b3cb..c888d58dd 100644 --- a/cmd/crd-linter/linters/schema_provided_test.go +++ b/cmd/crd-linter/linters/schema_provided_test.go @@ -21,7 +21,7 @@ import ( "sort" "testing" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/utils/diff" ) diff --git a/cmd/crd-linter/linters/util.go b/cmd/crd-linter/linters/util.go index 247196fd2..24b13f1c2 100644 --- a/cmd/crd-linter/linters/util.go +++ b/cmd/crd-linter/linters/util.go @@ -19,7 +19,7 @@ package linters import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) func recurseAllSchemas(versions []v1.CustomResourceDefinitionVersion, fn func(props v1.JSONSchemaProps, path string) []string) []string { diff --git a/cmd/crd-linter/reader/directory.go b/cmd/crd-linter/reader/directory.go index 567ef984a..2690f161e 100644 --- a/cmd/crd-linter/reader/directory.go +++ b/cmd/crd-linter/reader/directory.go @@ -23,7 +23,7 @@ import ( "log" "path/filepath" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) diff --git a/cmd/crd-linter/reader/scheme_test.go b/cmd/crd-linter/reader/scheme_test.go index daf8b339b..1619a3404 100644 --- a/cmd/crd-linter/reader/scheme_test.go +++ b/cmd/crd-linter/reader/scheme_test.go @@ -20,7 +20,7 @@ import ( "reflect" "testing" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" ) From 2f473e2464bf5f42ad847e9289460d4969c7fbf7 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Tue, 2 Nov 2021 11:50:49 +0000 Subject: [PATCH 4/4] Address review feedback --- cmd/crd-linter/evaluator/types.go | 2 +- cmd/crd-linter/exceptions/exceptions.go | 5 ++-- cmd/crd-linter/linters/interface.go | 23 ++++++++++++++++++- cmd/crd-linter/linters/linters.go | 2 +- cmd/crd-linter/linters/max_items_arrays.go | 6 ++--- .../linters/max_items_arrays_test.go | 8 +++---- cmd/crd-linter/linters/max_length_strings.go | 6 ++--- .../linters/max_length_strings_test.go | 8 +++---- .../linters/preserve_unknown_fields.go | 22 +++++++++--------- .../linters/preserve_unknown_fields_test.go | 18 +++++++-------- cmd/crd-linter/linters/schema_provided.go | 6 ++--- .../linters/schema_provided_test.go | 8 +++---- cmd/crd-linter/linters/util.go | 8 +++---- 13 files changed, 71 insertions(+), 51 deletions(-) diff --git a/cmd/crd-linter/evaluator/types.go b/cmd/crd-linter/evaluator/types.go index 33b3b28cd..26d9cdaa0 100644 --- a/cmd/crd-linter/evaluator/types.go +++ b/cmd/crd-linter/evaluator/types.go @@ -57,7 +57,7 @@ func (r *Results) ViolatedLinters() []ViolationList { type ResultsList []Results func (results ResultsList) ToExceptionList() *exceptions.ExceptionList { - list := exceptions.New() + list := exceptions.NewExceptionList() for _, r := range results { if !r.HasViolations() { continue diff --git a/cmd/crd-linter/exceptions/exceptions.go b/cmd/crd-linter/exceptions/exceptions.go index ea666493f..a9e0d2287 100644 --- a/cmd/crd-linter/exceptions/exceptions.go +++ b/cmd/crd-linter/exceptions/exceptions.go @@ -74,10 +74,11 @@ func (l *ExceptionList) Size() int { } func (l *ExceptionList) WriteToFile(path string) error { + // #nosec G306 return ioutil.WriteFile(path, []byte(l.String()), 0644) } -func New() *ExceptionList { +func NewExceptionList() *ExceptionList { return &ExceptionList{Exceptions: make([]Exception, 0)} } @@ -89,7 +90,7 @@ func LoadFromFile(path string) (*ExceptionList, error) { return nil, err } - list := New() + list := NewExceptionList() entries := strings.Split(string(data), "\n") for lineNo, e := range entries { // skip empty lines diff --git a/cmd/crd-linter/linters/interface.go b/cmd/crd-linter/linters/interface.go index 845e0d7e9..9c4e1b3ab 100644 --- a/cmd/crd-linter/linters/interface.go +++ b/cmd/crd-linter/linters/interface.go @@ -24,9 +24,30 @@ type Linter interface { Name() string // Execute runs the linter against the given custom resource definition - Execute(*v1.CustomResourceDefinition) []string + Execute(*v1.CustomResourceDefinition) WarningList // Description prints human-readable, actionable information and references about // what the linter does. Description() string } + +type Warning struct { + Message string +} + +type WarningList []Warning + +func (w WarningList) Len() int { return len(w) } +func (w WarningList) Swap(i, j int) { w[i], w[j] = w[j], w[i] } +func (w WarningList) Less(i, j int) bool { return w[i].Message < w[j].Message } + +// Used internally to construct a warning list from a set of strings. +// This function is not exported because as the Warning struct grows, the arguments +// to the function is likely to change in backward-incompatible ways. +func newWarningList(messages ...string) WarningList { + var w WarningList + for _, msg := range messages { + w = append(w, Warning{Message: msg}) + } + return w +} diff --git a/cmd/crd-linter/linters/linters.go b/cmd/crd-linter/linters/linters.go index 818aff48d..2bb7b7c46 100644 --- a/cmd/crd-linter/linters/linters.go +++ b/cmd/crd-linter/linters/linters.go @@ -22,7 +22,7 @@ import ( var allLinters = []Linter{ SchemaProvided{}, - PreserveUnknownFields{}, + NoPreserveUnknownFields{}, MaxLengthStrings{}, MaxItemsArrays{}, } diff --git a/cmd/crd-linter/linters/max_items_arrays.go b/cmd/crd-linter/linters/max_items_arrays.go index fb1963a7d..be59e4231 100644 --- a/cmd/crd-linter/linters/max_items_arrays.go +++ b/cmd/crd-linter/linters/max_items_arrays.go @@ -30,10 +30,10 @@ func (m MaxItemsArrays) Name() string { return "MaxItemsArrays" } -func (m MaxItemsArrays) Execute(crd *v1.CustomResourceDefinition) []string { - return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { +func (m MaxItemsArrays) Execute(crd *v1.CustomResourceDefinition) WarningList { + return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []Warning { if props.Type == "array" && props.MaxItems == nil { - return []string{fmt.Sprintf("%s.maxItems is not specified on a field of type 'array'", path)} + return newWarningList(fmt.Sprintf("%s.maxItems is not specified on a field of type 'array'", path)) } return nil }) diff --git a/cmd/crd-linter/linters/max_items_arrays_test.go b/cmd/crd-linter/linters/max_items_arrays_test.go index ce55c9281..42a26da63 100644 --- a/cmd/crd-linter/linters/max_items_arrays_test.go +++ b/cmd/crd-linter/linters/max_items_arrays_test.go @@ -55,16 +55,16 @@ func TestMaxItemsArrays_Execute(t *testing.T) { }, } - expectedErrors := []string{ + expectedErrors := newWarningList( "spec.versions[0].schema.openAPIV3Schema.properties.any-of-field.anyOf[2].maxItems is not specified on a field of type 'array'", "spec.versions[0].schema.openAPIV3Schema.properties.test-field-without-max-items.maxItems is not specified on a field of type 'array'", - } + ) fieldEvaluator := MaxItemsArrays{} errs := fieldEvaluator.Execute(crd) // sort strings to allow for consistent comparison - sort.Strings(expectedErrors) - sort.Strings(errs) + sort.Sort(expectedErrors) + sort.Sort(errs) if !reflect.DeepEqual(errs, expectedErrors) { t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) diff --git a/cmd/crd-linter/linters/max_length_strings.go b/cmd/crd-linter/linters/max_length_strings.go index a33038be6..88367dd46 100644 --- a/cmd/crd-linter/linters/max_length_strings.go +++ b/cmd/crd-linter/linters/max_length_strings.go @@ -30,10 +30,10 @@ func (m MaxLengthStrings) Name() string { return "MaxLengthStrings" } -func (m MaxLengthStrings) Execute(crd *v1.CustomResourceDefinition) []string { - return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { +func (m MaxLengthStrings) Execute(crd *v1.CustomResourceDefinition) WarningList { + return recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []Warning { if props.Type == "string" && props.MaxLength == nil { - return []string{fmt.Sprintf("%s.maxLength is not specified on a field of type 'string'", path)} + return newWarningList(fmt.Sprintf("%s.maxLength is not specified on a field of type 'string'", path)) } return nil }) diff --git a/cmd/crd-linter/linters/max_length_strings_test.go b/cmd/crd-linter/linters/max_length_strings_test.go index 6ef5ce5b9..2d2a83070 100644 --- a/cmd/crd-linter/linters/max_length_strings_test.go +++ b/cmd/crd-linter/linters/max_length_strings_test.go @@ -63,17 +63,17 @@ func TestMaxLengthString_Execute(t *testing.T) { }, } - expectedErrors := []string{ + expectedErrors := newWarningList( "spec.versions[0].schema.openAPIV3Schema.properties.any-of-field.anyOf[0].maxLength is not specified on a field of type 'string'", "spec.versions[0].schema.openAPIV3Schema.properties.test-array-fields.items.properties.string-field.maxLength is not specified on a field of type 'string'", "spec.versions[0].schema.openAPIV3Schema.properties.test-field-without-max-length.maxLength is not specified on a field of type 'string'", - } + ) fieldEvaluator := MaxLengthStrings{} errs := fieldEvaluator.Execute(crd) // sort strings to allow for consistent comparison - sort.Strings(expectedErrors) - sort.Strings(errs) + sort.Sort(expectedErrors) + sort.Sort(errs) if !reflect.DeepEqual(errs, expectedErrors) { t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) diff --git a/cmd/crd-linter/linters/preserve_unknown_fields.go b/cmd/crd-linter/linters/preserve_unknown_fields.go index b19d8e57d..c79121ef1 100644 --- a/cmd/crd-linter/linters/preserve_unknown_fields.go +++ b/cmd/crd-linter/linters/preserve_unknown_fields.go @@ -22,30 +22,30 @@ import ( v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) -// PreserveUnknownFields checks if a CRD contains a 'preserveUnknownFields' marker +// NoPreserveUnknownFields checks if a CRD contains a 'preserveUnknownFields' marker // either at the top level, or on any specific sub-field of the resource. -type PreserveUnknownFields struct{} +type NoPreserveUnknownFields struct{} -var _ Linter = PreserveUnknownFields{} +var _ Linter = NoPreserveUnknownFields{} -func (p PreserveUnknownFields) Name() string { - return "PreserveUnknownFields" +func (p NoPreserveUnknownFields) Name() string { + return "NoPreserveUnknownFields" } -func (p PreserveUnknownFields) Description() string { +func (p NoPreserveUnknownFields) Description() string { return "Fields should avoid using 'preserveUnknownFields' as it means any data can be persisted into the Kubernetes apiserver " + "without any guards on the size and type of data. Setting this to true is no longer permitted in the 'v1' API version of " + "CustomResourceDefinitions, and as such, it should not be used: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-pruning" } -func (p PreserveUnknownFields) Execute(crd *v1.CustomResourceDefinition) []string { - var errs []string +func (p NoPreserveUnknownFields) Execute(crd *v1.CustomResourceDefinition) WarningList { + var errs []Warning if crd.Spec.PreserveUnknownFields { - errs = append(errs, "spec.preserveUnknownFields is set to 'true'") + errs = append(errs, Warning{Message: "spec.preserveUnknownFields is set to 'true'"}) } - return append(errs, recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []string { + return append(errs, recurseAllSchemas(crd.Spec.Versions, func(props v1.JSONSchemaProps, path string) []Warning { if props.XPreserveUnknownFields != nil && *props.XPreserveUnknownFields == true { - return []string{fmt.Sprintf("%s.x-kubernetes-preserve-unknown-fields is set to 'true'", path)} + return newWarningList(fmt.Sprintf("%s.x-kubernetes-preserve-unknown-fields is set to 'true'", path)) } return nil })...) diff --git a/cmd/crd-linter/linters/preserve_unknown_fields_test.go b/cmd/crd-linter/linters/preserve_unknown_fields_test.go index 836433596..7501a3248 100644 --- a/cmd/crd-linter/linters/preserve_unknown_fields_test.go +++ b/cmd/crd-linter/linters/preserve_unknown_fields_test.go @@ -25,7 +25,7 @@ import ( "k8s.io/utils/diff" ) -func TestPreserveUnknownFields_Execute_WithPreserveUnknownFields(t *testing.T) { +func TestNoPreserveUnknownFields_Execute_WithNoPreserveUnknownFields(t *testing.T) { trueVal := true falseVal := false crd := &v1.CustomResourceDefinition{ @@ -60,25 +60,25 @@ func TestPreserveUnknownFields_Execute_WithPreserveUnknownFields(t *testing.T) { }, } - expectedErrors := []string{ + expectedErrors := newWarningList( "spec.preserveUnknownFields is set to 'true'", "spec.versions[0].schema.openAPIV3Schema.properties.array-type-field.items.x-kubernetes-preserve-unknown-fields is set to 'true'", "spec.versions[0].schema.openAPIV3Schema.properties.test-field-with-nested-preserve-fields.properties.nested-field.x-kubernetes-preserve-unknown-fields is set to 'true'", "spec.versions[0].schema.openAPIV3Schema.properties.test-field-with-preserve-fields.x-kubernetes-preserve-unknown-fields is set to 'true'", - } + ) - fieldEvaluator := PreserveUnknownFields{} + fieldEvaluator := NoPreserveUnknownFields{} errs := fieldEvaluator.Execute(crd) // sort strings to allow for consistent comparison - sort.Strings(expectedErrors) - sort.Strings(errs) + sort.Sort(expectedErrors) + sort.Sort(errs) if !reflect.DeepEqual(errs, expectedErrors) { t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) } } -func TestPreserveUnknownFields_Execute_WithNoPreserveUnknownFields(t *testing.T) { +func TestNoPreserveUnknownFields_Execute_WithNoNoPreserveUnknownFields(t *testing.T) { falseVal := false crd := &v1.CustomResourceDefinition{ Spec: v1.CustomResourceDefinitionSpec{ @@ -97,10 +97,8 @@ func TestPreserveUnknownFields_Execute_WithNoPreserveUnknownFields(t *testing.T) }, } - fieldEvaluator := PreserveUnknownFields{} + fieldEvaluator := NoPreserveUnknownFields{} errs := fieldEvaluator.Execute(crd) - // sort strings to allow for consistent comparison - sort.Strings(errs) if errs != nil { t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, nil)) diff --git a/cmd/crd-linter/linters/schema_provided.go b/cmd/crd-linter/linters/schema_provided.go index b88610c59..935a8afda 100644 --- a/cmd/crd-linter/linters/schema_provided.go +++ b/cmd/crd-linter/linters/schema_provided.go @@ -37,11 +37,11 @@ func (p SchemaProvided) Description() string { "data can be stored in objects, leading to potential API server instability." } -func (p SchemaProvided) Execute(crd *v1.CustomResourceDefinition) []string { - var errs []string +func (p SchemaProvided) Execute(crd *v1.CustomResourceDefinition) WarningList { + var errs []Warning for i, vers := range crd.Spec.Versions { if vers.Schema == nil || vers.Schema.OpenAPIV3Schema == nil { - errs = append(errs, fmt.Sprintf("spec.versions[%d] (%s) does not provide a schema", i, vers.Name)) + errs = append(errs, Warning{Message: fmt.Sprintf("spec.versions[%d] (%s) does not provide a schema", i, vers.Name)}) } } return errs diff --git a/cmd/crd-linter/linters/schema_provided_test.go b/cmd/crd-linter/linters/schema_provided_test.go index c888d58dd..5daac6898 100644 --- a/cmd/crd-linter/linters/schema_provided_test.go +++ b/cmd/crd-linter/linters/schema_provided_test.go @@ -46,16 +46,16 @@ func TestSchemaProvided_Execute_WithNoSchema(t *testing.T) { }, } - expectedErrors := []string{ + expectedErrors := newWarningList( "spec.versions[0] (v1alpha1) does not provide a schema", "spec.versions[1] (v1alpha2) does not provide a schema", - } + ) fieldEvaluator := SchemaProvided{} errs := fieldEvaluator.Execute(crd) // sort strings to allow for consistent comparison - sort.Strings(expectedErrors) - sort.Strings(errs) + sort.Sort(expectedErrors) + sort.Sort(errs) if !reflect.DeepEqual(errs, expectedErrors) { t.Errorf("returned errors were not as expected: %s", diff.ObjectGoPrintSideBySide(errs, expectedErrors)) diff --git a/cmd/crd-linter/linters/util.go b/cmd/crd-linter/linters/util.go index 24b13f1c2..aeaf59362 100644 --- a/cmd/crd-linter/linters/util.go +++ b/cmd/crd-linter/linters/util.go @@ -22,8 +22,8 @@ import ( v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) -func recurseAllSchemas(versions []v1.CustomResourceDefinitionVersion, fn func(props v1.JSONSchemaProps, path string) []string) []string { - var errs []string +func recurseAllSchemas(versions []v1.CustomResourceDefinitionVersion, fn func(props v1.JSONSchemaProps, path string) []Warning) []Warning { + var errs []Warning for i, vers := range versions { // Skip versions that do not specify a schema if vers.Schema == nil || vers.Schema.OpenAPIV3Schema == nil { @@ -35,8 +35,8 @@ func recurseAllSchemas(versions []v1.CustomResourceDefinitionVersion, fn func(pr return errs } -func recurseAllProps(props v1.JSONSchemaProps, path string, fn func(props v1.JSONSchemaProps, path string) []string) []string { - var errs []string +func recurseAllProps(props v1.JSONSchemaProps, path string, fn func(props v1.JSONSchemaProps, path string) []Warning) []Warning { + var errs []Warning errs = append(errs, fn(props, path)...) for i, val := range props.AnyOf { errs = append(errs, recurseAllProps(val, fmt.Sprintf("%s.anyOf[%d]", path, i), fn)...)