From 92da06aa3848dad187c534e9bdd3e2652d6ac11c Mon Sep 17 00:00:00 2001 From: Chas Honton Date: Mon, 26 Aug 2024 21:01:01 -0700 Subject: [PATCH] validate ObjectMeta closes #287, #275, #286 --- acceptance.bats | 35 ++++++ fixtures/annotation_key_invalid.yaml | 8 ++ fixtures/annotation_missing_value.yaml | 8 ++ fixtures/annotation_null_value.yaml | 8 ++ fixtures/generate_name.yaml | 2 +- fixtures/label_name_length.yaml | 8 ++ fixtures/label_namespace.yaml | 8 ++ fixtures/label_value_length.yaml | 8 ++ fixtures/metadata_missing.yaml | 4 + fixtures/metadata_name_missing.yaml | 6 + fixtures/object_name-max_length.yaml | 6 + output.xml | 5 + pkg/resource/resource.go | 17 ++- pkg/validator/validator.go | 157 ++++++++++++++++++++++++- pkg/validator/validator_test.go | 4 + 15 files changed, 276 insertions(+), 8 deletions(-) create mode 100644 fixtures/annotation_key_invalid.yaml create mode 100644 fixtures/annotation_missing_value.yaml create mode 100644 fixtures/annotation_null_value.yaml create mode 100644 fixtures/label_name_length.yaml create mode 100644 fixtures/label_namespace.yaml create mode 100644 fixtures/label_value_length.yaml create mode 100644 fixtures/metadata_missing.yaml create mode 100644 fixtures/metadata_name_missing.yaml create mode 100644 fixtures/object_name-max_length.yaml create mode 100644 output.xml diff --git a/acceptance.bats b/acceptance.bats index 8a7e180..04a29b7 100755 --- a/acceptance.bats +++ b/acceptance.bats @@ -81,6 +81,41 @@ resetCacheFolder() { [ "$status" -eq 1 ] } +@test "Fail when annotation key is invalid" { + run bin/kubeconform fixtures/annotation_key_invalid.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when annotation value is missing" { + run bin/kubeconform fixtures/annotation_missing_value.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when annotation value is null" { + run bin/kubeconform fixtures/annotation_null_value.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when label name is too long" { + run bin/kubeconform fixtures/label_name_length.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when label namespace is invalid domain" { + run bin/kubeconform fixtures/label_namespace.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when label value is too long" { + run bin/kubeconform fixtures/label_value_length.yaml + [ "$status" -eq 1 ] +} + +@test "Fail when metadata name is missing" { + run bin/kubeconform fixtures/metadata_name_missing.yaml + [ "$status" -eq 1 ] +} + @test "Return relevant error for non-existent file" { run bin/kubeconform fixtures/not-here [ "$status" -eq 1 ] diff --git a/fixtures/annotation_key_invalid.yaml b/fixtures/annotation_key_invalid.yaml new file mode 100644 index 0000000..ca5500b --- /dev/null +++ b/fixtures/annotation_key_invalid.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + labels: + cert-manager.io/cluster-issuer": issue #275 +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/annotation_missing_value.yaml b/fixtures/annotation_missing_value.yaml new file mode 100644 index 0000000..48071e7 --- /dev/null +++ b/fixtures/annotation_missing_value.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + annotations: + some.domain/some-key: +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/annotation_null_value.yaml b/fixtures/annotation_null_value.yaml new file mode 100644 index 0000000..b4ffd4c --- /dev/null +++ b/fixtures/annotation_null_value.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + annotations: + some.domain/some-key: null +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/generate_name.yaml b/fixtures/generate_name.yaml index d960ee2..c4d92ef 100644 --- a/fixtures/generate_name.yaml +++ b/fixtures/generate_name.yaml @@ -1,7 +1,7 @@ apiVersion: batch/v1 kind: Job metadata: - generateName: pi- + generateName: pi spec: template: spec: diff --git a/fixtures/label_name_length.yaml b/fixtures/label_name_length.yaml new file mode 100644 index 0000000..c4d0044 --- /dev/null +++ b/fixtures/label_name_length.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + labels: + abcdefghijklmnopqrstuvwxyz-01234567890-ABCDEFGHIJKLMNOPQRSTUVWXYZ: "123456789_123456789_123456789_123456789_123456789_123456789_123" +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/label_namespace.yaml b/fixtures/label_namespace.yaml new file mode 100644 index 0000000..a5dbf33 --- /dev/null +++ b/fixtures/label_namespace.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + labels: + abcdefghijklmnopqrstuvwxyz-01234567890-ABCDEFGHIJKLMNOPQRSTUVWXYZ.example.com/ABCDEFGHIJKLMNOPQRSTUVWXYZ: "123456789_123456789_123456789_123456789_123456789_123456789_123" +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/label_value_length.yaml b/fixtures/label_value_length.yaml new file mode 100644 index 0000000..8d2aa1f --- /dev/null +++ b/fixtures/label_value_length.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: some-values + labels: + some.domain/some-key: 123456789_123456789_123456789_123456789_123456789_123456789_1234 +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/metadata_missing.yaml b/fixtures/metadata_missing.yaml new file mode 100644 index 0000000..7f1dac7 --- /dev/null +++ b/fixtures/metadata_missing.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ConfigMap +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/metadata_name_missing.yaml b/fixtures/metadata_name_missing.yaml new file mode 100644 index 0000000..3b36a28 --- /dev/null +++ b/fixtures/metadata_name_missing.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + +data: + file.name: "a value" \ No newline at end of file diff --git a/fixtures/object_name-max_length.yaml b/fixtures/object_name-max_length.yaml new file mode 100644 index 0000000..b363166 --- /dev/null +++ b/fixtures/object_name-max_length.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: abcdefghijklmnopqrstuvwxyz-01234567890-ABCDEFGHIJKLMNOPQRSTUVWXYZ +data: + file.name: "a value" \ No newline at end of file diff --git a/output.xml b/output.xml new file mode 100644 index 0000000..bf40b5e --- /dev/null +++ b/output.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index 102eabd..0f93ef5 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -13,6 +13,7 @@ type Resource struct { Bytes []byte sig *Signature // Cache signature parsing sigErr error // Cache potential signature parsing error + Metadata *ObjectMeta } // Signature is a key representing a Kubernetes resource @@ -20,6 +21,15 @@ type Signature struct { Kind, Version, Namespace, Name string } +// Metadata holds Kubernetes resource ObjectMeta +type ObjectMeta struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace"` + GenerateName string `yaml:"generateName"` + Annotations map[string]string `yaml:annotations` + Labels map[string]string `yaml:labels` +} + // GroupVersionKind returns a string with the GVK encoding of a resource signature. // This encoding slightly differs from the Kubernetes upstream implementation // in order to be suitable for being used in the kubeconform command-line arguments. @@ -41,13 +51,10 @@ func (res *Resource) Signature() (*Signature, error) { resource := struct { APIVersion string `yaml:"apiVersion"` Kind string `yaml:"kind"` - Metadata struct { - Name string `yaml:"name"` - Namespace string `yaml:"namespace"` - GenerateName string `yaml:"generateName"` - } `yaml:"Metadata"` + Metadata ObjectMeta `yaml:"metadata"` }{} err := yaml.Unmarshal(res.Bytes, &resource) + res.Metadata = &resource.Metadata name := resource.Metadata.Name if resource.Metadata.GenerateName != "" { diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index 0f8e85a..9a5883f 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "io" + "regexp" + "strings" jsonschema "github.com/santhosh-tekuri/jsonschema/v5" _ "github.com/santhosh-tekuri/jsonschema/v5/httploader" @@ -191,9 +193,9 @@ func (val *v) ValidateResource(res resource.Resource) Result { return Result{Resource: res, Err: fmt.Errorf("could not find schema for %s", sig.Kind), Status: Error} } + validationErrors := []ValidationError{} err = schema.Validate(r) if err != nil { - validationErrors := []ValidationError{} var e *jsonschema.ValidationError if errors.As(err, &e) { for _, ve := range e.Causes { @@ -202,7 +204,6 @@ func (val *v) ValidateResource(res resource.Resource) Result { Msg: ve.Message, }) } - } return Result{ Resource: res, @@ -212,6 +213,35 @@ func (val *v) ValidateResource(res resource.Resource) Result { } } + if res.Metadata != nil { + metadataPath := res.Path + " - .metadata" + namePath := metadataPath + ".name" + name := res.Metadata.Name + if name == "" { + if res.Metadata.GenerateName != "" { + name = res.Metadata.GenerateName + namePath = metadataPath + ".generateName" + } + } + + if !validateDnsLabels(name) { + validationErrors = append(validationErrors, ValidationError{ + Path: namePath, + Msg: "invalid metadata name", + }) + } + + validationErrors = validateMeta(metadataPath, res.Metadata, validationErrors) + if len(validationErrors) > 0 { + return Result{ + Resource: res, + Status: Invalid, + Err: fmt.Errorf("invalid metadata."), + ValidationErrors: validationErrors, + } + } + } + return Result{Resource: res, Status: Valid} } @@ -279,3 +309,126 @@ func downloadSchema(registries []registry.Registry, kind, version, k8sVersion st return nil, nil // No schema found - we don't consider it an error, resource will be skipped } + +func validateMeta(path string, metadata *resource.ObjectMeta, validationErrors []ValidationError) []ValidationError { + if metadata.Annotations != nil { + validationErrors = validateAnnotations(path + ".annotations", metadata.Annotations, validationErrors) + } + if metadata.Labels != nil { + validationErrors = validateLabels(path + ".labels", metadata.Labels, validationErrors) + } + return validationErrors +} + +/* Annotations are key/value pairs. */ +func validateAnnotations(path string, annotations map[string]string, validationErrors []ValidationError) []ValidationError { + for k, v := range annotations { + keypath := path + "[" + k + "]" + validationErrors = validateKey(keypath, k, validationErrors) + if !validateAnnotationValue(v) { + validationErrors = append(validationErrors, ValidationError{ + Path: keypath, + Msg: "invalid annotation value", + }) + } + } + return validationErrors +} + +/* Labels are key/value pairs. + */ +func validateLabels(path string, labels map[string]string, validationErrors []ValidationError) []ValidationError { + for k, v := range labels { + keypath := path + "[" + k + "]" + validationErrors= validateKey(keypath, k, validationErrors) + if !validateNameSegment(v) { + validationErrors = append(validationErrors, ValidationError{ + Path: keypath, + Msg: "invalid label value", + }) + } + } + return validationErrors +} + +/* Valid keys have two segments: an optional prefix and name, separated by a slash (/) +*/ +func validateKey(keypath string, key string, validationErrors []ValidationError) []ValidationError { + if len(key) == 0 { + validationErrors = append(validationErrors, ValidationError{ + Path: keypath, + Msg: "invalid annotation key", + }) + } else { + var name string + prefix, suffix, found := strings.Cut(key, "/") + if found { + name = suffix + if !validateDnsLabels(prefix) { + validationErrors = append(validationErrors, ValidationError{ + Path: keypath, + Msg: "invalid annotation key prefix", + }) + } + } else { + name = key + } + + if !validateNameSegment(name) { + validationErrors = append(validationErrors, ValidationError{ + Path: keypath, + Msg: "invalid annotation key name", + }) + + } + } + return validationErrors +} + +var alphanumericPlusUnderscorePeriodHyphen = regexp.MustCompile("^[0-9A-Za-z_.-]+$") + +func isAlphaNumeric(v byte) bool { + return (v >= '0' && v <= '9') || + (v >= 'A' && v <= 'Z') || + (v >= 'a' && v <= 'z') +} + +/* The name segment must be 63 characters or less, beginning and ending with an alphanumeric character + ([a-z0-9A-Z]) with dashes (-), underscores (_), dots (.), and alphanumerics between. +*/ +func validateNameSegment(name string) bool { + return len(name) <= 63 && + alphanumericPlusUnderscorePeriodHyphen.MatchString(name) && + isAlphaNumeric(name[0]) && + isAlphaNumeric(name[len(name)-1]) +} + +var alphanumericPlusHyphen = regexp.MustCompile("^[0-9A-Za-z-]+$") + +/* The domain name may not exceed the length of 253 characters in its textual representation. + A label may contain one to 63 characters of a through z, A through Z, digits 0 through 9, and hyphen. + Labels may not start or end with a hyphen. +*/ +func validateDnsLabels(domain string) bool { + if len(domain) == 0 || len(domain) > 253 { + return false + } else { + labels := strings.Split(domain, ".") + for _, label := range labels { + if len(label) == 0 || + len(label) > 63 || + !alphanumericPlusHyphen.MatchString(label) || + label[0] == '-' || + label[len(label)-1] == '-' { + return false + } + } + } + return true +} + +/* annotation must have value +*/ +func validateAnnotationValue(value string) bool { + return len(value) != 0 +} diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index aa86951..ec336c2 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -475,11 +475,15 @@ func TestValidateFile(t *testing.T) { inputData := []byte(` kind: name apiVersion: v1 +metadata: + name: bar.qux firstName: bar lastName: qux --- kind: name apiVersion: v1 +metadata: + name: foo firstName: foo `)