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

Forbid duplicate keys in strict mode #121

Merged
merged 2 commits into from
Jul 15, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions acceptance.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down
18 changes: 18 additions & 0 deletions fixtures/duplicate_property.yaml
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
7 changes: 6 additions & 1 deletion pkg/validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
}

Expand Down
70 changes: 69 additions & 1 deletion pkg/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestValidate(t *testing.T) {
rawResource, schemaRegistry1 []byte
schemaRegistry2 []byte
ignoreMissingSchema bool
strict bool
expect Status
}{
{
Expand Down Expand Up @@ -60,6 +61,7 @@ lastName: bar
}`),
nil,
false,
false,
Valid,
},
{
Expand Down Expand Up @@ -93,6 +95,7 @@ lastName: bar
}`),
nil,
false,
false,
Invalid,
},
{
Expand Down Expand Up @@ -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(`
Expand Down Expand Up @@ -161,6 +217,7 @@ lastName: bar
}`),
nil,
false,
false,
Error,
},
{
Expand Down Expand Up @@ -196,6 +253,7 @@ lastName: bar
},
"required": ["firstName", "lastName"]
}`),
false,
false,
Valid,
},
Expand Down Expand Up @@ -232,6 +290,7 @@ lastName: bar
},
"required": ["firstName", "lastName"]
}`),
false,
false,
Valid,
},
Expand All @@ -246,6 +305,7 @@ lastName: bar
nil,
nil,
true,
false,
Skipped,
},
{
Expand All @@ -259,6 +319,7 @@ lastName: bar
nil,
nil,
false,
false,
Error,
},
{
Expand All @@ -272,6 +333,7 @@ lastName: bar
[]byte(`<html>error page</html>`),
[]byte(`<html>error page</html>`),
true,
false,
Skipped,
},
{
Expand All @@ -285,6 +347,7 @@ lastName: bar
[]byte(`<html>error page</html>`),
[]byte(`<html>error page</html>`),
false,
false,
Error,
},
} {
Expand All @@ -293,6 +356,7 @@ lastName: bar
SkipKinds: map[string]struct{}{},
RejectKinds: map[string]struct{}{},
IgnoreMissingSchemas: testCase.ignoreMissingSchema,
Strict: testCase.strict,
},
schemaCache: nil,
schemaDownload: downloadSchema,
Expand All @@ -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)
}
}
}
}
4 changes: 2 additions & 2 deletions site/content/docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 >}}
{{< /prism >}}
4 changes: 2 additions & 2 deletions site/public/docs/usage/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ <h1>Usage</h1>
-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
Expand Down Expand Up @@ -117,4 +117,4 @@ <h3 id=validating-a-folder-increasing-the-number-of-parallel-workers>Validating
</div>
<script defer src=/js/prism.js></script>
</body>
</html>
</html>