From 0600a8fd0dffdc7ec2e057924cfcbc05ec5ee1f3 Mon Sep 17 00:00:00 2001 From: Andy Baer Date: Tue, 22 Nov 2022 17:51:13 +0100 Subject: [PATCH] Validation (#255) * make use go-get * use apimachinery validation * use apimachinery validation * revert makefile changes * linter finding fix --- .golangci.yaml | 1 - api/v1/eventlogger_types.go | 6 ++-- api/v1/eventlogger_validation.go | 41 +++++++-------------------- api/v1/eventlogger_validation_test.go | 15 ++++++---- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index bd55863..7ea6ac0 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -30,7 +30,6 @@ linters: - unconvert - unparam - unused - - wastedassign linters-settings: gosec: # Exclude generated files diff --git a/api/v1/eventlogger_types.go b/api/v1/eventlogger_types.go index 4809fbc..9aacdfc 100644 --- a/api/v1/eventlogger_types.go +++ b/api/v1/eventlogger_types.go @@ -32,10 +32,10 @@ type EventLoggerSpec struct { EventTypes []string `json:"eventTypes,omitempty"` // Labels additional labels for the logger pod - Labels map[string]string `json:"labels,omitempty" validate:"k8s-label-keys,k8s-label-values"` + Labels map[string]string `json:"labels,omitempty" validate:"k8s-label-annotation-keys,k8s-label-values"` // Labels additional annotations for the logger pod - Annotations map[string]string `json:"annotations,omitempty" validate:"k8s-annotation-keys"` + Annotations map[string]string `json:"annotations,omitempty" validate:"k8s-label-annotation-keys"` // ScrapeMetrics if true, prometheus scrape annotations are added to the pod ScrapeMetrics *bool `json:"scrapeMetrics,omitempty"` @@ -52,7 +52,7 @@ type EventLoggerSpec struct { // Selector which must match a node's labels for the pod to be scheduled on that node. // More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ // +optional - NodeSelector map[string]string `json:"nodeSelector,omitempty" validate:"k8s-label-keys,k8s-label-values"` + NodeSelector map[string]string `json:"nodeSelector,omitempty" validate:"k8s-label-annotation-keys,k8s-label-values"` // LogFields fields ot the event to be logged. LogFields []LogField `json:"logFields,omitempty"` diff --git a/api/v1/eventlogger_validation.go b/api/v1/eventlogger_validation.go index 5a401e6..e2e57e8 100644 --- a/api/v1/eventlogger_validation.go +++ b/api/v1/eventlogger_validation.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "reflect" - "regexp" "strings" "github.com/bakito/k8s-event-logger-operator/version" @@ -15,12 +14,7 @@ import ( ut "github.com/go-playground/universal-translator" "github.com/go-playground/validator/v10" "github.com/go-playground/validator/v10/translations/en" -) - -var ( - annotationKeyPattern = regexp.MustCompile(`^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9](\/([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$`) - labelKeyPattern = regexp.MustCompile(`^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$`) - labelValuePattern = regexp.MustCompile(`^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$`) + "k8s.io/apimachinery/pkg/util/validation" ) // Custom type for context key, so we don't have to use 'string' directly @@ -53,7 +47,7 @@ func (in *EventLoggerSpec) Validate() error { func k8sLabelValues(_ context.Context, fl validator.FieldLevel) bool { if labels, ok := fl.Field().Interface().(map[string]string); ok { for _, v := range labels { - if !labelValuePattern.MatchString(v) { + if errs := validation.IsValidLabelValue(v); len(errs) > 0 { return false } } @@ -61,21 +55,10 @@ func k8sLabelValues(_ context.Context, fl validator.FieldLevel) bool { return true } -func k8sLabelKeys(_ context.Context, fl validator.FieldLevel) bool { +func k8sLabelAnnotationKeys(_ context.Context, fl validator.FieldLevel) bool { if annotations, ok := fl.Field().Interface().(map[string]string); ok { for k := range annotations { - if !labelKeyPattern.MatchString(k) { - return false - } - } - } - return true -} - -func k8sAnnotationKeys(_ context.Context, fl validator.FieldLevel) bool { - if annotations, ok := fl.Field().Interface().(map[string]string); ok { - for k := range annotations { - if !annotationKeyPattern.MatchString(k) { + if errs := validation.IsQualifiedName(k); len(errs) > 0 { return false } } @@ -108,9 +91,11 @@ func (err *eventLoggerValidatorError) addError(errStr string) { func newEventLoggerValidator(spec *EventLoggerSpec) *eventLoggerValidator { result := validator.New() - _ = result.RegisterValidationCtx("k8s-label-keys", k8sLabelKeys) + _ = result.RegisterValidationCtx("k8s-label-annotation-keys", k8sLabelAnnotationKeys) _ = result.RegisterValidationCtx("k8s-label-values", k8sLabelValues) - _ = result.RegisterValidationCtx("k8s-annotation-keys", k8sAnnotationKeys) + + errKey := strings.Join(validation.IsQualifiedName("a@a"), " ") + errLabelVal := strings.Join(validation.IsValidLabelValue("a:/a"), " ") // context ctx := context.WithValue(context.Background(), specKey, spec) @@ -128,16 +113,12 @@ func newEventLoggerValidator(spec *EventLoggerSpec) *eventLoggerValidator { translation string }{ { - tag: "k8s-label-keys", - translation: fmt.Sprintf("'key in {0}' must match the pattern %s", labelKeyPattern.String()), + tag: "k8s-label-annotation-keys", + translation: fmt.Sprintf("'key in {0}' must match the pattern %s", errKey), }, { tag: "k8s-label-values", - translation: fmt.Sprintf("'values in {0}' must match the pattern %s", labelValuePattern.String()), - }, - { - tag: "k8s-annotation-keys", - translation: fmt.Sprintf("'key in {0}' must match the pattern %s", annotationKeyPattern.String()), + translation: fmt.Sprintf("'values in {0}' must match the pattern %s", errLabelVal), }, } for _, t := range translations { diff --git a/api/v1/eventlogger_validation_test.go b/api/v1/eventlogger_validation_test.go index e7ecda2..7906da7 100644 --- a/api/v1/eventlogger_validation_test.go +++ b/api/v1/eventlogger_validation_test.go @@ -19,27 +19,30 @@ var _ = Describe("V1", func() { s := &v1.EventLoggerSpec{ Labels: map[string]string{"in valid": "valid"}, } - + Ω(s.Validate()).Should(HaveOccurred()) + s = &v1.EventLoggerSpec{ + Labels: map[string]string{"in:valid": "valid"}, + } Ω(s.Validate()).Should(HaveOccurred()) }) It("should have invalid label value", func() { s := &v1.EventLoggerSpec{ Labels: map[string]string{"valid": "in valid"}, } - + Ω(s.Validate()).Should(HaveOccurred()) + s = &v1.EventLoggerSpec{ + Labels: map[string]string{"valid": "in:valid"}, + } Ω(s.Validate()).Should(HaveOccurred()) }) It("should have invalid annotation key", func() { s := &v1.EventLoggerSpec{ Annotations: map[string]string{"in valid": "valid"}, } - Ω(s.Validate()).Should(HaveOccurred()) - s = &v1.EventLoggerSpec{ - Annotations: map[string]string{"in/valid/": "valid"}, + Annotations: map[string]string{"in:valid:": "valid"}, } - Ω(s.Validate()).Should(HaveOccurred()) }) })