From 78ed040b7d8c19dfbe2d12cc728e641f6454e945 Mon Sep 17 00:00:00 2001 From: Bill Franklin Date: Fri, 15 Jul 2022 10:56:15 +0100 Subject: [PATCH 1/2] Forbid duplicate keys in strict mode Prevents specifying duplicate keys using UnmarshallStrict: https://pkg.go.dev/gopkg.in/yaml.v2#UnmarshalStrict --- Readme.md | 2 +- pkg/config/config.go | 2 +- pkg/validator/validator.go | 7 +++- pkg/validator/validator_test.go | 70 ++++++++++++++++++++++++++++++- site/content/docs/usage.md | 4 +- site/public/docs/usage/index.html | 4 +- 6 files changed, 81 insertions(+), 8 deletions(-) diff --git a/Readme.md b/Readme.md index 99cb35a..e791dd9 100644 --- a/Readme.md +++ b/Readme.md @@ -98,7 +98,7 @@ Usage: ./bin/kubeconform [OPTION]... [FILE OR FOLDER]... -skip string comma-separated list of kinds to ignore -strict - disallow additional properties not in schema + disallow additional properties not in schema or duplicated keys -summary print a summary at the end (ignored for junit output) -v show version information diff --git a/pkg/config/config.go b/pkg/config/config.go index c0c374e..37df885 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -72,7 +72,7 @@ func FromFlags(progName string, args []string) (Config, string, error) { flags.Var(&ignoreFilenamePatterns, "ignore-filename-pattern", "regular expression specifying paths to ignore (can be specified multiple times)") flags.BoolVar(&c.Summary, "summary", false, "print a summary at the end (ignored for junit output)") flags.IntVar(&c.NumberOfWorkers, "n", 4, "number of goroutines to run concurrently") - flags.BoolVar(&c.Strict, "strict", false, "disallow additional properties not in schema") + flags.BoolVar(&c.Strict, "strict", false, "disallow additional properties not in schema or duplicated keys") flags.StringVar(&c.OutputFormat, "output", "text", "output format - json, junit, tap, text") flags.BoolVar(&c.Verbose, "verbose", false, "print results for all resources (ignored for tap and junit output)") flags.BoolVar(&c.SkipTLS, "insecure-skip-tls-verify", false, "disable verification of the server's SSL certificate. This will make your HTTPS connections insecure") diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index ffa224d..08d2103 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -112,7 +112,12 @@ func (val *v) ValidateResource(res resource.Resource) Result { } var r map[string]interface{} - if err := yaml.Unmarshal(res.Bytes, &r); err != nil { + unmarshaller := yaml.Unmarshal + if val.opts.Strict { + unmarshaller = yaml.UnmarshalStrict + } + + if err := unmarshaller(res.Bytes, &r); err != nil { return Result{Resource: res, Status: Error, Err: fmt.Errorf("error unmarshalling resource: %s", err)} } diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index d8e7751..f78548a 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -27,6 +27,7 @@ func TestValidate(t *testing.T) { rawResource, schemaRegistry1 []byte schemaRegistry2 []byte ignoreMissingSchema bool + strict bool expect Status }{ { @@ -60,6 +61,7 @@ lastName: bar }`), nil, false, + false, Valid, }, { @@ -93,6 +95,7 @@ lastName: bar }`), nil, false, + false, Invalid, }, { @@ -125,8 +128,61 @@ firstName: foo }`), nil, false, + false, Invalid, }, + { + "key \"firstName\" already set in map", + []byte(` +kind: name +apiVersion: v1 +firstName: foo +firstName: bar +`), + []byte(`{ + "title": "Example Schema", + "type": "object", + "properties": { + "kind": { + "type": "string" + }, + "firstName": { + "type": "string" + } + }, + "required": ["firstName"] +}`), + nil, + false, + true, + Error, + }, + { + "key firstname already set in map in non-strict mode", + []byte(` +kind: name +apiVersion: v1 +firstName: foo +firstName: bar +`), + []byte(`{ + "title": "Example Schema", + "type": "object", + "properties": { + "kind": { + "type": "string" + }, + "firstName": { + "type": "string" + } + }, + "required": ["firstName"] +}`), + nil, + false, + false, + Valid, + }, { "resource has invalid yaml", []byte(` @@ -161,6 +217,7 @@ lastName: bar }`), nil, false, + false, Error, }, { @@ -196,6 +253,7 @@ lastName: bar }, "required": ["firstName", "lastName"] }`), + false, false, Valid, }, @@ -232,6 +290,7 @@ lastName: bar }, "required": ["firstName", "lastName"] }`), + false, false, Valid, }, @@ -246,6 +305,7 @@ lastName: bar nil, nil, true, + false, Skipped, }, { @@ -259,6 +319,7 @@ lastName: bar nil, nil, false, + false, Error, }, { @@ -272,6 +333,7 @@ lastName: bar []byte(`error page`), []byte(`error page`), true, + false, Skipped, }, { @@ -285,6 +347,7 @@ lastName: bar []byte(`error page`), []byte(`error page`), false, + false, Error, }, } { @@ -293,6 +356,7 @@ lastName: bar SkipKinds: map[string]struct{}{}, RejectKinds: map[string]struct{}{}, IgnoreMissingSchemas: testCase.ignoreMissingSchema, + Strict: testCase.strict, }, schemaCache: nil, schemaDownload: downloadSchema, @@ -306,7 +370,11 @@ lastName: bar }, } if got := val.ValidateResource(resource.Resource{Bytes: testCase.rawResource}); got.Status != testCase.expect { - t.Errorf("%d - expected %d, got %d: %s", i, testCase.expect, got.Status, got.Err.Error()) + if got.Err != nil { + t.Errorf("%d - expected %d, got %d: %s", i, testCase.expect, got.Status, got.Err.Error()) + } else { + t.Errorf("%d - expected %d, got %d", i, testCase.expect, got.Status) + } } } } diff --git a/site/content/docs/usage.md b/site/content/docs/usage.md index 0aa2a0f..71ff14d 100644 --- a/site/content/docs/usage.md +++ b/site/content/docs/usage.md @@ -34,7 +34,7 @@ Usage: ./bin/kubeconform [OPTION]... [FILE OR FOLDER]... -skip string comma-separated list of kinds to ignore -strict - disallow additional properties not in schema + disallow additional properties not in schema or duplicated keys -summary print a summary at the end (ignored for junit output) -v show version information @@ -83,4 +83,4 @@ fixtures/crd_schema.yaml - CustomResourceDefinition trainingjobs.sagemaker.aws.a fixtures/invalid.yaml - ReplicationController bob is invalid: Invalid type. Expected: [integer,null], given: string [...] Summary: 65 resources found in 34 files - Valid: 55, Invalid: 2, Errors: 8 Skipped: 0 -{{< /prism >}} \ No newline at end of file +{{< /prism >}} diff --git a/site/public/docs/usage/index.html b/site/public/docs/usage/index.html index 09a2234..f8cfb21 100644 --- a/site/public/docs/usage/index.html +++ b/site/public/docs/usage/index.html @@ -59,7 +59,7 @@

Usage

-skip string comma-separated list of kinds to ignore -strict - disallow additional properties not in schema + disallow additional properties not in schema or duplicated keys -summary print a summary at the end (ignored for junit output) -v show version information @@ -117,4 +117,4 @@

Validating - \ No newline at end of file + From 937f6f4bc9b9059f703fe39665b326d98e440f15 Mon Sep 17 00:00:00 2001 From: Bill Franklin Date: Fri, 15 Jul 2022 11:39:15 +0100 Subject: [PATCH 2/2] Add acceptance tests for duplicated properties --- acceptance.bats | 10 ++++++++++ fixtures/duplicate_property.yaml | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 fixtures/duplicate_property.yaml diff --git a/acceptance.bats b/acceptance.bats index f8ec232..fde5999 100755 --- a/acceptance.bats +++ b/acceptance.bats @@ -138,6 +138,16 @@ resetCacheFolder() { [ "$status" -eq 1 ] } +@test "Fail when parsing a config with duplicate properties and strict set" { + run bin/kubeconform -strict -kubernetes-version 1.16.0 fixtures/duplicate_property.yaml + [ "$status" -eq 1 ] +} + +@test "Pass when parsing a config with duplicate properties and strict NOT set" { + run bin/kubeconform -kubernetes-version 1.16.0 fixtures/duplicate_property.yaml + [ "$status" -eq 0 ] +} + @test "Pass when using a valid, preset -schema-location" { run bin/kubeconform -schema-location default fixtures/valid.yaml [ "$status" -eq 0 ] diff --git a/fixtures/duplicate_property.yaml b/fixtures/duplicate_property.yaml new file mode 100644 index 0000000..322c145 --- /dev/null +++ b/fixtures/duplicate_property.yaml @@ -0,0 +1,18 @@ +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: nginx-ds +spec: + replicas: 2 + selector: + matchLabels: + k8s-app: nginx-ds + template: + spec: + containers: + - image: envoy + name: envoy + containers: + - image: nginx + name: nginx