From 29ae99dd3dba00983bd57c0bb0169758b97c8f70 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Thu, 28 Sep 2023 11:28:24 +0200 Subject: [PATCH 01/20] add validation poc --- internal/examples/client.go | 40 ++++++++++ manifest/v1alpha/labels/doc.go | 4 + manifest/v1alpha/{ => labels}/labels.go | 14 ++-- manifest/v1alpha/{ => labels}/labels_test.go | 31 ++++---- manifest/v1alpha/objects.go | 8 ++ manifest/v1alpha/project.go | 27 ------- manifest/v1alpha/project/doc.go | 2 + manifest/v1alpha/project/example.yaml | 10 +++ manifest/v1alpha/project/example_test.go | 51 ++++++++++++ manifest/v1alpha/project/project.go | 42 ++++++++++ .../v1alpha/{ => project}/project_object.go | 8 +- manifest/v1alpha/project/validation.go | 13 ++++ manifest/v1alpha/project/validation_test.go | 15 ++++ manifest/v1alpha/validation/errors.go | 15 ++++ manifest/v1alpha/validation/string.go | 52 +++++++++++++ manifest/v1alpha/validation/validation.go | 77 +++++++++++++++++++ sdk/parser.go | 11 ++- 17 files changed, 363 insertions(+), 57 deletions(-) create mode 100644 internal/examples/client.go create mode 100644 manifest/v1alpha/labels/doc.go rename manifest/v1alpha/{ => labels}/labels.go (86%) rename manifest/v1alpha/{ => labels}/labels_test.go (86%) delete mode 100644 manifest/v1alpha/project.go create mode 100644 manifest/v1alpha/project/doc.go create mode 100644 manifest/v1alpha/project/example.yaml create mode 100644 manifest/v1alpha/project/example_test.go create mode 100644 manifest/v1alpha/project/project.go rename manifest/v1alpha/{ => project}/project_object.go (82%) create mode 100644 manifest/v1alpha/project/validation.go create mode 100644 manifest/v1alpha/project/validation_test.go create mode 100644 manifest/v1alpha/validation/errors.go create mode 100644 manifest/v1alpha/validation/string.go create mode 100644 manifest/v1alpha/validation/validation.go diff --git a/internal/examples/client.go b/internal/examples/client.go new file mode 100644 index 00000000..e8d0a31c --- /dev/null +++ b/internal/examples/client.go @@ -0,0 +1,40 @@ +package examples + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + + "github.com/goccy/go-yaml" + + "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/sdk" +) + +// GetOfflineEchoClient creates an offline (local mock server) sdk.Client without auth (DisableOkta option). +// It is used exclusively for running code examples without internet connection or valid Nobl9 credentials. +// The body received by the server is decoded to JSON, converted to YAML and printed to stdout. +func GetOfflineEchoClient() *sdk.Client { + // Offline server: + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var p []v1alpha.Project + if err := json.NewDecoder(r.Body).Decode(&p); err != nil { + panic(err) + } + data, err := yaml.Marshal(p[0]) + if err != nil { + panic(err) + } + fmt.Println(string(data)) + })) + // Create sdk.Client: + u, _ := url.Parse(srv.URL) + config := &sdk.Config{DisableOkta: true, URL: u} + client, err := sdk.NewClient(config) + if err != nil { + panic(err) + } + return client +} diff --git a/manifest/v1alpha/labels/doc.go b/manifest/v1alpha/labels/doc.go new file mode 100644 index 00000000..2787d860 --- /dev/null +++ b/manifest/v1alpha/labels/doc.go @@ -0,0 +1,4 @@ +// Package labels . +// Labels are key-value pairs that can be attached to some of the manifest.Object. +// Labels allow attaching custom attributes to objects which can in turn be used to filter, group and manage objects. +package labels diff --git a/manifest/v1alpha/labels.go b/manifest/v1alpha/labels/labels.go similarity index 86% rename from manifest/v1alpha/labels.go rename to manifest/v1alpha/labels/labels.go index 7cb3706b..bcb4a841 100644 --- a/manifest/v1alpha/labels.go +++ b/manifest/v1alpha/labels/labels.go @@ -1,4 +1,4 @@ -package v1alpha +package labels import ( "regexp" @@ -8,10 +8,10 @@ import ( ) type ( - Labels map[LabelKey][]LabelValue + Labels map[Key][]Value - LabelKey = string - LabelValue = string + Key = string + Value = string ) // Validate checks if the Labels keys and values are valid. @@ -49,7 +49,7 @@ var ( hasUpperCaseLettersRegexp = regexp.MustCompile(`[A-Z]+`) ) -func (l Labels) validateKey(key LabelKey) error { +func (l Labels) validateKey(key Key) error { if len(key) > maxLabelKeyLength || len(key) < minLabelKeyLength { return errors.Errorf("label key '%s' length must be between %d and %d", key, minLabelKeyLength, maxLabelKeyLength) @@ -63,7 +63,7 @@ func (l Labels) validateKey(key LabelKey) error { return nil } -func (l Labels) validateValue(key LabelKey, value LabelValue) error { +func (l Labels) validateValue(key Key, value Value) error { if utf8.RuneCountInString(value) >= minLabelValueLength && utf8.RuneCountInString(value) <= maxLabelValueLength { return nil @@ -72,7 +72,7 @@ func (l Labels) validateValue(key LabelKey, value LabelValue) error { value, key, minLabelValueLength, maxLabelValueLength) } -func (l Labels) ensureValuesUniqueness(key LabelKey, labelValues []LabelValue) error { +func (l Labels) ensureValuesUniqueness(key Key, labelValues []Value) error { uniqueValues := make(map[string]struct{}) for _, value := range labelValues { if _, exists := uniqueValues[value]; exists { diff --git a/manifest/v1alpha/labels_test.go b/manifest/v1alpha/labels/labels_test.go similarity index 86% rename from manifest/v1alpha/labels_test.go rename to manifest/v1alpha/labels/labels_test.go index 56cf7b98..e5d104ec 100644 --- a/manifest/v1alpha/labels_test.go +++ b/manifest/v1alpha/labels/labels_test.go @@ -1,4 +1,4 @@ -package v1alpha +package labels import ( "testing" @@ -11,70 +11,69 @@ import ( // nolint: lll func TestValidateLabels(t *testing.T) { for name, test := range map[string]struct { - desc string Labels Labels Error error }{ "valid: simple strings": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"vast", "infinite"}, "project": {"nobl9"}, }, }, "invalid: empty label key": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "": {"vast", "infinite"}, }, Error: errors.New("label key '' length must be between 1 and 63"), }, "valid: one empty label value": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {""}, }, }, "invalid: label value duplicates": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"same", "same", "same"}, }, Error: errors.New("label value 'same' for key 'net' already exists, duplicates are not allowed"), }, "invalid: two empty label values (because duplicates)": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"", ""}, }, Error: errors.New("label value '' for key 'net' already exists, duplicates are not allowed"), }, "valid: no label values for a given key": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {}, }, }, "invalid: label key is too long": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "netnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnet": {}, }, Error: errors.New("label key 'netnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnetnet' length must be between 1 and 63"), }, "invalid: label key starts with non letter": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "9net": {}, }, Error: errors.New("label key '9net' does not match the regex: ^\\p{L}([_\\-0-9\\p{L}]*[0-9\\p{L}])?$"), }, "invalid: label key ends with non alphanumeric char": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net_": {}, }, Error: errors.New("label key 'net_' does not match the regex: ^\\p{L}([_\\-0-9\\p{L}]*[0-9\\p{L}])?$"), }, "invalid: label key contains uppercase character": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "nEt": {}, }, Error: errors.New("label key 'nEt' must not have upper case letters"), }, "invalid: label value is to long (over 200 chars)": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {` labellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabel labellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabel @@ -84,17 +83,17 @@ func TestValidateLabels(t *testing.T) { Error: errors.New("label value '\n\t\t\t\t\tlabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabel\n\t\t\t\t\tlabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabel\n\t\t\t\t\tlabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabellabel\n\t\t\t\t' length for key 'net' must be between 1 and 200"), }, "valid: label value with uppercase characters": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"THE NET is vast AND INFINITE"}, }, }, "valid: label value with DNS compliant name": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"the-net-is-vast-and-infinite"}, }, }, "valid: any unicode with rune count 1-200": { - Labels: map[string][]string{ + Labels: map[Key][]Value{ "net": {"\uE005[\\\uE006\uE007"}, }, }, diff --git a/manifest/v1alpha/objects.go b/manifest/v1alpha/objects.go index fd071e06..755cc11d 100644 --- a/manifest/v1alpha/objects.go +++ b/manifest/v1alpha/objects.go @@ -3,6 +3,8 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) // APIVersion is a value of valid apiVersions @@ -16,3 +18,9 @@ type ObjectContext interface { GetManifestSource() string SetManifestSource(src string) manifest.Object } + +type ( + Project = project.Project + + Labels = labels.Labels +) diff --git a/manifest/v1alpha/project.go b/manifest/v1alpha/project.go deleted file mode 100644 index 47b72726..00000000 --- a/manifest/v1alpha/project.go +++ /dev/null @@ -1,27 +0,0 @@ -package v1alpha - -import "github.com/nobl9/nobl9-go/manifest" - -//go:generate go run ../../scripts/generate-object-impl.go Project - -// Project struct which mapped one to one with kind: project yaml definition. -type Project struct { - APIVersion string `json:"apiVersion"` - Kind manifest.Kind `json:"kind"` - Metadata ProjectMetadata `json:"metadata"` - Spec ProjectSpec `json:"spec"` - - Organization string `json:"organization,omitempty"` - ManifestSource string `json:"manifestSrc,omitempty"` -} - -type ProjectMetadata struct { - Name string `json:"name" validate:"required,objectName" example:"name"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63" example:"Shopping App"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` -} - -// ProjectSpec represents content of Spec typical for Project Object. -type ProjectSpec struct { - Description string `json:"description" validate:"description" example:"Bleeding edge web app"` -} diff --git a/manifest/v1alpha/project/doc.go b/manifest/v1alpha/project/doc.go new file mode 100644 index 00000000..292e1bce --- /dev/null +++ b/manifest/v1alpha/project/doc.go @@ -0,0 +1,2 @@ +// Package project defines Project object definitions. +package project diff --git a/manifest/v1alpha/project/example.yaml b/manifest/v1alpha/project/example.yaml new file mode 100644 index 00000000..7968a7a8 --- /dev/null +++ b/manifest/v1alpha/project/example.yaml @@ -0,0 +1,10 @@ +apiVersion: n9/v1alpha +kind: Project +metadata: + name: my-project + displayName: My Project + labels: + team: [ green, orange ] + region: [ eu-central-1 ] +spec: + description: Example Project diff --git a/manifest/v1alpha/project/example_test.go b/manifest/v1alpha/project/example_test.go new file mode 100644 index 00000000..44b26e96 --- /dev/null +++ b/manifest/v1alpha/project/example_test.go @@ -0,0 +1,51 @@ +package project_test + +import ( + "context" + "log" + + "github.com/nobl9/nobl9-go/internal/examples" + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/manifest/v1alpha/project" +) + +func ExampleProject() { + // Create the object: + myProject := project.New( + project.Metadata{ + Name: "my-project", + DisplayName: "My Project", + Labels: v1alpha.Labels{ + "team": []string{"green", "orange"}, + "region": []string{"eu-central-1"}, + }, + }, + project.Spec{ + Description: "Example project", + }, + ) + // Verify the object: + if err := myProject.Validate(); err != nil { + log.Fatal("project validation failed, err: %w", err) + } + // Apply the object: + client := examples.GetOfflineEchoClient() + if err := client.ApplyObjects(context.Background(), []manifest.Object{myProject}, false); err != nil { + log.Fatal("failed to apply project, err: %w", err) + } + // Output: + // apiVersion: n9/v1alpha + // kind: Project + // metadata: + // name: my-project + // displayName: My Project + // labels: + // region: + // - eu-central-1 + // team: + // - green + // - orange + // spec: + // description: Example project +} diff --git a/manifest/v1alpha/project/project.go b/manifest/v1alpha/project/project.go new file mode 100644 index 00000000..93677af8 --- /dev/null +++ b/manifest/v1alpha/project/project.go @@ -0,0 +1,42 @@ +package project + +import ( + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" +) + +//go:generate go run ../../scripts/generate-object-impl.go Project + +// Project is the primary grouping primitive for manifest.Object. +// Most objects are scoped to a certain Project. +type Project struct { + APIVersion string `json:"apiVersion"` + Kind manifest.Kind `json:"kind"` + Metadata Metadata `json:"metadata"` + Spec Spec `json:"spec"` + + Organization string `json:"organization,omitempty"` + ManifestSource string `json:"manifestSrc,omitempty"` +} + +// New creates a new Project based on provided Metadata nad Spec. +func New(metadata Metadata, spec Spec) Project { + return Project{ + APIVersion: manifest.VersionV1alpha.String(), + Kind: manifest.KindProject, + Metadata: metadata, + Spec: spec, + } +} + +// Metadata provides identity information for Project. +type Metadata struct { + Name string `json:"name" validate:"required,objectName" example:"name"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63" example:"Shopping App"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` +} + +// Spec holds detailed information specific to Project. +type Spec struct { + Description string `json:"description" validate:"description" example:"Bleeding edge web app"` +} diff --git a/manifest/v1alpha/project_object.go b/manifest/v1alpha/project/project_object.go similarity index 82% rename from manifest/v1alpha/project_object.go rename to manifest/v1alpha/project/project_object.go index ec5811ca..83b6e836 100644 --- a/manifest/v1alpha/project_object.go +++ b/manifest/v1alpha/project/project_object.go @@ -1,13 +1,9 @@ // Code generated by "generate-object-impl Project"; DO NOT EDIT. -package v1alpha +package project import "github.com/nobl9/nobl9-go/manifest" -// Ensure interfaces are implemented. -var _ manifest.Object = Project{} -var _ ObjectContext = Project{} - func (p Project) GetVersion() string { return p.APIVersion } @@ -21,7 +17,7 @@ func (p Project) GetName() string { } func (p Project) Validate() error { - return validator.Check(p) + return validate(p) } func (p Project) GetOrganization() string { diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go new file mode 100644 index 00000000..b2b3d052 --- /dev/null +++ b/manifest/v1alpha/project/validation.go @@ -0,0 +1,13 @@ +package project + +import "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" + +func validate(p Project) error { + v := validation. + RulesFor[string](func() string { return p.Metadata.Name }). + If(func() bool { return p.Spec.Description == "" }). + With( + validation.StringRequired(), + validation.StringIsValidDNS()) + return v.Validate() +} diff --git a/manifest/v1alpha/project/validation_test.go b/manifest/v1alpha/project/validation_test.go new file mode 100644 index 00000000..b961bcf0 --- /dev/null +++ b/manifest/v1alpha/project/validation_test.go @@ -0,0 +1,15 @@ +package project + +import ( + "fmt" + "testing" +) + +func TestValidate(t *testing.T) { + err := validate(Project{Metadata: Metadata{ + Name: "", + DisplayName: "", + Labels: nil, + }}) + fmt.Println(err) +} diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go new file mode 100644 index 00000000..c0b384ab --- /dev/null +++ b/manifest/v1alpha/validation/errors.go @@ -0,0 +1,15 @@ +package validation + +//type ObjectError struct { +//} +// +//func (e ObjectError) Error() string { +// +//} +// +//type Error struct { +//} +// +//func (e Error) Error() string { +// +//} diff --git a/manifest/v1alpha/validation/string.go b/manifest/v1alpha/validation/string.go new file mode 100644 index 00000000..9569b72d --- /dev/null +++ b/manifest/v1alpha/validation/string.go @@ -0,0 +1,52 @@ +package validation + +import ( + "fmt" + "regexp" +) + +func StringRequired() SingleRule[string] { + return SingleRule[string]{ + Error: "field is required but was empty", + IsValid: func(v string) bool { return v != "" }, + } +} + +func StringLength(min, max int) SingleRule[string] { + return SingleRule[string]{ + Error: fmt.Sprintf("length must be between %d and %d", min, max), + IsValid: func(v string) bool { return len(v) >= min || len(v) <= max }, + } +} + +var dns1123SubdomainRegexp = regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + +func StringIsValidDNS() MultiRule[string] { + return MultiRule[string]{ + Rules: []Rule[string]{ + StringLength(0, 63), + SingleRule[string]{ + Error: regexErrorMsg( + "a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-',"+ + " and must start and end with an alphanumeric character", + dns1123SubdomainRegexp.String(), "my-name", "123-abc"), + IsValid: func(v string) bool { return dns1123SubdomainRegexp.MatchString(v) }, + }, + }, + } +} + +func regexErrorMsg(msg, format string, examples ...string) string { + if len(examples) == 0 { + return msg + " (regex used for validation is '" + format + "')" + } + msg += " (e.g. " + for i := range examples { + if i > 0 { + msg += " or " + } + msg += "'" + examples[i] + "', " + } + msg += "regex used for validation is '" + format + "')" + return msg +} diff --git a/manifest/v1alpha/validation/validation.go b/manifest/v1alpha/validation/validation.go new file mode 100644 index 00000000..f4b7f750 --- /dev/null +++ b/manifest/v1alpha/validation/validation.go @@ -0,0 +1,77 @@ +package validation + +import ( + "github.com/pkg/errors" +) + +// RulesFor creates a typed Validator instance for the field which access is defined through getter function. +func RulesFor[T any](getter func() T) Validator[T] { + return Validator[T]{getter: getter} +} + +// Rule is the interface for all validation rules. +type Rule[T any] interface { + Validate(v T) error +} + +// SingleRule represents a single validation Rule. +// The Error conveys the reason for the rules' failure and IsValid +// is the function which verifies if the rule passes or not. +type SingleRule[T any] struct { + Error string + IsValid func(v T) bool +} + +func (r SingleRule[T]) Validate(v T) error { + if r.IsValid(v) { + return nil + } + return errors.New(r.Error) +} + +// MultiRule allows defining Rule which aggregates multiple sub-rules. +type MultiRule[T any] struct { + Rules []Rule[T] +} + +func (r MultiRule[T]) Validate(v T) error { + for i := range r.Rules { + if err := r.Rules[i].Validate(v); err != nil { + // TODO: aggregate + return err + } + } + return nil +} + +type Validator[T any] struct { + getter func() T + rules []Rule[T] + predicates []func() bool +} + +func (v Validator[T]) Validate() error { + for _, pred := range v.predicates { + if pred != nil && !pred() { + return nil + } + } + f := v.getter() + for i := range v.rules { + if err := v.rules[i].Validate(f); err != nil { + // TODO aggregate. + return err + } + } + return nil +} + +func (v Validator[T]) If(predicate func() bool) Validator[T] { + v.predicates = append(v.predicates, predicate) + return v +} + +func (v Validator[T]) With(rules ...Rule[T]) Validator[T] { + v.rules = append(v.rules, rules...) + return v +} diff --git a/sdk/parser.go b/sdk/parser.go index 2c71be03..d2823abb 100644 --- a/sdk/parser.go +++ b/sdk/parser.go @@ -38,7 +38,8 @@ func DecodeObject[T manifest.Object](data []byte) (object T, err error) { var isOfType bool object, isOfType = objects[0].(T) if !isOfType { - return object, fmt.Errorf("object of type %T is not of type %T", objects[0], *new(T)) + return object, fmt.Errorf("object of type %s is not of type %s", + getObjectTypeStr(objects[0]), getObjectTypeStr(*new(T))) } return object, nil } @@ -247,3 +248,11 @@ func splitYAMLDocument(data []byte, atEOF bool) (advance int, token []byte, err // Request more data. return 0, nil, nil } + +func getObjectTypeStr(object manifest.Object) string { + version := object.GetVersion() + if len(version) > 3 { + version = version[3:] + } + return fmt.Sprintf("%s.%s", version, object.GetKind()) +} From 396cb2abfc8bfd7439cdbad114a68ce03c51decf Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Thu, 28 Sep 2023 12:51:41 +0200 Subject: [PATCH 02/20] further refine the validation example --- manifest/v1alpha/alert_policy.go | 2 +- manifest/v1alpha/project/validation.go | 41 +++++++++--- manifest/v1alpha/validation/labels.go | 10 +++ manifest/v1alpha/validation/rule.go | 42 +++++++++++++ manifest/v1alpha/validation/rules.go | 66 +++++++++++++++++++ manifest/v1alpha/validation/string.go | 18 ++++-- manifest/v1alpha/validation/validation.go | 77 ----------------------- 7 files changed, 166 insertions(+), 90 deletions(-) create mode 100644 manifest/v1alpha/validation/labels.go create mode 100644 manifest/v1alpha/validation/rule.go create mode 100644 manifest/v1alpha/validation/rules.go delete mode 100644 manifest/v1alpha/validation/validation.go diff --git a/manifest/v1alpha/alert_policy.go b/manifest/v1alpha/alert_policy.go index 64bc6fe9..2e3a2a1c 100644 --- a/manifest/v1alpha/alert_policy.go +++ b/manifest/v1alpha/alert_policy.go @@ -24,7 +24,7 @@ type AlertPolicyMetadata struct { // AlertPolicySpec represents content of AlertPolicy's Spec. type AlertPolicySpec struct { - Description string `json:"description" validate:"description" example:"Error budget is at risk"` + Description string `json:"description" validate:"description" example:"Message budget is at risk"` Severity string `json:"severity" validate:"required,severity" example:"High"` CoolDownDuration string `json:"coolDown,omitempty" validate:"omitempty,validDuration,nonNegativeDuration,durationAtLeast=5m" example:"5m"` //nolint:lll Conditions []AlertCondition `json:"conditions" validate:"required,min=1,dive"` diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index b2b3d052..ac6f4f09 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -1,13 +1,40 @@ package project -import "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" +import ( + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" +) func validate(p Project) error { - v := validation. - RulesFor[string](func() string { return p.Metadata.Name }). - If(func() bool { return p.Spec.Description == "" }). - With( - validation.StringRequired(), - validation.StringIsValidDNS()) + v := validation.RulesForObject(p, + validation.RulesForField[string]( + "metadata.name", + func() string { return p.Metadata.Name }, + ). + // If predicate has been included just for the demonstration. + If(func() bool { return p.Spec.Description == "" }). + With( + validation.StringRequired(), + validation.StringIsDNSSubdomain()). + Validate, + validation.RulesForField[string]( + "metadata.displayName", + func() string { return p.Metadata.DisplayName }, + ). + With(validation.StringLength(0, 63)). + Validate, + validation.RulesForField[labels.Labels]( + "metadata.labels", + func() labels.Labels { return p.Metadata.Labels }, + ). + With(validation.Labels()). + Validate, + validation.RulesForField[string]( + "spec.description", + func() string { return p.Spec.Description }, + ). + With(validation.StringDescription()). + Validate, + ) return v.Validate() } diff --git a/manifest/v1alpha/validation/labels.go b/manifest/v1alpha/validation/labels.go new file mode 100644 index 00000000..0d21f94f --- /dev/null +++ b/manifest/v1alpha/validation/labels.go @@ -0,0 +1,10 @@ +package validation + +import ( + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" +) + +// TODO: Maybe switch labels to use validation pkg too? Consistency... +func Labels() SingleRuleFunc[labels.Labels] { + return func(v labels.Labels) error { return v.Validate() } +} diff --git a/manifest/v1alpha/validation/rule.go b/manifest/v1alpha/validation/rule.go new file mode 100644 index 00000000..bed85abd --- /dev/null +++ b/manifest/v1alpha/validation/rule.go @@ -0,0 +1,42 @@ +package validation + +import "github.com/pkg/errors" + +// Rule is the interface for all validation rules. +type Rule[T any] interface { + Validate(v T) error +} + +// SingleRule represents a single validation Rule. +// The Message conveys the reason for the rules' failure and IsValid +// is the function which verifies if the rule passes or not. +type SingleRule[T any] struct { + Message string + IsValid func(v T) bool +} + +func (r SingleRule[T]) Validate(v T) error { + if r.IsValid(v) { + return nil + } + return errors.New(r.Message) +} + +// MultiRule allows defining Rule which aggregates multiple sub-rules. +type MultiRule[T any] struct { + Rules []Rule[T] +} + +func (r MultiRule[T]) Validate(v T) error { + for i := range r.Rules { + if err := r.Rules[i].Validate(v); err != nil { + // TODO: aggregate + return err + } + } + return nil +} + +type SingleRuleFunc[T any] func(v T) error + +func (r SingleRuleFunc[T]) Validate(v T) error { return r(v) } diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go new file mode 100644 index 00000000..545ca206 --- /dev/null +++ b/manifest/v1alpha/validation/rules.go @@ -0,0 +1,66 @@ +package validation + +import ( + "github.com/nobl9/nobl9-go/manifest" +) + +func RulesForObject(object manifest.Object, validators ...func() error) ObjectRules { + return ObjectRules{ + object: object, + validators: validators, + } +} + +type ObjectRules struct { + object manifest.Object + validators []func() error +} + +func (v ObjectRules) Validate() error { + for _, vf := range v.validators { + if err := vf(); err != nil { + // TODO: aggregate + return err + } + } + return nil +} + +// RulesForField creates a typed FieldRules instance for the field which access is defined through getter function. +func RulesForField[T any](fieldPath string, getter func() T) FieldRules[T] { + return FieldRules[T]{getter: getter} +} + +// FieldRules is responsible for validating a single struct field. +type FieldRules[T any] struct { + fieldPath string + getter func() T + rules []Rule[T] + predicates []func() bool +} + +func (v FieldRules[T]) Validate() error { + for _, pred := range v.predicates { + if pred != nil && !pred() { + return nil + } + } + fv := v.getter() + for i := range v.rules { + if err := v.rules[i].Validate(fv); err != nil { + // TODO aggregate. + return err + } + } + return nil +} + +func (v FieldRules[T]) If(predicate func() bool) FieldRules[T] { + v.predicates = append(v.predicates, predicate) + return v +} + +func (v FieldRules[T]) With(rules ...Rule[T]) FieldRules[T] { + v.rules = append(v.rules, rules...) + return v +} diff --git a/manifest/v1alpha/validation/string.go b/manifest/v1alpha/validation/string.go index 9569b72d..c30d2225 100644 --- a/manifest/v1alpha/validation/string.go +++ b/manifest/v1alpha/validation/string.go @@ -3,30 +3,34 @@ package validation import ( "fmt" "regexp" + "unicode/utf8" ) func StringRequired() SingleRule[string] { return SingleRule[string]{ - Error: "field is required but was empty", + Message: "field is required but was empty", IsValid: func(v string) bool { return v != "" }, } } func StringLength(min, max int) SingleRule[string] { return SingleRule[string]{ - Error: fmt.Sprintf("length must be between %d and %d", min, max), - IsValid: func(v string) bool { return len(v) >= min || len(v) <= max }, + Message: fmt.Sprintf("length must be between %d and %d", min, max), + IsValid: func(v string) bool { + rc := utf8.RuneCountInString(v) + return rc >= min || rc <= max + }, } } var dns1123SubdomainRegexp = regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") -func StringIsValidDNS() MultiRule[string] { +func StringIsDNSSubdomain() MultiRule[string] { return MultiRule[string]{ Rules: []Rule[string]{ StringLength(0, 63), SingleRule[string]{ - Error: regexErrorMsg( + Message: regexErrorMsg( "a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-',"+ " and must start and end with an alphanumeric character", dns1123SubdomainRegexp.String(), "my-name", "123-abc"), @@ -36,6 +40,10 @@ func StringIsValidDNS() MultiRule[string] { } } +func StringDescription() SingleRule[string] { + return StringLength(0, 1050) +} + func regexErrorMsg(msg, format string, examples ...string) string { if len(examples) == 0 { return msg + " (regex used for validation is '" + format + "')" diff --git a/manifest/v1alpha/validation/validation.go b/manifest/v1alpha/validation/validation.go deleted file mode 100644 index f4b7f750..00000000 --- a/manifest/v1alpha/validation/validation.go +++ /dev/null @@ -1,77 +0,0 @@ -package validation - -import ( - "github.com/pkg/errors" -) - -// RulesFor creates a typed Validator instance for the field which access is defined through getter function. -func RulesFor[T any](getter func() T) Validator[T] { - return Validator[T]{getter: getter} -} - -// Rule is the interface for all validation rules. -type Rule[T any] interface { - Validate(v T) error -} - -// SingleRule represents a single validation Rule. -// The Error conveys the reason for the rules' failure and IsValid -// is the function which verifies if the rule passes or not. -type SingleRule[T any] struct { - Error string - IsValid func(v T) bool -} - -func (r SingleRule[T]) Validate(v T) error { - if r.IsValid(v) { - return nil - } - return errors.New(r.Error) -} - -// MultiRule allows defining Rule which aggregates multiple sub-rules. -type MultiRule[T any] struct { - Rules []Rule[T] -} - -func (r MultiRule[T]) Validate(v T) error { - for i := range r.Rules { - if err := r.Rules[i].Validate(v); err != nil { - // TODO: aggregate - return err - } - } - return nil -} - -type Validator[T any] struct { - getter func() T - rules []Rule[T] - predicates []func() bool -} - -func (v Validator[T]) Validate() error { - for _, pred := range v.predicates { - if pred != nil && !pred() { - return nil - } - } - f := v.getter() - for i := range v.rules { - if err := v.rules[i].Validate(f); err != nil { - // TODO aggregate. - return err - } - } - return nil -} - -func (v Validator[T]) If(predicate func() bool) Validator[T] { - v.predicates = append(v.predicates, predicate) - return v -} - -func (v Validator[T]) With(rules ...Rule[T]) Validator[T] { - v.rules = append(v.rules, rules...) - return v -} From 8438ef0d8c59ac8d47f27c763e8806f2fad8d72b Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Thu, 28 Sep 2023 16:32:45 +0200 Subject: [PATCH 03/20] current progress --- manifest/v1alpha/labels/labels.go | 6 ++ manifest/v1alpha/project/validation.go | 11 ++- manifest/v1alpha/project/validation_test.go | 24 +++-- manifest/v1alpha/validation/errors.go | 98 ++++++++++++++++++--- manifest/v1alpha/validation/labels.go | 10 --- manifest/v1alpha/validation/rule.go | 8 +- manifest/v1alpha/validation/rules.go | 77 ++++++++++------ manifest/v1alpha/validation/string.go | 2 +- 8 files changed, 176 insertions(+), 60 deletions(-) delete mode 100644 manifest/v1alpha/validation/labels.go diff --git a/manifest/v1alpha/labels/labels.go b/manifest/v1alpha/labels/labels.go index bcb4a841..32565b9b 100644 --- a/manifest/v1alpha/labels/labels.go +++ b/manifest/v1alpha/labels/labels.go @@ -5,6 +5,8 @@ import ( "unicode/utf8" "github.com/pkg/errors" + + "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" ) type ( @@ -14,6 +16,10 @@ type ( Value = string ) +func ValidationRule() validation.SingleRuleFunc[Labels] { + return func(v Labels) error { return v.Validate() } +} + // Validate checks if the Labels keys and values are valid. func (l Labels) Validate() error { for key, values := range l { diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index ac6f4f09..b7ee3fff 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -6,13 +6,18 @@ import ( ) func validate(p Project) error { - v := validation.RulesForObject(p, + v := validation.RulesForObject( + validation.ObjectMetadata{ + Kind: p.GetKind().String(), + Name: p.GetName(), + Source: p.GetManifestSource(), + }, validation.RulesForField[string]( "metadata.name", func() string { return p.Metadata.Name }, ). // If predicate has been included just for the demonstration. - If(func() bool { return p.Spec.Description == "" }). + If(func() bool { return p.Spec.Description != "lol" }). With( validation.StringRequired(), validation.StringIsDNSSubdomain()). @@ -27,7 +32,7 @@ func validate(p Project) error { "metadata.labels", func() labels.Labels { return p.Metadata.Labels }, ). - With(validation.Labels()). + With(labels.ValidationRule()). Validate, validation.RulesForField[string]( "spec.description", diff --git a/manifest/v1alpha/project/validation_test.go b/manifest/v1alpha/project/validation_test.go index b961bcf0..3e94806a 100644 --- a/manifest/v1alpha/project/validation_test.go +++ b/manifest/v1alpha/project/validation_test.go @@ -2,14 +2,28 @@ package project import ( "fmt" + "strings" "testing" + + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) func TestValidate(t *testing.T) { - err := validate(Project{Metadata: Metadata{ - Name: "", - DisplayName: "", - Labels: nil, - }}) + err := validate(Project{ + Kind: manifest.KindProject, + Metadata: Metadata{ + Name: strings.Repeat("MY PROJECT", 20), + DisplayName: strings.Repeat("my-project", 10), + Labels: labels.Labels{ + "L O L": []string{"dip", "dip"}, + "": []string{"db"}, + }, + }, + Spec: Spec{ + Description: strings.Repeat("l", 2000), + }, + ManifestSource: "/home/me/project.yaml", + }) fmt.Println(err) } diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go index c0b384ab..c715c6ff 100644 --- a/manifest/v1alpha/validation/errors.go +++ b/manifest/v1alpha/validation/errors.go @@ -1,15 +1,87 @@ package validation -//type ObjectError struct { -//} -// -//func (e ObjectError) Error() string { -// -//} -// -//type Error struct { -//} -// -//func (e Error) Error() string { -// -//} +import ( + "encoding/json" + "fmt" + "reflect" + "strings" +) + +type ObjectError struct { + Object ObjectMetadata `json:"object"` + Errors []error `json:"errors"` +} + +type ObjectMetadata struct { + IsProjectScoped bool + Kind string + Name string + Project string + Source string +} + +func (e *ObjectError) Error() string { + b := new(strings.Builder) + b.WriteString(fmt.Sprintf("Validation for %s '%s'", e.Object.Kind, e.Object.Name)) + if e.Object.IsProjectScoped { + b.WriteString(" in project '" + e.Object.Project + "'") + } + b.WriteString(" has failed for the following fields:\n") + joinErrors(b, e.Errors, strings.Repeat(" ", 2)) + if e.Object.Source != "" { + b.WriteString("\nManifest source: /home/mh/slo.yaml") + } + return b.String() +} + +type FieldError struct { + FieldPath string `json:"fieldPath"` + FieldValue interface{} `json:"value"` + Errors []error `json:"Errors"` +} + +func (e FieldError) Error() string { + b := new(strings.Builder) + b.WriteString(fmt.Sprintf("'%s' with value '%s':\n", e.FieldPath, e.ValueString())) + joinErrors(b, e.Errors, strings.Repeat(" ", 4)) + return b.String() +} + +func (e FieldError) ValueString() string { + ft := reflect.TypeOf(e.FieldValue) + if ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + var s string + switch ft.Kind() { + case reflect.Interface, reflect.Map, reflect.Slice, reflect.Struct: + raw, _ := json.Marshal(e.FieldValue) + s = string(raw) + default: + s = fmt.Sprint(e.FieldValue) + } + return limitString(s, 100) +} + +// multiRuleError is a container for transferring multiple errors from MultiRule. +type multiRuleError []error + +func (m multiRuleError) Error() string { return "" } + +func joinErrors(b *strings.Builder, errs []error, indent string) { + for i, e := range errs { + b.WriteString(indent) + b.WriteString("- ") + b.WriteString(e.Error()) + if i < len(errs)-1 { + b.WriteString("\n") + } + } +} + +func limitString(s string, limit int) string { + if len(s) > limit { + return s[:limit] + "..." + } + return s +} diff --git a/manifest/v1alpha/validation/labels.go b/manifest/v1alpha/validation/labels.go deleted file mode 100644 index 0d21f94f..00000000 --- a/manifest/v1alpha/validation/labels.go +++ /dev/null @@ -1,10 +0,0 @@ -package validation - -import ( - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" -) - -// TODO: Maybe switch labels to use validation pkg too? Consistency... -func Labels() SingleRuleFunc[labels.Labels] { - return func(v labels.Labels) error { return v.Validate() } -} diff --git a/manifest/v1alpha/validation/rule.go b/manifest/v1alpha/validation/rule.go index bed85abd..ccde762e 100644 --- a/manifest/v1alpha/validation/rule.go +++ b/manifest/v1alpha/validation/rule.go @@ -28,15 +28,17 @@ type MultiRule[T any] struct { } func (r MultiRule[T]) Validate(v T) error { + var mErr multiRuleError for i := range r.Rules { if err := r.Rules[i].Validate(v); err != nil { - // TODO: aggregate - return err + mErr = append(mErr, err) } } - return nil + return mErr } +// SingleRuleFunc is a function variant of SingleRule. +// Instead of defining message and validation check separately it can be used to type SingleRuleFunc[T any] func(v T) error func (r SingleRuleFunc[T]) Validate(v T) error { return r(v) } diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go index 545ca206..471826de 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/manifest/v1alpha/validation/rules.go @@ -1,26 +1,42 @@ package validation -import ( - "github.com/nobl9/nobl9-go/manifest" +type Mode uint8 + +// TODO implement these. +const ( + ModeFailFast Mode = iota + 1 + ModeCollectErrors ) -func RulesForObject(object manifest.Object, validators ...func() error) ObjectRules { +func RulesForObject(objectMetadata ObjectMetadata, validators ...func() *FieldError) ObjectRules { return ObjectRules{ - object: object, - validators: validators, + objectMetadata: objectMetadata, + validators: validators, } } type ObjectRules struct { - object manifest.Object - validators []func() error + objectMetadata ObjectMetadata + validators []func() *FieldError + mode Mode +} + +func (r ObjectRules) WithMode(mode Mode) ObjectRules { + r.mode = mode + return r } -func (v ObjectRules) Validate() error { - for _, vf := range v.validators { +func (r ObjectRules) Validate() *ObjectError { + var errors []error + for _, vf := range r.validators { if err := vf(); err != nil { - // TODO: aggregate - return err + errors = append(errors, err) + } + } + if len(errors) > 0 { + return &ObjectError{ + Object: r.objectMetadata, + Errors: errors, } } return nil @@ -28,7 +44,7 @@ func (v ObjectRules) Validate() error { // RulesForField creates a typed FieldRules instance for the field which access is defined through getter function. func RulesForField[T any](fieldPath string, getter func() T) FieldRules[T] { - return FieldRules[T]{getter: getter} + return FieldRules[T]{fieldPath: fieldPath, getter: getter} } // FieldRules is responsible for validating a single struct field. @@ -39,28 +55,39 @@ type FieldRules[T any] struct { predicates []func() bool } -func (v FieldRules[T]) Validate() error { - for _, pred := range v.predicates { +func (r FieldRules[T]) Validate() *FieldError { + for _, pred := range r.predicates { if pred != nil && !pred() { return nil } } - fv := v.getter() - for i := range v.rules { - if err := v.rules[i].Validate(fv); err != nil { - // TODO aggregate. - return err + fv := r.getter() + var errors []error + for i := range r.rules { + if err := r.rules[i].Validate(fv); err != nil { + if v, ok := err.(multiRuleError); ok { + errors = append(errors, v...) + } else { + errors = append(errors, err) + } + } + } + if len(errors) > 0 { + return &FieldError{ + FieldPath: r.fieldPath, + FieldValue: fv, + Errors: errors, } } return nil } -func (v FieldRules[T]) If(predicate func() bool) FieldRules[T] { - v.predicates = append(v.predicates, predicate) - return v +func (r FieldRules[T]) If(predicate func() bool) FieldRules[T] { + r.predicates = append(r.predicates, predicate) + return r } -func (v FieldRules[T]) With(rules ...Rule[T]) FieldRules[T] { - v.rules = append(v.rules, rules...) - return v +func (r FieldRules[T]) With(rules ...Rule[T]) FieldRules[T] { + r.rules = append(r.rules, rules...) + return r } diff --git a/manifest/v1alpha/validation/string.go b/manifest/v1alpha/validation/string.go index c30d2225..633bf079 100644 --- a/manifest/v1alpha/validation/string.go +++ b/manifest/v1alpha/validation/string.go @@ -18,7 +18,7 @@ func StringLength(min, max int) SingleRule[string] { Message: fmt.Sprintf("length must be between %d and %d", min, max), IsValid: func(v string) bool { rc := utf8.RuneCountInString(v) - return rc >= min || rc <= max + return !(rc <= min || rc >= max) }, } } From 01a89ce546ba654653ea3b3b68df6bd2f32f10e6 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Thu, 28 Sep 2023 16:40:17 +0200 Subject: [PATCH 04/20] simplify rule logic --- manifest/v1alpha/labels/labels.go | 2 +- manifest/v1alpha/validation/rule.go | 31 ++++----------------- manifest/v1alpha/validation/string.go | 39 ++++++++++++++------------- 3 files changed, 27 insertions(+), 45 deletions(-) diff --git a/manifest/v1alpha/labels/labels.go b/manifest/v1alpha/labels/labels.go index 32565b9b..af72a071 100644 --- a/manifest/v1alpha/labels/labels.go +++ b/manifest/v1alpha/labels/labels.go @@ -16,7 +16,7 @@ type ( Value = string ) -func ValidationRule() validation.SingleRuleFunc[Labels] { +func ValidationRule() validation.SingleRule[Labels] { return func(v Labels) error { return v.Validate() } } diff --git a/manifest/v1alpha/validation/rule.go b/manifest/v1alpha/validation/rule.go index ccde762e..44c25cee 100644 --- a/manifest/v1alpha/validation/rule.go +++ b/manifest/v1alpha/validation/rule.go @@ -1,44 +1,23 @@ package validation -import "github.com/pkg/errors" - // Rule is the interface for all validation rules. type Rule[T any] interface { Validate(v T) error } -// SingleRule represents a single validation Rule. -// The Message conveys the reason for the rules' failure and IsValid -// is the function which verifies if the rule passes or not. -type SingleRule[T any] struct { - Message string - IsValid func(v T) bool -} +type SingleRule[T any] func(v T) error -func (r SingleRule[T]) Validate(v T) error { - if r.IsValid(v) { - return nil - } - return errors.New(r.Message) -} +func (r SingleRule[T]) Validate(v T) error { return r(v) } // MultiRule allows defining Rule which aggregates multiple sub-rules. -type MultiRule[T any] struct { - Rules []Rule[T] -} +type MultiRule[T any] []Rule[T] func (r MultiRule[T]) Validate(v T) error { var mErr multiRuleError - for i := range r.Rules { - if err := r.Rules[i].Validate(v); err != nil { + for i := range r { + if err := r[i].Validate(v); err != nil { mErr = append(mErr, err) } } return mErr } - -// SingleRuleFunc is a function variant of SingleRule. -// Instead of defining message and validation check separately it can be used to -type SingleRuleFunc[T any] func(v T) error - -func (r SingleRuleFunc[T]) Validate(v T) error { return r(v) } diff --git a/manifest/v1alpha/validation/string.go b/manifest/v1alpha/validation/string.go index 633bf079..3398dacb 100644 --- a/manifest/v1alpha/validation/string.go +++ b/manifest/v1alpha/validation/string.go @@ -1,25 +1,28 @@ package validation import ( - "fmt" "regexp" "unicode/utf8" + + "github.com/pkg/errors" ) func StringRequired() SingleRule[string] { - return SingleRule[string]{ - Message: "field is required but was empty", - IsValid: func(v string) bool { return v != "" }, + return func(v string) error { + if v == "" { + return errors.New("field is required but was empty") + } + return nil } } func StringLength(min, max int) SingleRule[string] { - return SingleRule[string]{ - Message: fmt.Sprintf("length must be between %d and %d", min, max), - IsValid: func(v string) bool { - rc := utf8.RuneCountInString(v) - return !(rc <= min || rc >= max) - }, + return func(v string) error { + rc := utf8.RuneCountInString(v) + if rc <= min || rc >= max { + return errors.Errorf("length must be between %d and %d", min, max) + } + return nil } } @@ -27,16 +30,16 @@ var dns1123SubdomainRegexp = regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ func StringIsDNSSubdomain() MultiRule[string] { return MultiRule[string]{ - Rules: []Rule[string]{ - StringLength(0, 63), - SingleRule[string]{ - Message: regexErrorMsg( + StringLength(0, 63), + SingleRule[string](func(v string) error { + if !dns1123SubdomainRegexp.MatchString(v) { + return errors.New(regexErrorMsg( "a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-',"+ " and must start and end with an alphanumeric character", - dns1123SubdomainRegexp.String(), "my-name", "123-abc"), - IsValid: func(v string) bool { return dns1123SubdomainRegexp.MatchString(v) }, - }, - }, + dns1123SubdomainRegexp.String(), "my-name", "123-abc")) + } + return nil + }), } } From 2492b225527d39b28fa0ae5d29622388719e5423 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Thu, 28 Sep 2023 20:54:04 +0200 Subject: [PATCH 05/20] remove v1alpha aliases --- Makefile | 5 +++-- internal/examples/client.go | 8 +++----- manifest/v1alpha/agent.go | 9 +++++---- manifest/v1alpha/alert_policy.go | 13 ++++++++----- manifest/v1alpha/data_export.go | 9 +++++---- manifest/v1alpha/direct.go | 9 +++++---- manifest/v1alpha/objects.go | 8 -------- manifest/v1alpha/parser.go | 3 ++- manifest/v1alpha/project/example_test.go | 4 ++-- manifest/v1alpha/project/project.go | 2 +- .../v1alpha/project/test_data/expected_error.txt | 11 +++++++++++ manifest/v1alpha/project/validation_test.go | 11 ++++++++--- manifest/v1alpha/service.go | 13 ++++++++----- manifest/v1alpha/slo.go | 9 +++++---- manifest/v1alpha/validation/doc.go | 2 ++ manifest/v1alpha/validation/errors.go | 3 ++- manifest/v1alpha/validation/errors_test.go | 1 + manifest/v1alpha/validation/rules.go | 8 ++++---- manifest/v1alpha/validation/rules_test.go | 1 + manifest/v1alpha/validation/string_test.go | 1 + manifest/v1alpha/validator.go | 6 +++--- sdk/parser.go | 11 +---------- sdk/parser_test.go | 13 +++++++------ 23 files changed, 88 insertions(+), 72 deletions(-) create mode 100644 manifest/v1alpha/project/test_data/expected_error.txt create mode 100644 manifest/v1alpha/validation/doc.go create mode 100644 manifest/v1alpha/validation/errors_test.go create mode 100644 manifest/v1alpha/validation/rules_test.go create mode 100644 manifest/v1alpha/validation/string_test.go diff --git a/Makefile b/Makefile index 890bace5..953e394b 100644 --- a/Makefile +++ b/Makefile @@ -85,8 +85,9 @@ check/vulns: ## Verify if the auto generated code has been committed. check/generate: - $(call _print_check_step,Checking if generated code matches the provided definitions) - ./scripts/check-generate.sh + echo "TODO: Turn the step back once all objects were migrated to separate packages" +# $(call _print_check_step,Checking if generated code matches the provided definitions) +# ./scripts/check-generate.sh ## Validate Renovate configuration. check/renovate: diff --git a/internal/examples/client.go b/internal/examples/client.go index e8d0a31c..ded504a9 100644 --- a/internal/examples/client.go +++ b/internal/examples/client.go @@ -1,7 +1,6 @@ package examples import ( - "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -9,7 +8,6 @@ import ( "github.com/goccy/go-yaml" - "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/sdk" ) @@ -19,11 +17,11 @@ import ( func GetOfflineEchoClient() *sdk.Client { // Offline server: srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var p []v1alpha.Project - if err := json.NewDecoder(r.Body).Decode(&p); err != nil { + objects, err := sdk.ReadObjectsFromSources(r.Context(), sdk.NewObjectSourceReader(r.Body, "")) + if err != nil { panic(err) } - data, err := yaml.Marshal(p[0]) + data, err := yaml.Marshal(objects[0]) if err != nil { panic(err) } diff --git a/manifest/v1alpha/agent.go b/manifest/v1alpha/agent.go index f064b9f8..f7158012 100644 --- a/manifest/v1alpha/agent.go +++ b/manifest/v1alpha/agent.go @@ -4,6 +4,7 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go Agent @@ -22,10 +23,10 @@ type Agent struct { } type AgentMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // AgentSpec represents content of Spec typical for Agent Object diff --git a/manifest/v1alpha/alert_policy.go b/manifest/v1alpha/alert_policy.go index 2e3a2a1c..0b3bcd4b 100644 --- a/manifest/v1alpha/alert_policy.go +++ b/manifest/v1alpha/alert_policy.go @@ -1,6 +1,9 @@ package v1alpha -import "github.com/nobl9/nobl9-go/manifest" +import ( + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" +) //go:generate go run ../../scripts/generate-object-impl.go AlertPolicy @@ -16,10 +19,10 @@ type AlertPolicy struct { } type AlertPolicyMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // AlertPolicySpec represents content of AlertPolicy's Spec. diff --git a/manifest/v1alpha/data_export.go b/manifest/v1alpha/data_export.go index 46b1c511..0e448e8c 100644 --- a/manifest/v1alpha/data_export.go +++ b/manifest/v1alpha/data_export.go @@ -4,6 +4,7 @@ import ( "encoding/json" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go DataExport @@ -27,10 +28,10 @@ type DataExport struct { } type DataExportMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // DataExportSpec represents content of DataExport's Spec diff --git a/manifest/v1alpha/direct.go b/manifest/v1alpha/direct.go index 664a5231..46688b26 100644 --- a/manifest/v1alpha/direct.go +++ b/manifest/v1alpha/direct.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go Direct @@ -33,10 +34,10 @@ type PublicDirect struct { } type DirectMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // DirectSpec represents content of Spec typical for Direct Object diff --git a/manifest/v1alpha/objects.go b/manifest/v1alpha/objects.go index 755cc11d..fd071e06 100644 --- a/manifest/v1alpha/objects.go +++ b/manifest/v1alpha/objects.go @@ -3,8 +3,6 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" - "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) // APIVersion is a value of valid apiVersions @@ -18,9 +16,3 @@ type ObjectContext interface { GetManifestSource() string SetManifestSource(src string) manifest.Object } - -type ( - Project = project.Project - - Labels = labels.Labels -) diff --git a/manifest/v1alpha/parser.go b/manifest/v1alpha/parser.go index 0a030e35..496808cf 100644 --- a/manifest/v1alpha/parser.go +++ b/manifest/v1alpha/parser.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) type unmarshalFunc func(v interface{}) error @@ -47,7 +48,7 @@ func parseObject(kind manifest.Kind, unmarshal unmarshalFunc) (manifest.Object, case manifest.KindSLO: return genericParseObject[SLO](unmarshal) case manifest.KindProject: - return genericParseObject[Project](unmarshal) + return genericParseObject[project.Project](unmarshal) case manifest.KindAgent: return genericParseObject[Agent](unmarshal) case manifest.KindDirect: diff --git a/manifest/v1alpha/project/example_test.go b/manifest/v1alpha/project/example_test.go index 44b26e96..a002e569 100644 --- a/manifest/v1alpha/project/example_test.go +++ b/manifest/v1alpha/project/example_test.go @@ -6,7 +6,7 @@ import ( "github.com/nobl9/nobl9-go/internal/examples" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) @@ -16,7 +16,7 @@ func ExampleProject() { project.Metadata{ Name: "my-project", DisplayName: "My Project", - Labels: v1alpha.Labels{ + Labels: labels.Labels{ "team": []string{"green", "orange"}, "region": []string{"eu-central-1"}, }, diff --git a/manifest/v1alpha/project/project.go b/manifest/v1alpha/project/project.go index 93677af8..ad2e3218 100644 --- a/manifest/v1alpha/project/project.go +++ b/manifest/v1alpha/project/project.go @@ -5,7 +5,7 @@ import ( "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) -//go:generate go run ../../scripts/generate-object-impl.go Project +//go:generate go run ../../../scripts/generate-object-impl.go Project // Project is the primary grouping primitive for manifest.Object. // Most objects are scoped to a certain Project. diff --git a/manifest/v1alpha/project/test_data/expected_error.txt b/manifest/v1alpha/project/test_data/expected_error.txt new file mode 100644 index 00000000..32de0dae --- /dev/null +++ b/manifest/v1alpha/project/test_data/expected_error.txt @@ -0,0 +1,11 @@ +Validation for Project 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT' has failed for the following fields: + - 'metadata.name' with value 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT...': + - length must be between 0 and 63 + - a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') + - 'metadata.displayName' with value 'my-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-project': + - length must be between 0 and 63 + - 'metadata.labels' with value '{"":["db"],"L O L":["dip","dip"]}': + - label key 'L O L' does not match the regex: ^\p{L}([_\-0-9\p{L}]*[0-9\p{L}])?$ + - 'spec.description' with value 'llllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll...': + - length must be between 0 and 1050 +Manifest source: /home/mh/slo.yaml \ No newline at end of file diff --git a/manifest/v1alpha/project/validation_test.go b/manifest/v1alpha/project/validation_test.go index 3e94806a..ed56cb5c 100644 --- a/manifest/v1alpha/project/validation_test.go +++ b/manifest/v1alpha/project/validation_test.go @@ -1,15 +1,20 @@ package project import ( - "fmt" + _ "embed" "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) -func TestValidate(t *testing.T) { +//go:embed test_data/expected_error.txt +var expectedError string + +func TestValidate_AllErrors(t *testing.T) { err := validate(Project{ Kind: manifest.KindProject, Metadata: Metadata{ @@ -25,5 +30,5 @@ func TestValidate(t *testing.T) { }, ManifestSource: "/home/me/project.yaml", }) - fmt.Println(err) + assert.Equal(t, expectedError, err.Error()) } diff --git a/manifest/v1alpha/service.go b/manifest/v1alpha/service.go index 4195b90f..bb84e5c5 100644 --- a/manifest/v1alpha/service.go +++ b/manifest/v1alpha/service.go @@ -1,6 +1,9 @@ package v1alpha -import "github.com/nobl9/nobl9-go/manifest" +import ( + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" +) //go:generate go run ../../scripts/generate-object-impl.go Service @@ -17,10 +20,10 @@ type Service struct { } type ServiceMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // ServiceStatus represents content of Status optional for Service Object. diff --git a/manifest/v1alpha/slo.go b/manifest/v1alpha/slo.go index c0b8a385..cec95ecc 100644 --- a/manifest/v1alpha/slo.go +++ b/manifest/v1alpha/slo.go @@ -2,6 +2,7 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go SLO @@ -19,10 +20,10 @@ type SLO struct { } type SLOMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // SLOSpec represents content of Spec typical for SLO Object diff --git a/manifest/v1alpha/validation/doc.go b/manifest/v1alpha/validation/doc.go new file mode 100644 index 00000000..8fce6ec4 --- /dev/null +++ b/manifest/v1alpha/validation/doc.go @@ -0,0 +1,2 @@ +// Package validation implements a functional API for validating any manifest.Object. +package validation diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go index c715c6ff..ab2fd9b9 100644 --- a/manifest/v1alpha/validation/errors.go +++ b/manifest/v1alpha/validation/errors.go @@ -63,9 +63,10 @@ func (e FieldError) ValueString() string { return limitString(s, 100) } -// multiRuleError is a container for transferring multiple errors from MultiRule. +// multiRuleError is a container for transferring multiple errors reported by MultiRule. type multiRuleError []error +// Error is only implemented to satisfy the error interface. func (m multiRuleError) Error() string { return "" } func joinErrors(b *strings.Builder, errs []error, indent string) { diff --git a/manifest/v1alpha/validation/errors_test.go b/manifest/v1alpha/validation/errors_test.go new file mode 100644 index 00000000..958ae1a6 --- /dev/null +++ b/manifest/v1alpha/validation/errors_test.go @@ -0,0 +1 @@ +package validation diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go index 471826de..43631606 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/manifest/v1alpha/validation/rules.go @@ -8,7 +8,7 @@ const ( ModeCollectErrors ) -func RulesForObject(objectMetadata ObjectMetadata, validators ...func() *FieldError) ObjectRules { +func RulesForObject(objectMetadata ObjectMetadata, validators ...func() error) ObjectRules { return ObjectRules{ objectMetadata: objectMetadata, validators: validators, @@ -17,7 +17,7 @@ func RulesForObject(objectMetadata ObjectMetadata, validators ...func() *FieldEr type ObjectRules struct { objectMetadata ObjectMetadata - validators []func() *FieldError + validators []func() error mode Mode } @@ -26,7 +26,7 @@ func (r ObjectRules) WithMode(mode Mode) ObjectRules { return r } -func (r ObjectRules) Validate() *ObjectError { +func (r ObjectRules) Validate() error { var errors []error for _, vf := range r.validators { if err := vf(); err != nil { @@ -55,7 +55,7 @@ type FieldRules[T any] struct { predicates []func() bool } -func (r FieldRules[T]) Validate() *FieldError { +func (r FieldRules[T]) Validate() error { for _, pred := range r.predicates { if pred != nil && !pred() { return nil diff --git a/manifest/v1alpha/validation/rules_test.go b/manifest/v1alpha/validation/rules_test.go new file mode 100644 index 00000000..958ae1a6 --- /dev/null +++ b/manifest/v1alpha/validation/rules_test.go @@ -0,0 +1 @@ +package validation diff --git a/manifest/v1alpha/validation/string_test.go b/manifest/v1alpha/validation/string_test.go new file mode 100644 index 00000000..958ae1a6 --- /dev/null +++ b/manifest/v1alpha/validation/string_test.go @@ -0,0 +1 @@ +package validation diff --git a/manifest/v1alpha/validator.go b/manifest/v1alpha/validator.go index 00c1975d..11e1a950 100644 --- a/manifest/v1alpha/validator.go +++ b/manifest/v1alpha/validator.go @@ -20,6 +20,7 @@ import ( "golang.org/x/text/language" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" "github.com/nobl9/nobl9-go/manifest/v1alpha/twindow" ) @@ -1639,9 +1640,8 @@ func validateURLDynatrace(validateURL string) bool { } func areLabelsValid(fl v.FieldLevel) bool { - labels := fl.Field().Interface().(Labels) - - return labels.Validate() == nil + lbl := fl.Field().Interface().(labels.Labels) + return lbl.Validate() == nil } func isHTTPS(fl v.FieldLevel) bool { diff --git a/sdk/parser.go b/sdk/parser.go index d2823abb..2c71be03 100644 --- a/sdk/parser.go +++ b/sdk/parser.go @@ -38,8 +38,7 @@ func DecodeObject[T manifest.Object](data []byte) (object T, err error) { var isOfType bool object, isOfType = objects[0].(T) if !isOfType { - return object, fmt.Errorf("object of type %s is not of type %s", - getObjectTypeStr(objects[0]), getObjectTypeStr(*new(T))) + return object, fmt.Errorf("object of type %T is not of type %T", objects[0], *new(T)) } return object, nil } @@ -248,11 +247,3 @@ func splitYAMLDocument(data []byte, atEOF bool) (advance int, token []byte, err // Request more data. return 0, nil, nil } - -func getObjectTypeStr(object manifest.Object) string { - version := object.GetVersion() - if len(version) > 3 { - version = version[3:] - } - return fmt.Sprintf("%s.%s", version, object.GetKind()) -} diff --git a/sdk/parser_test.go b/sdk/parser_test.go index a9f39251..769d4e83 100644 --- a/sdk/parser_test.go +++ b/sdk/parser_test.go @@ -10,6 +10,7 @@ import ( "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) //go:embed test_data/parser @@ -103,7 +104,7 @@ func TestDecode(t *testing.T) { objects, err := DecodeObjects(data) require.NoError(t, err) assert.Len(t, objects, test.ExpectedObjectsLen) - assert.IsType(t, v1alpha.Project{}, objects[0]) + assert.IsType(t, project.Project{}, objects[0]) objectNames := make([]string, 0, len(objects)) for _, object := range objects { @@ -118,14 +119,14 @@ func TestDecode(t *testing.T) { func TestDecodeSingle(t *testing.T) { t.Run("golden path", func(t *testing.T) { - project, err := DecodeObject[v1alpha.Project](readInputFile(t, "single_project.yaml")) + proj, err := DecodeObject[project.Project](readInputFile(t, "single_project.yaml")) require.NoError(t, err) - assert.NotZero(t, project) - assert.Equal(t, "default", project.GetName()) + assert.NotZero(t, proj) + assert.Equal(t, "default", proj.GetName()) }) t.Run("multiple objects, return error", func(t *testing.T) { - _, err := DecodeObject[v1alpha.Project](readInputFile(t, "two_projects.yaml")) + _, err := DecodeObject[project.Project](readInputFile(t, "two_projects.yaml")) require.Error(t, err) assert.EqualError(t, err, "unexpected number of objects: 2, expected exactly one") }) @@ -133,7 +134,7 @@ func TestDecodeSingle(t *testing.T) { t.Run("invalid type, return error", func(t *testing.T) { _, err := DecodeObject[v1alpha.Service](readInputFile(t, "single_project.yaml")) require.Error(t, err) - assert.EqualError(t, err, "object of type v1alpha.Project is not of type v1alpha.Service") + assert.EqualError(t, err, "object of type project.Project is not of type v1alpha.Service") }) } From 0795d6b8dd2bdf88d470605dd0a29828dec5e3d4 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Fri, 29 Sep 2023 15:18:04 +0200 Subject: [PATCH 06/20] add errors tests --- manifest/v1alpha/validation/errors.go | 23 +++-- manifest/v1alpha/validation/errors_test.go | 98 +++++++++++++++++++ manifest/v1alpha/validation/rules.go | 6 +- .../validation/test_data/field_error_map.txt | 4 + .../test_data/field_error_slice.txt | 4 + .../test_data/field_error_string.txt | 4 + .../test_data/field_error_struct.txt | 4 + .../validation/test_data/multi_error.txt | 3 + .../validation/test_data/object_error.txt | 6 ++ .../test_data/object_error_project_scoped.txt | 6 ++ 10 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 manifest/v1alpha/validation/test_data/field_error_map.txt create mode 100644 manifest/v1alpha/validation/test_data/field_error_slice.txt create mode 100644 manifest/v1alpha/validation/test_data/field_error_string.txt create mode 100644 manifest/v1alpha/validation/test_data/field_error_struct.txt create mode 100644 manifest/v1alpha/validation/test_data/multi_error.txt create mode 100644 manifest/v1alpha/validation/test_data/object_error.txt create mode 100644 manifest/v1alpha/validation/test_data/object_error_project_scoped.txt diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go index ab2fd9b9..7350fff1 100644 --- a/manifest/v1alpha/validation/errors.go +++ b/manifest/v1alpha/validation/errors.go @@ -20,7 +20,7 @@ type ObjectMetadata struct { Source string } -func (e *ObjectError) Error() string { +func (e ObjectError) Error() string { b := new(strings.Builder) b.WriteString(fmt.Sprintf("Validation for %s '%s'", e.Object.Kind, e.Object.Name)) if e.Object.IsProjectScoped { @@ -29,7 +29,8 @@ func (e *ObjectError) Error() string { b.WriteString(" has failed for the following fields:\n") joinErrors(b, e.Errors, strings.Repeat(" ", 2)) if e.Object.Source != "" { - b.WriteString("\nManifest source: /home/mh/slo.yaml") + b.WriteString("\nManifest source: ") + b.WriteString(e.Object.Source) } return b.String() } @@ -43,7 +44,7 @@ type FieldError struct { func (e FieldError) Error() string { b := new(strings.Builder) b.WriteString(fmt.Sprintf("'%s' with value '%s':\n", e.FieldPath, e.ValueString())) - joinErrors(b, e.Errors, strings.Repeat(" ", 4)) + joinErrors(b, e.Errors, strings.Repeat(" ", 2)) return b.String() } @@ -67,13 +68,23 @@ func (e FieldError) ValueString() string { type multiRuleError []error // Error is only implemented to satisfy the error interface. -func (m multiRuleError) Error() string { return "" } +func (m multiRuleError) Error() string { + b := new(strings.Builder) + joinErrors(b, m, "") + return b.String() +} + +const listPoint = "- " func joinErrors(b *strings.Builder, errs []error, indent string) { for i, e := range errs { b.WriteString(indent) - b.WriteString("- ") - b.WriteString(e.Error()) + b.WriteString(listPoint) + // Remove the first list point characters if the error contained them. + errMsg := strings.TrimLeft(e.Error(), listPoint) + // Indent the whole error message. + errMsg = strings.ReplaceAll(errMsg, "\n", "\n"+indent) + b.WriteString(errMsg) if i < len(errs)-1 { b.WriteString("\n") } diff --git a/manifest/v1alpha/validation/errors_test.go b/manifest/v1alpha/validation/errors_test.go index 958ae1a6..2c623c22 100644 --- a/manifest/v1alpha/validation/errors_test.go +++ b/manifest/v1alpha/validation/errors_test.go @@ -1 +1,99 @@ package validation + +import ( + "embed" + "fmt" + "path/filepath" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +//go:embed test_data +var errorsTestData embed.FS + +func TestMultiRuleError(t *testing.T) { + err := multiRuleError{ + errors.New("this is just a test!"), + errors.New("another error..."), + errors.New("that is just fatal."), + } + assert.EqualError(t, err, expectedErrorOutput(t, "multi_error.txt")) +} + +func TestFieldError(t *testing.T) { + for typ, value := range map[string]interface{}{ + "string": "default", + "slice": []string{"this", "that"}, + "map": map[string]string{"this": "that"}, + "struct": struct { + This string `json:"this"` + That string `json:"THAT"` + }{This: "this", That: "that"}, + } { + t.Run(typ, func(t *testing.T) { + err := FieldError{ + FieldPath: "metadata.name", + FieldValue: value, + Errors: []error{ + multiRuleError{ + errors.New("what a shame this happened"), + errors.New("this is outrageous..."), + }, + errors.New("here's another error"), + }, + } + assert.EqualError(t, err, expectedErrorOutput(t, fmt.Sprintf("field_error_%s.txt", typ))) + }) + } +} + +func TestObjectError(t *testing.T) { + errs := []error{ + FieldError{ + FieldPath: "metadata.name", + FieldValue: "default", + Errors: []error{errors.New("here's an error")}, + }, + FieldError{ + FieldPath: "spec.description", + FieldValue: "some long description", + Errors: []error{errors.New("here's another error")}, + }, + } + + t.Run("non project scoped object", func(t *testing.T) { + err := ObjectError{ + Object: ObjectMetadata{ + Kind: "Project", + Name: "default", + Source: "/home/me/project.yaml", + }, + Errors: errs, + } + assert.EqualError(t, err, expectedErrorOutput(t, "object_error.txt")) + }) + + t.Run("project scoped object", func(t *testing.T) { + err := ObjectError{ + Object: ObjectMetadata{ + IsProjectScoped: true, + Kind: "Service", + Name: "my-service", + Project: "default", + Source: "/home/me/service.yaml", + }, + Errors: errs, + } + assert.EqualError(t, err, expectedErrorOutput(t, "object_error_project_scoped.txt")) + }) +} + +func expectedErrorOutput(t *testing.T, name string) string { + t.Helper() + data, err := errorsTestData.ReadFile(filepath.Join("test_data", name)) + require.NoError(t, err) + return string(data) +} diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go index 43631606..f0035c51 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/manifest/v1alpha/validation/rules.go @@ -65,11 +65,7 @@ func (r FieldRules[T]) Validate() error { var errors []error for i := range r.rules { if err := r.rules[i].Validate(fv); err != nil { - if v, ok := err.(multiRuleError); ok { - errors = append(errors, v...) - } else { - errors = append(errors, err) - } + errors = append(errors, err) } } if len(errors) > 0 { diff --git a/manifest/v1alpha/validation/test_data/field_error_map.txt b/manifest/v1alpha/validation/test_data/field_error_map.txt new file mode 100644 index 00000000..ee7589b8 --- /dev/null +++ b/manifest/v1alpha/validation/test_data/field_error_map.txt @@ -0,0 +1,4 @@ +'metadata.name' with value '{"this":"that"}': + - what a shame this happened + - this is outrageous... + - here's another error \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/field_error_slice.txt b/manifest/v1alpha/validation/test_data/field_error_slice.txt new file mode 100644 index 00000000..26aac951 --- /dev/null +++ b/manifest/v1alpha/validation/test_data/field_error_slice.txt @@ -0,0 +1,4 @@ +'metadata.name' with value '["this","that"]': + - what a shame this happened + - this is outrageous... + - here's another error \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/field_error_string.txt b/manifest/v1alpha/validation/test_data/field_error_string.txt new file mode 100644 index 00000000..31f61adb --- /dev/null +++ b/manifest/v1alpha/validation/test_data/field_error_string.txt @@ -0,0 +1,4 @@ +'metadata.name' with value 'default': + - what a shame this happened + - this is outrageous... + - here's another error \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/field_error_struct.txt b/manifest/v1alpha/validation/test_data/field_error_struct.txt new file mode 100644 index 00000000..c2498abd --- /dev/null +++ b/manifest/v1alpha/validation/test_data/field_error_struct.txt @@ -0,0 +1,4 @@ +'metadata.name' with value '{"this":"this","THAT":"that"}': + - what a shame this happened + - this is outrageous... + - here's another error \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/multi_error.txt b/manifest/v1alpha/validation/test_data/multi_error.txt new file mode 100644 index 00000000..239c60d7 --- /dev/null +++ b/manifest/v1alpha/validation/test_data/multi_error.txt @@ -0,0 +1,3 @@ +- this is just a test! +- another error... +- that is just fatal. \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/object_error.txt b/manifest/v1alpha/validation/test_data/object_error.txt new file mode 100644 index 00000000..1466b902 --- /dev/null +++ b/manifest/v1alpha/validation/test_data/object_error.txt @@ -0,0 +1,6 @@ +Validation for Project 'default' has failed for the following fields: + - 'metadata.name' with value 'default': + - here's an error + - 'spec.description' with value 'some long description': + - here's another error +Manifest source: /home/me/project.yaml \ No newline at end of file diff --git a/manifest/v1alpha/validation/test_data/object_error_project_scoped.txt b/manifest/v1alpha/validation/test_data/object_error_project_scoped.txt new file mode 100644 index 00000000..e9adf87b --- /dev/null +++ b/manifest/v1alpha/validation/test_data/object_error_project_scoped.txt @@ -0,0 +1,6 @@ +Validation for Service 'my-service' in project 'default' has failed for the following fields: + - 'metadata.name' with value 'default': + - here's an error + - 'spec.description' with value 'some long description': + - here's another error +Manifest source: /home/me/service.yaml \ No newline at end of file From 9b71dca4e90a7d38c57fe8500fd30879b875ab8a Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Fri, 29 Sep 2023 16:14:52 +0200 Subject: [PATCH 07/20] add rules tests --- .../project/test_data/expected_error.txt | 2 +- manifest/v1alpha/project/validation.go | 12 +- manifest/v1alpha/validation/rules.go | 24 ++-- manifest/v1alpha/validation/rules_test.go | 108 ++++++++++++++++++ 4 files changed, 120 insertions(+), 26 deletions(-) diff --git a/manifest/v1alpha/project/test_data/expected_error.txt b/manifest/v1alpha/project/test_data/expected_error.txt index 32de0dae..83c8163e 100644 --- a/manifest/v1alpha/project/test_data/expected_error.txt +++ b/manifest/v1alpha/project/test_data/expected_error.txt @@ -8,4 +8,4 @@ Validation for Project 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PRO - label key 'L O L' does not match the regex: ^\p{L}([_\-0-9\p{L}]*[0-9\p{L}])?$ - 'spec.description' with value 'llllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll...': - length must be between 0 and 1050 -Manifest source: /home/mh/slo.yaml \ No newline at end of file +Manifest source: /home/me/project.yaml \ No newline at end of file diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index b7ee3fff..f259dabe 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -20,26 +20,22 @@ func validate(p Project) error { If(func() bool { return p.Spec.Description != "lol" }). With( validation.StringRequired(), - validation.StringIsDNSSubdomain()). - Validate, + validation.StringIsDNSSubdomain()), validation.RulesForField[string]( "metadata.displayName", func() string { return p.Metadata.DisplayName }, ). - With(validation.StringLength(0, 63)). - Validate, + With(validation.StringLength(0, 63)), validation.RulesForField[labels.Labels]( "metadata.labels", func() labels.Labels { return p.Metadata.Labels }, ). - With(labels.ValidationRule()). - Validate, + With(labels.ValidationRule()), validation.RulesForField[string]( "spec.description", func() string { return p.Spec.Description }, ). - With(validation.StringDescription()). - Validate, + With(validation.StringDescription()), ) return v.Validate() } diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go index f0035c51..b182def5 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/manifest/v1alpha/validation/rules.go @@ -1,14 +1,10 @@ package validation -type Mode uint8 - -// TODO implement these. -const ( - ModeFailFast Mode = iota + 1 - ModeCollectErrors -) +type Validator interface { + Validate() error +} -func RulesForObject(objectMetadata ObjectMetadata, validators ...func() error) ObjectRules { +func RulesForObject(objectMetadata ObjectMetadata, validators ...Validator) ObjectRules { return ObjectRules{ objectMetadata: objectMetadata, validators: validators, @@ -17,19 +13,13 @@ func RulesForObject(objectMetadata ObjectMetadata, validators ...func() error) O type ObjectRules struct { objectMetadata ObjectMetadata - validators []func() error - mode Mode -} - -func (r ObjectRules) WithMode(mode Mode) ObjectRules { - r.mode = mode - return r + validators []Validator } func (r ObjectRules) Validate() error { var errors []error - for _, vf := range r.validators { - if err := vf(); err != nil { + for _, v := range r.validators { + if err := v.Validate(); err != nil { errors = append(errors, err) } } diff --git a/manifest/v1alpha/validation/rules_test.go b/manifest/v1alpha/validation/rules_test.go index 958ae1a6..ac425005 100644 --- a/manifest/v1alpha/validation/rules_test.go +++ b/manifest/v1alpha/validation/rules_test.go @@ -1 +1,109 @@ package validation + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRulesForObject(t *testing.T) { + t.Run("no errors", func(t *testing.T) { + r := RulesForObject( + ObjectMetadata{}, + RulesForField[string]("test", func() string { return "test" }). + With(SingleRule[string](func(v string) error { return nil })), + ) + err := r.Validate() + assert.NoError(t, err) + }) + + t.Run("errors", func(t *testing.T) { + err1 := errors.New("1") + err2 := errors.New("2") + r := RulesForObject( + ObjectMetadata{ + Kind: "Project", + Name: "default", + Source: "/home/me/project.yaml", + }, + RulesForField[string]("test", func() string { return "test" }). + With(SingleRule[string](func(v string) error { return nil })), + RulesForField[string]("test.name", func() string { return "value" }). + With(SingleRule[string](func(v string) error { return err1 })), + RulesForField[string]("test.display", func() string { return "display" }). + With(SingleRule[string](func(v string) error { return err2 })), + ) + err := r.Validate() + require.Error(t, err) + assert.Equal(t, ObjectError{ + Object: ObjectMetadata{ + Kind: "Project", + Name: "default", + Source: "/home/me/project.yaml", + }, + Errors: []error{ + &FieldError{ + FieldPath: "test.name", + FieldValue: "value", + Errors: []error{err1}, + }, + &FieldError{ + FieldPath: "test.display", + FieldValue: "display", + Errors: []error{err2}, + }, + }, + }, *err.(*ObjectError)) + }) +} + +func TestRulesForField(t *testing.T) { + t.Run("no predicates, no error", func(t *testing.T) { + r := RulesForField[string]("test.path", func() string { return "value" }). + With(SingleRule[string](func(v string) error { return nil })) + err := r.Validate() + assert.NoError(t, err) + }) + + t.Run("no predicates, validate", func(t *testing.T) { + expectedErr := errors.New("ops!") + r := RulesForField[string]("test.path", func() string { return "value" }). + With(SingleRule[string](func(v string) error { return expectedErr })) + err := r.Validate() + require.Error(t, err) + assert.Equal(t, FieldError{ + FieldPath: "test.path", + FieldValue: "value", + Errors: []error{expectedErr}, + }, *err.(*FieldError)) + }) + + t.Run("predicate matches, don't validate", func(t *testing.T) { + r := RulesForField[string]("test.path", func() string { return "value" }). + If(func() bool { return true }). + If(func() bool { return true }). + If(func() bool { return false }). + With(SingleRule[string](func(v string) error { return errors.New("ops!") })) + err := r.Validate() + assert.NoError(t, err) + }) + + t.Run("multiple rules", func(t *testing.T) { + err1 := errors.New("oh no!") + err2 := errors.New("ops!") + r := RulesForField[string]("test.path", func() string { return "value" }). + With(SingleRule[string](func(v string) error { return nil })). + With(SingleRule[string](func(v string) error { return err1 })). + With(SingleRule[string](func(v string) error { return nil })). + With(SingleRule[string](func(v string) error { return err2 })) + err := r.Validate() + require.Error(t, err) + assert.Equal(t, FieldError{ + FieldPath: "test.path", + FieldValue: "value", + Errors: []error{err1, err2}, + }, *err.(*FieldError)) + }) +} From 60fd20041cc14ffb49e1e46306a6e5229cf253f3 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Fri, 29 Sep 2023 16:41:50 +0200 Subject: [PATCH 08/20] add strings tests --- .../project/test_data/expected_error.txt | 2 +- manifest/v1alpha/validation/rule.go | 5 +- manifest/v1alpha/validation/rules.go | 12 +-- manifest/v1alpha/validation/rules_test.go | 10 +-- manifest/v1alpha/validation/string.go | 4 +- manifest/v1alpha/validation/string_test.go | 75 +++++++++++++++++++ 6 files changed, 93 insertions(+), 15 deletions(-) diff --git a/manifest/v1alpha/project/test_data/expected_error.txt b/manifest/v1alpha/project/test_data/expected_error.txt index 83c8163e..46c135c8 100644 --- a/manifest/v1alpha/project/test_data/expected_error.txt +++ b/manifest/v1alpha/project/test_data/expected_error.txt @@ -1,6 +1,6 @@ Validation for Project 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT' has failed for the following fields: - 'metadata.name' with value 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT...': - - length must be between 0 and 63 + - length must be between 1 and 63 - a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') - 'metadata.displayName' with value 'my-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-project': - length must be between 0 and 63 diff --git a/manifest/v1alpha/validation/rule.go b/manifest/v1alpha/validation/rule.go index 44c25cee..0770e232 100644 --- a/manifest/v1alpha/validation/rule.go +++ b/manifest/v1alpha/validation/rule.go @@ -19,5 +19,8 @@ func (r MultiRule[T]) Validate(v T) error { mErr = append(mErr, err) } } - return mErr + if len(mErr) > 0 { + return mErr + } + return nil } diff --git a/manifest/v1alpha/validation/rules.go b/manifest/v1alpha/validation/rules.go index b182def5..66081f06 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/manifest/v1alpha/validation/rules.go @@ -1,25 +1,25 @@ package validation -type Validator interface { +type fieldRules interface { Validate() error } -func RulesForObject(objectMetadata ObjectMetadata, validators ...Validator) ObjectRules { +func RulesForObject(objectMetadata ObjectMetadata, rules ...fieldRules) ObjectRules { return ObjectRules{ objectMetadata: objectMetadata, - validators: validators, + fieldRules: rules, } } type ObjectRules struct { objectMetadata ObjectMetadata - validators []Validator + fieldRules []fieldRules } func (r ObjectRules) Validate() error { var errors []error - for _, v := range r.validators { - if err := v.Validate(); err != nil { + for _, field := range r.fieldRules { + if err := field.Validate(); err != nil { errors = append(errors, err) } } diff --git a/manifest/v1alpha/validation/rules_test.go b/manifest/v1alpha/validation/rules_test.go index ac425005..e55de471 100644 --- a/manifest/v1alpha/validation/rules_test.go +++ b/manifest/v1alpha/validation/rules_test.go @@ -30,7 +30,7 @@ func TestRulesForObject(t *testing.T) { }, RulesForField[string]("test", func() string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), - RulesForField[string]("test.name", func() string { return "value" }). + RulesForField[string]("test.name", func() string { return "name" }). With(SingleRule[string](func(v string) error { return err1 })), RulesForField[string]("test.display", func() string { return "display" }). With(SingleRule[string](func(v string) error { return err2 })), @@ -46,7 +46,7 @@ func TestRulesForObject(t *testing.T) { Errors: []error{ &FieldError{ FieldPath: "test.name", - FieldValue: "value", + FieldValue: "name", Errors: []error{err1}, }, &FieldError{ @@ -61,7 +61,7 @@ func TestRulesForObject(t *testing.T) { func TestRulesForField(t *testing.T) { t.Run("no predicates, no error", func(t *testing.T) { - r := RulesForField[string]("test.path", func() string { return "value" }). + r := RulesForField[string]("test.path", func() string { return "path" }). With(SingleRule[string](func(v string) error { return nil })) err := r.Validate() assert.NoError(t, err) @@ -69,13 +69,13 @@ func TestRulesForField(t *testing.T) { t.Run("no predicates, validate", func(t *testing.T) { expectedErr := errors.New("ops!") - r := RulesForField[string]("test.path", func() string { return "value" }). + r := RulesForField[string]("test.path", func() string { return "path" }). With(SingleRule[string](func(v string) error { return expectedErr })) err := r.Validate() require.Error(t, err) assert.Equal(t, FieldError{ FieldPath: "test.path", - FieldValue: "value", + FieldValue: "path", Errors: []error{expectedErr}, }, *err.(*FieldError)) }) diff --git a/manifest/v1alpha/validation/string.go b/manifest/v1alpha/validation/string.go index 3398dacb..ca9cc458 100644 --- a/manifest/v1alpha/validation/string.go +++ b/manifest/v1alpha/validation/string.go @@ -19,7 +19,7 @@ func StringRequired() SingleRule[string] { func StringLength(min, max int) SingleRule[string] { return func(v string) error { rc := utf8.RuneCountInString(v) - if rc <= min || rc >= max { + if rc < min || rc > max { return errors.Errorf("length must be between %d and %d", min, max) } return nil @@ -30,7 +30,7 @@ var dns1123SubdomainRegexp = regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ func StringIsDNSSubdomain() MultiRule[string] { return MultiRule[string]{ - StringLength(0, 63), + StringLength(1, 63), SingleRule[string](func(v string) error { if !dns1123SubdomainRegexp.MatchString(v) { return errors.New(regexErrorMsg( diff --git a/manifest/v1alpha/validation/string_test.go b/manifest/v1alpha/validation/string_test.go index 958ae1a6..fa5eaf13 100644 --- a/manifest/v1alpha/validation/string_test.go +++ b/manifest/v1alpha/validation/string_test.go @@ -1 +1,76 @@ package validation + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStringRequired(t *testing.T) { + t.Run("passes", func(t *testing.T) { + err := StringRequired().Validate("test") + assert.NoError(t, err) + }) + t.Run("fails", func(t *testing.T) { + err := StringRequired().Validate("") + assert.Error(t, err) + }) +} + +func TestStringLength(t *testing.T) { + t.Run("passes", func(t *testing.T) { + err := StringLength(0, 20).Validate("test") + assert.NoError(t, err) + }) + t.Run("fails, upper bound", func(t *testing.T) { + err := StringLength(0, 2).Validate("test") + assert.Error(t, err) + }) + t.Run("fails, lower bound", func(t *testing.T) { + err := StringLength(10, 20).Validate("test") + assert.Error(t, err) + }) +} + +func TestStringIsDNSSubdomain(t *testing.T) { + t.Run("passes", func(t *testing.T) { + for _, input := range []string{ + "test", + "s", + "test-this", + "test-1-this", + "test1-this", + "123", + strings.Repeat("l", 63), + } { + err := StringIsDNSSubdomain().Validate(input) + assert.NoError(t, err) + } + }) + t.Run("fails", func(t *testing.T) { + for _, input := range []string{ + "tesT", + "", + strings.Repeat("l", 64), + "test?", + "test this", + "1_2", + "LOL", + } { + err := StringIsDNSSubdomain().Validate(input) + assert.Error(t, err) + } + }) +} + +func TestStringDescription(t *testing.T) { + t.Run("passes", func(t *testing.T) { + err := StringDescription().Validate(strings.Repeat("l", 1050)) + assert.NoError(t, err) + }) + t.Run("fails", func(t *testing.T) { + err := StringDescription().Validate(strings.Repeat("l", 1051)) + assert.Error(t, err) + }) +} From 4e3f47c0c982ba9bf4368beded015e51bd83f9d7 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Fri, 29 Sep 2023 17:27:18 +0200 Subject: [PATCH 09/20] make sure labels testing is deterministic --- manifest/v1alpha/project/test_data/expected_error.txt | 2 +- manifest/v1alpha/project/validation_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/manifest/v1alpha/project/test_data/expected_error.txt b/manifest/v1alpha/project/test_data/expected_error.txt index 46c135c8..158fd319 100644 --- a/manifest/v1alpha/project/test_data/expected_error.txt +++ b/manifest/v1alpha/project/test_data/expected_error.txt @@ -4,7 +4,7 @@ Validation for Project 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PRO - a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') - 'metadata.displayName' with value 'my-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-projectmy-project': - length must be between 0 and 63 - - 'metadata.labels' with value '{"":["db"],"L O L":["dip","dip"]}': + - 'metadata.labels' with value '{"L O L":["dip","dip"]}': - label key 'L O L' does not match the regex: ^\p{L}([_\-0-9\p{L}]*[0-9\p{L}])?$ - 'spec.description' with value 'llllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll...': - length must be between 0 and 1050 diff --git a/manifest/v1alpha/project/validation_test.go b/manifest/v1alpha/project/validation_test.go index ed56cb5c..67a529f3 100644 --- a/manifest/v1alpha/project/validation_test.go +++ b/manifest/v1alpha/project/validation_test.go @@ -22,7 +22,6 @@ func TestValidate_AllErrors(t *testing.T) { DisplayName: strings.Repeat("my-project", 10), Labels: labels.Labels{ "L O L": []string{"dip", "dip"}, - "": []string{"db"}, }, }, Spec: Spec{ From 296068a5bf1c78490f2c1d0e3b64dc76377c4386 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Mon, 2 Oct 2023 10:51:36 +0200 Subject: [PATCH 10/20] invert dependency between project and v1alpha --- manifest/v1alpha/agent.go | 9 +++--- manifest/v1alpha/alert_policy.go | 9 +++--- manifest/v1alpha/data_export.go | 9 +++--- manifest/v1alpha/direct.go | 9 +++--- manifest/v1alpha/{labels => }/labels.go | 2 +- manifest/v1alpha/labels/doc.go | 4 --- manifest/v1alpha/{labels => }/labels_test.go | 2 +- manifest/v1alpha/parser/doc.go | 2 ++ manifest/v1alpha/{ => parser}/parser.go | 29 ++++++++++--------- manifest/v1alpha/{ => parser}/parser_test.go | 9 +++--- .../test_data}/cloudwatch_agent.json | 0 .../test_data}/cloudwatch_agent.yaml | 0 .../test_data}/generic_project.json | 0 .../test_data}/generic_project.yaml | 0 .../project_with_non_existing_keys.json | 0 .../project_with_non_existing_keys.yaml | 0 .../test_data}/redshift_agent.json | 0 .../test_data}/redshift_agent.yaml | 0 manifest/v1alpha/project/example_test.go | 4 +-- manifest/v1alpha/project/project.go | 8 ++--- manifest/v1alpha/project/validation.go | 8 ++--- manifest/v1alpha/project/validation_test.go | 4 +-- manifest/v1alpha/service.go | 9 +++--- manifest/v1alpha/slo.go | 9 +++--- manifest/v1alpha/validation/errors.go | 12 ++++---- manifest/v1alpha/validator.go | 3 +- sdk/parser.go | 3 +- 27 files changed, 69 insertions(+), 75 deletions(-) rename manifest/v1alpha/{labels => }/labels.go (99%) delete mode 100644 manifest/v1alpha/labels/doc.go rename manifest/v1alpha/{labels => }/labels_test.go (99%) create mode 100644 manifest/v1alpha/parser/doc.go rename manifest/v1alpha/{ => parser}/parser.go (78%) rename manifest/v1alpha/{ => parser}/parser_test.go (95%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/cloudwatch_agent.json (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/cloudwatch_agent.yaml (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/generic_project.json (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/generic_project.yaml (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/project_with_non_existing_keys.json (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/project_with_non_existing_keys.yaml (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/redshift_agent.json (100%) rename manifest/v1alpha/{test_data/parser => parser/test_data}/redshift_agent.yaml (100%) diff --git a/manifest/v1alpha/agent.go b/manifest/v1alpha/agent.go index f7158012..f064b9f8 100644 --- a/manifest/v1alpha/agent.go +++ b/manifest/v1alpha/agent.go @@ -4,7 +4,6 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go Agent @@ -23,10 +22,10 @@ type Agent struct { } type AgentMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // AgentSpec represents content of Spec typical for Agent Object diff --git a/manifest/v1alpha/alert_policy.go b/manifest/v1alpha/alert_policy.go index 0b3bcd4b..bceaac55 100644 --- a/manifest/v1alpha/alert_policy.go +++ b/manifest/v1alpha/alert_policy.go @@ -2,7 +2,6 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go AlertPolicy @@ -19,10 +18,10 @@ type AlertPolicy struct { } type AlertPolicyMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // AlertPolicySpec represents content of AlertPolicy's Spec. diff --git a/manifest/v1alpha/data_export.go b/manifest/v1alpha/data_export.go index 0e448e8c..46b1c511 100644 --- a/manifest/v1alpha/data_export.go +++ b/manifest/v1alpha/data_export.go @@ -4,7 +4,6 @@ import ( "encoding/json" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go DataExport @@ -28,10 +27,10 @@ type DataExport struct { } type DataExportMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // DataExportSpec represents content of DataExport's Spec diff --git a/manifest/v1alpha/direct.go b/manifest/v1alpha/direct.go index 46688b26..664a5231 100644 --- a/manifest/v1alpha/direct.go +++ b/manifest/v1alpha/direct.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go Direct @@ -34,10 +33,10 @@ type PublicDirect struct { } type DirectMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // DirectSpec represents content of Spec typical for Direct Object diff --git a/manifest/v1alpha/labels/labels.go b/manifest/v1alpha/labels.go similarity index 99% rename from manifest/v1alpha/labels/labels.go rename to manifest/v1alpha/labels.go index af72a071..e7d364dc 100644 --- a/manifest/v1alpha/labels/labels.go +++ b/manifest/v1alpha/labels.go @@ -1,4 +1,4 @@ -package labels +package v1alpha import ( "regexp" diff --git a/manifest/v1alpha/labels/doc.go b/manifest/v1alpha/labels/doc.go deleted file mode 100644 index 2787d860..00000000 --- a/manifest/v1alpha/labels/doc.go +++ /dev/null @@ -1,4 +0,0 @@ -// Package labels . -// Labels are key-value pairs that can be attached to some of the manifest.Object. -// Labels allow attaching custom attributes to objects which can in turn be used to filter, group and manage objects. -package labels diff --git a/manifest/v1alpha/labels/labels_test.go b/manifest/v1alpha/labels_test.go similarity index 99% rename from manifest/v1alpha/labels/labels_test.go rename to manifest/v1alpha/labels_test.go index e5d104ec..e31836d7 100644 --- a/manifest/v1alpha/labels/labels_test.go +++ b/manifest/v1alpha/labels_test.go @@ -1,4 +1,4 @@ -package labels +package v1alpha import ( "testing" diff --git a/manifest/v1alpha/parser/doc.go b/manifest/v1alpha/parser/doc.go new file mode 100644 index 00000000..fc53126d --- /dev/null +++ b/manifest/v1alpha/parser/doc.go @@ -0,0 +1,2 @@ +// Package parser provides parsing methods for v1alpha objects. +package parser diff --git a/manifest/v1alpha/parser.go b/manifest/v1alpha/parser/parser.go similarity index 78% rename from manifest/v1alpha/parser.go rename to manifest/v1alpha/parser/parser.go index 496808cf..42199231 100644 --- a/manifest/v1alpha/parser.go +++ b/manifest/v1alpha/parser/parser.go @@ -1,4 +1,4 @@ -package v1alpha +package parser import ( "bytes" @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) @@ -44,38 +45,38 @@ func parseObject(kind manifest.Kind, unmarshal unmarshalFunc) (manifest.Object, //exhaustive:enforce switch kind { case manifest.KindService: - return genericParseObject[Service](unmarshal) + return genericParseObject[v1alpha.Service](unmarshal) case manifest.KindSLO: - return genericParseObject[SLO](unmarshal) + return genericParseObject[v1alpha.SLO](unmarshal) case manifest.KindProject: return genericParseObject[project.Project](unmarshal) case manifest.KindAgent: - return genericParseObject[Agent](unmarshal) + return genericParseObject[v1alpha.Agent](unmarshal) case manifest.KindDirect: - return genericParseObject[Direct](unmarshal) + return genericParseObject[v1alpha.Direct](unmarshal) case manifest.KindAlert: - return genericParseObject[Alert](unmarshal) + return genericParseObject[v1alpha.Alert](unmarshal) case manifest.KindAlertMethod: - return genericParseObject[AlertMethod](unmarshal) + return genericParseObject[v1alpha.AlertMethod](unmarshal) case manifest.KindAlertPolicy: - return genericParseObject[AlertPolicy](unmarshal) + return genericParseObject[v1alpha.AlertPolicy](unmarshal) case manifest.KindAlertSilence: - return genericParseObject[AlertSilence](unmarshal) + return genericParseObject[v1alpha.AlertSilence](unmarshal) case manifest.KindRoleBinding: - return genericParseObject[RoleBinding](unmarshal) + return genericParseObject[v1alpha.RoleBinding](unmarshal) case manifest.KindDataExport: - return genericParseObject[DataExport](unmarshal) + return genericParseObject[v1alpha.DataExport](unmarshal) case manifest.KindAnnotation: - return genericParseObject[Annotation](unmarshal) + return genericParseObject[v1alpha.Annotation](unmarshal) case manifest.KindUserGroup: - return genericParseObject[UserGroup](unmarshal) + return genericParseObject[v1alpha.UserGroup](unmarshal) default: return nil, fmt.Errorf("%s is %w", kind, manifest.ErrInvalidKind) } } func parseGenericObject(unmarshal unmarshalFunc) (manifest.Object, error) { - return genericParseObject[GenericObject](unmarshal) + return genericParseObject[v1alpha.GenericObject](unmarshal) } func getUnmarshalFunc(data []byte, format manifest.ObjectFormat) (unmarshalFunc, error) { diff --git a/manifest/v1alpha/parser_test.go b/manifest/v1alpha/parser/parser_test.go similarity index 95% rename from manifest/v1alpha/parser_test.go rename to manifest/v1alpha/parser/parser_test.go index 8754ff8d..f612f9d6 100644 --- a/manifest/v1alpha/parser_test.go +++ b/manifest/v1alpha/parser/parser_test.go @@ -1,4 +1,4 @@ -package v1alpha +package parser import ( "embed" @@ -10,9 +10,10 @@ import ( "github.com/stretchr/testify/require" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) -//go:embed test_data/parser +//go:embed test_data var parserTestData embed.FS func TestParseObject(t *testing.T) { @@ -84,7 +85,7 @@ func TestParseObjectUsingGenericObject(t *testing.T) { require.NoError(t, err) assert.Equal(t, jsonObject, yamlObject) - assert.Equal(t, GenericObject{ + assert.Equal(t, v1alpha.GenericObject{ "apiVersion": "n9/v1alpha", "kind": "Project", "metadata": map[string]interface{}{ @@ -96,7 +97,7 @@ func TestParseObjectUsingGenericObject(t *testing.T) { func readParserTestFile(t *testing.T, filename string) ([]byte, manifest.ObjectFormat) { t.Helper() - data, err := parserTestData.ReadFile(filepath.Join("test_data", "parser", filename)) + data, err := parserTestData.ReadFile(filepath.Join("test_data", filename)) require.NoError(t, err) format, err := manifest.ParseObjectFormat(filepath.Ext(filename)[1:]) require.NoError(t, err) diff --git a/manifest/v1alpha/test_data/parser/cloudwatch_agent.json b/manifest/v1alpha/parser/test_data/cloudwatch_agent.json similarity index 100% rename from manifest/v1alpha/test_data/parser/cloudwatch_agent.json rename to manifest/v1alpha/parser/test_data/cloudwatch_agent.json diff --git a/manifest/v1alpha/test_data/parser/cloudwatch_agent.yaml b/manifest/v1alpha/parser/test_data/cloudwatch_agent.yaml similarity index 100% rename from manifest/v1alpha/test_data/parser/cloudwatch_agent.yaml rename to manifest/v1alpha/parser/test_data/cloudwatch_agent.yaml diff --git a/manifest/v1alpha/test_data/parser/generic_project.json b/manifest/v1alpha/parser/test_data/generic_project.json similarity index 100% rename from manifest/v1alpha/test_data/parser/generic_project.json rename to manifest/v1alpha/parser/test_data/generic_project.json diff --git a/manifest/v1alpha/test_data/parser/generic_project.yaml b/manifest/v1alpha/parser/test_data/generic_project.yaml similarity index 100% rename from manifest/v1alpha/test_data/parser/generic_project.yaml rename to manifest/v1alpha/parser/test_data/generic_project.yaml diff --git a/manifest/v1alpha/test_data/parser/project_with_non_existing_keys.json b/manifest/v1alpha/parser/test_data/project_with_non_existing_keys.json similarity index 100% rename from manifest/v1alpha/test_data/parser/project_with_non_existing_keys.json rename to manifest/v1alpha/parser/test_data/project_with_non_existing_keys.json diff --git a/manifest/v1alpha/test_data/parser/project_with_non_existing_keys.yaml b/manifest/v1alpha/parser/test_data/project_with_non_existing_keys.yaml similarity index 100% rename from manifest/v1alpha/test_data/parser/project_with_non_existing_keys.yaml rename to manifest/v1alpha/parser/test_data/project_with_non_existing_keys.yaml diff --git a/manifest/v1alpha/test_data/parser/redshift_agent.json b/manifest/v1alpha/parser/test_data/redshift_agent.json similarity index 100% rename from manifest/v1alpha/test_data/parser/redshift_agent.json rename to manifest/v1alpha/parser/test_data/redshift_agent.json diff --git a/manifest/v1alpha/test_data/parser/redshift_agent.yaml b/manifest/v1alpha/parser/test_data/redshift_agent.yaml similarity index 100% rename from manifest/v1alpha/test_data/parser/redshift_agent.yaml rename to manifest/v1alpha/parser/test_data/redshift_agent.yaml diff --git a/manifest/v1alpha/project/example_test.go b/manifest/v1alpha/project/example_test.go index a002e569..44b26e96 100644 --- a/manifest/v1alpha/project/example_test.go +++ b/manifest/v1alpha/project/example_test.go @@ -6,7 +6,7 @@ import ( "github.com/nobl9/nobl9-go/internal/examples" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/project" ) @@ -16,7 +16,7 @@ func ExampleProject() { project.Metadata{ Name: "my-project", DisplayName: "My Project", - Labels: labels.Labels{ + Labels: v1alpha.Labels{ "team": []string{"green", "orange"}, "region": []string{"eu-central-1"}, }, diff --git a/manifest/v1alpha/project/project.go b/manifest/v1alpha/project/project.go index ad2e3218..0d1b75a2 100644 --- a/manifest/v1alpha/project/project.go +++ b/manifest/v1alpha/project/project.go @@ -2,7 +2,7 @@ package project import ( "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) //go:generate go run ../../../scripts/generate-object-impl.go Project @@ -31,9 +31,9 @@ func New(metadata Metadata, spec Spec) Project { // Metadata provides identity information for Project. type Metadata struct { - Name string `json:"name" validate:"required,objectName" example:"name"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63" example:"Shopping App"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName" example:"name"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63" example:"Shopping App"` + Labels v1alpha.Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // Spec holds detailed information specific to Project. diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index f259dabe..9161abfa 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -1,7 +1,7 @@ package project import ( - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" ) @@ -26,11 +26,11 @@ func validate(p Project) error { func() string { return p.Metadata.DisplayName }, ). With(validation.StringLength(0, 63)), - validation.RulesForField[labels.Labels]( + validation.RulesForField[v1alpha.Labels]( "metadata.labels", - func() labels.Labels { return p.Metadata.Labels }, + func() v1alpha.Labels { return p.Metadata.Labels }, ). - With(labels.ValidationRule()), + With(v1alpha.ValidationRule()), validation.RulesForField[string]( "spec.description", func() string { return p.Spec.Description }, diff --git a/manifest/v1alpha/project/validation_test.go b/manifest/v1alpha/project/validation_test.go index 67a529f3..9cd52977 100644 --- a/manifest/v1alpha/project/validation_test.go +++ b/manifest/v1alpha/project/validation_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) //go:embed test_data/expected_error.txt @@ -20,7 +20,7 @@ func TestValidate_AllErrors(t *testing.T) { Metadata: Metadata{ Name: strings.Repeat("MY PROJECT", 20), DisplayName: strings.Repeat("my-project", 10), - Labels: labels.Labels{ + Labels: v1alpha.Labels{ "L O L": []string{"dip", "dip"}, }, }, diff --git a/manifest/v1alpha/service.go b/manifest/v1alpha/service.go index bb84e5c5..b8fa3b3c 100644 --- a/manifest/v1alpha/service.go +++ b/manifest/v1alpha/service.go @@ -2,7 +2,6 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go Service @@ -20,10 +19,10 @@ type Service struct { } type ServiceMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // ServiceStatus represents content of Status optional for Service Object. diff --git a/manifest/v1alpha/slo.go b/manifest/v1alpha/slo.go index cec95ecc..c0b8a385 100644 --- a/manifest/v1alpha/slo.go +++ b/manifest/v1alpha/slo.go @@ -2,7 +2,6 @@ package v1alpha import ( "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" ) //go:generate go run ../../scripts/generate-object-impl.go SLO @@ -20,10 +19,10 @@ type SLO struct { } type SLOMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels labels.Labels `json:"labels,omitempty" validate:"omitempty,labels"` + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` } // SLOSpec represents content of Spec typical for SLO Object diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go index 7350fff1..f73032fb 100644 --- a/manifest/v1alpha/validation/errors.go +++ b/manifest/v1alpha/validation/errors.go @@ -13,11 +13,11 @@ type ObjectError struct { } type ObjectMetadata struct { - IsProjectScoped bool - Kind string - Name string - Project string - Source string + IsProjectScoped bool `json:"isProjectScoped"` + Kind string `json:"kind"` + Name string `json:"name"` + Project string `json:"project"` + Source string `json:"source"` } func (e ObjectError) Error() string { @@ -38,7 +38,7 @@ func (e ObjectError) Error() string { type FieldError struct { FieldPath string `json:"fieldPath"` FieldValue interface{} `json:"value"` - Errors []error `json:"Errors"` + Errors []error `json:"errors"` } func (e FieldError) Error() string { diff --git a/manifest/v1alpha/validator.go b/manifest/v1alpha/validator.go index 11e1a950..d3d3b4a0 100644 --- a/manifest/v1alpha/validator.go +++ b/manifest/v1alpha/validator.go @@ -20,7 +20,6 @@ import ( "golang.org/x/text/language" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha/labels" "github.com/nobl9/nobl9-go/manifest/v1alpha/twindow" ) @@ -1640,7 +1639,7 @@ func validateURLDynatrace(validateURL string) bool { } func areLabelsValid(fl v.FieldLevel) bool { - lbl := fl.Field().Interface().(labels.Labels) + lbl := fl.Field().Interface().(Labels) return lbl.Validate() == nil } diff --git a/sdk/parser.go b/sdk/parser.go index 2c71be03..a8943fa1 100644 --- a/sdk/parser.go +++ b/sdk/parser.go @@ -12,6 +12,7 @@ import ( "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha" + v1alphaParser "github.com/nobl9/nobl9-go/manifest/v1alpha/parser" ) var ErrNoDefinitionsFound = errors.New("no definitions in input") @@ -168,7 +169,7 @@ func (o *genericObject) unmarshalGeneric(data []byte, format manifest.ObjectForm } switch object.ApiVersion { case manifest.VersionV1alpha: - parsed, err := v1alpha.ParseObject(data, object.Kind, format) + parsed, err := v1alphaParser.ParseObject(data, object.Kind, format) if err != nil { return err } From 5b582d38c19c76316359ffe101d6b9bf3e030fa6 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Mon, 2 Oct 2023 19:51:25 +0200 Subject: [PATCH 11/20] facilitate recent suggestions and feedback --- manifest/object.go | 13 +- manifest/object_test.go | 30 ++--- manifest/v1alpha/errors.go | 112 ++++++++++++++++++ manifest/v1alpha/errors_test.go | 96 +++++++++++++++ manifest/v1alpha/labels.go | 2 +- manifest/v1alpha/project/project_object.go | 4 +- manifest/v1alpha/project/validation.go | 14 +-- .../errors}/object_error.txt | 0 .../errors}/object_error_project_scoped.txt | 0 manifest/v1alpha/validation/doc.go | 2 - manifest/v1alpha/validation/errors.go | 99 ---------------- manifest/v1alpha/validation/errors_test.go | 99 ---------------- validation/doc.go | 2 + validation/errors.go | 101 ++++++++++++++++ validation/errors_test.go | 56 +++++++++ .../v1alpha/validation => validation}/rule.go | 0 .../validation => validation}/rules.go | 26 +--- .../validation => validation}/rules_test.go | 45 +++---- .../validation => validation}/string.go | 0 .../validation => validation}/string_test.go | 0 .../test_data/field_error_map.txt | 0 .../test_data/field_error_slice.txt | 0 .../test_data/field_error_string.txt | 0 .../test_data/field_error_struct.txt | 0 .../test_data/multi_error.txt | 0 25 files changed, 422 insertions(+), 279 deletions(-) create mode 100644 manifest/v1alpha/errors.go create mode 100644 manifest/v1alpha/errors_test.go rename manifest/v1alpha/{validation/test_data => test_data/errors}/object_error.txt (100%) rename manifest/v1alpha/{validation/test_data => test_data/errors}/object_error_project_scoped.txt (100%) delete mode 100644 manifest/v1alpha/validation/doc.go delete mode 100644 manifest/v1alpha/validation/errors.go delete mode 100644 manifest/v1alpha/validation/errors_test.go create mode 100644 validation/doc.go create mode 100644 validation/errors.go create mode 100644 validation/errors_test.go rename {manifest/v1alpha/validation => validation}/rule.go (100%) rename {manifest/v1alpha/validation => validation}/rules.go (72%) rename {manifest/v1alpha/validation => validation}/rules_test.go (78%) rename {manifest/v1alpha/validation => validation}/string.go (100%) rename {manifest/v1alpha/validation => validation}/string_test.go (100%) rename {manifest/v1alpha/validation => validation}/test_data/field_error_map.txt (100%) rename {manifest/v1alpha/validation => validation}/test_data/field_error_slice.txt (100%) rename {manifest/v1alpha/validation => validation}/test_data/field_error_string.txt (100%) rename {manifest/v1alpha/validation => validation}/test_data/field_error_struct.txt (100%) rename {manifest/v1alpha/validation => validation}/test_data/multi_error.txt (100%) diff --git a/manifest/object.go b/manifest/object.go index fe409bea..4ac2eec4 100644 --- a/manifest/object.go +++ b/manifest/object.go @@ -48,17 +48,20 @@ func FilterByKind[T Object](objects []Object) []T { // Validate performs validation of all the provided objects. // It aggregates the results into a single error. -func Validate(objects []Object) error { - errs := make([]string, 0) +func Validate(objects []Object) []error { + errs := make([]error, 0) for i := range objects { if err := objects[i].Validate(); err != nil { - errs = append(errs, err.Error()) + errs = append(errs, err) } } if len(errs) > 0 { - return errors.New(strings.Join(errs, "\n")) + return errs } - return validateObjectsUniqueness(objects) + if err := validateObjectsUniqueness(objects); err != nil { + return []error{err} + } + return nil } // SetDefaultProject sets the default project for each object only if the object is diff --git a/manifest/object_test.go b/manifest/object_test.go index 9738343d..754e3c64 100644 --- a/manifest/object_test.go +++ b/manifest/object_test.go @@ -45,35 +45,37 @@ var expectedUniquenessConstraintMessage string func TestValidate(t *testing.T) { t.Run("nil objects slice", func(t *testing.T) { - err := Validate(nil) - assert.NoError(t, err) + errs := Validate(nil) + assert.Empty(t, errs) }) t.Run("empty objects slice", func(t *testing.T) { - err := Validate([]Object{}) - assert.NoError(t, err) + errs := Validate([]Object{}) + assert.Empty(t, errs) }) t.Run("no errors", func(t *testing.T) { - err := Validate([]Object{ + errs := Validate([]Object{ customObject{kind: KindProject, name: "default"}, customObject{kind: KindRoleBinding, name: "default"}, }) - assert.NoError(t, err) + assert.Empty(t, errs) }) t.Run("errors", func(t *testing.T) { - err := Validate([]Object{ + err1 := errors.New("I failed!") + err2 := errors.New("I failed too!") + errs := Validate([]Object{ customObject{}, - customObject{validationError: errors.New("I failed!")}, - customObject{validationError: errors.New("I failed too!")}, + customObject{validationError: err1}, + customObject{validationError: err2}, }) - assert.Error(t, err) - assert.EqualError(t, err, "I failed!\nI failed too!") + assert.Len(t, errs, 2) + assert.ElementsMatch(t, []error{err1, err2}, errs) }) t.Run("uniqueness constraint violated", func(t *testing.T) { - err := Validate([]Object{ + errs := Validate([]Object{ customObject{kind: KindProject, name: "sun"}, customObject{kind: KindProject, name: "sun"}, customObject{kind: KindProject, name: "moon"}, @@ -106,8 +108,8 @@ func TestValidate(t *testing.T) { kind: KindService, name: "jupiter"}, project: "default"}, }) - assert.Error(t, err) - assert.EqualError(t, err, strings.ReplaceAll(expectedUniquenessConstraintMessage, "\n", "; ")) + assert.Len(t, errs, 1) + assert.EqualError(t, errs[0], strings.ReplaceAll(expectedUniquenessConstraintMessage, "\n", "; ")) }) } diff --git a/manifest/v1alpha/errors.go b/manifest/v1alpha/errors.go new file mode 100644 index 00000000..4412c7af --- /dev/null +++ b/manifest/v1alpha/errors.go @@ -0,0 +1,112 @@ +package v1alpha + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/pkg/errors" + + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/validation" +) + +func NewObjectError(object manifest.Object, errs []error) error { + oErr := &ObjectError{ + Object: ObjectMetadata{ + Kind: object.GetKind(), + Name: object.GetName(), + }, + Errors: errs, + } + if v, ok := object.(ObjectContext); ok { + oErr.Object.Source = v.GetManifestSource() + } + if v, ok := object.(manifest.ProjectScopedObject); ok { + oErr.Object.IsProjectScoped = true + oErr.Object.Project = v.GetProject() + } + return oErr +} + +type ObjectError struct { + Object ObjectMetadata `json:"object"` + Errors []error `json:"errors"` +} + +type ObjectMetadata struct { + Kind manifest.Kind `json:"kind"` + Name string `json:"name"` + Source string `json:"source"` + IsProjectScoped bool `json:"isProjectScoped"` + Project string `json:"project,omitempty"` +} + +func (o *ObjectError) Error() string { + b := new(strings.Builder) + b.WriteString(fmt.Sprintf("Validation for %s '%s'", o.Object.Kind, o.Object.Name)) + if o.Object.IsProjectScoped { + b.WriteString(" in project '" + o.Object.Project + "'") + } + b.WriteString(" has failed for the following fields:\n") + validation.JoinErrors(b, o.Errors, strings.Repeat(" ", 2)) + if o.Object.Source != "" { + b.WriteString("\nManifest source: ") + b.WriteString(o.Object.Source) + } + return b.String() +} + +func (o *ObjectError) MarshalJSON() ([]byte, error) { + var errs []json.RawMessage + for _, oErr := range o.Errors { + switch v := oErr.(type) { + case validation.FieldError: + data, err := json.Marshal(v) + if err != nil { + return nil, err + } + errs = append(errs, data) + default: + data, err := json.Marshal(oErr.Error()) + if err != nil { + return nil, err + } + errs = append(errs, data) + } + } + return json.Marshal(struct { + Object ObjectMetadata `json:"object"` + Errors []json.RawMessage `json:"errors"` + }{ + Object: o.Object, + Errors: errs, + }) +} + +func (o *ObjectError) UnmarshalJSON(bytes []byte) error { + var intermediate struct { + Object ObjectMetadata `json:"object"` + Errors []json.RawMessage `json:"errors"` + } + if err := json.Unmarshal(bytes, &intermediate); err != nil { + return err + } + o.Object = intermediate.Object + for _, rawErr := range intermediate.Errors { + if len(rawErr) > 0 && rawErr[0] == '{' { + var fErr validation.FieldError + if err := json.Unmarshal(rawErr, &fErr); err != nil { + return err + } + o.Errors = append(o.Errors, fErr) + } else { + var stringErr string + if err := json.Unmarshal(rawErr, &stringErr); err != nil { + return err + } + o.Errors = append(o.Errors, errors.New(stringErr)) + } + } + return nil +} diff --git a/manifest/v1alpha/errors_test.go b/manifest/v1alpha/errors_test.go new file mode 100644 index 00000000..17759ed7 --- /dev/null +++ b/manifest/v1alpha/errors_test.go @@ -0,0 +1,96 @@ +package v1alpha + +import ( + "embed" + "encoding/json" + "path/filepath" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/validation" +) + +//go:embed test_data/errors +var errorsTestData embed.FS + +func TestObjectError(t *testing.T) { + errs := []error{ + validation.FieldError{ + FieldPath: "metadata.name", + FieldValue: "default", + Errors: []string{"here's an error"}, + }, + validation.FieldError{ + FieldPath: "spec.description", + FieldValue: "some long description", + Errors: []string{"here's another error"}, + }, + } + + t.Run("non project scoped object", func(t *testing.T) { + err := &ObjectError{ + Object: ObjectMetadata{ + Kind: manifest.KindProject, + Name: "default", + Source: "/home/me/project.yaml", + }, + Errors: errs, + } + assert.EqualError(t, err, expectedErrorOutput(t, "object_error.txt")) + }) + + t.Run("project scoped object", func(t *testing.T) { + err := &ObjectError{ + Object: ObjectMetadata{ + IsProjectScoped: true, + Kind: manifest.KindService, + Name: "my-service", + Project: "default", + Source: "/home/me/service.yaml", + }, + Errors: errs, + } + assert.EqualError(t, err, expectedErrorOutput(t, "object_error_project_scoped.txt")) + }) +} + +func TestObjectError_UnmarshalJSON(t *testing.T) { + expected := &ObjectError{ + Object: ObjectMetadata{ + Kind: manifest.KindService, + Name: "test-service", + Source: "/home/me/service.yaml", + IsProjectScoped: true, + Project: "default", + }, + Errors: []error{ + validation.FieldError{ + FieldPath: "metadata.project", + FieldValue: "default", + Errors: []string{"nested"}, + }, + errors.New("some error"), + }, + } + data, err := json.Marshal(expected) + require.NoError(t, err) + + var actual ObjectError + err = json.Unmarshal(data, &actual) + require.NoError(t, err) + + assert.Equal(t, expected.Object, actual.Object) + assert.Equal(t, expected.Errors[0], actual.Errors[0]) + assert.Equal(t, expected.Errors[1].Error(), actual.Errors[1].Error()) +} + +func expectedErrorOutput(t *testing.T, name string) string { + t.Helper() + data, err := errorsTestData.ReadFile(filepath.Join("test_data", "errors", name)) + require.NoError(t, err) + return string(data) +} diff --git a/manifest/v1alpha/labels.go b/manifest/v1alpha/labels.go index e7d364dc..baaefeea 100644 --- a/manifest/v1alpha/labels.go +++ b/manifest/v1alpha/labels.go @@ -6,7 +6,7 @@ import ( "github.com/pkg/errors" - "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" + "github.com/nobl9/nobl9-go/validation" ) type ( diff --git a/manifest/v1alpha/project/project_object.go b/manifest/v1alpha/project/project_object.go index 83b6e836..4af6de34 100644 --- a/manifest/v1alpha/project/project_object.go +++ b/manifest/v1alpha/project/project_object.go @@ -2,7 +2,9 @@ package project -import "github.com/nobl9/nobl9-go/manifest" +import ( + "github.com/nobl9/nobl9-go/manifest" +) func (p Project) GetVersion() string { return p.APIVersion diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index 9161abfa..e0abe168 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -2,22 +2,15 @@ package project import ( "github.com/nobl9/nobl9-go/manifest/v1alpha" - "github.com/nobl9/nobl9-go/manifest/v1alpha/validation" + "github.com/nobl9/nobl9-go/validation" ) func validate(p Project) error { v := validation.RulesForObject( - validation.ObjectMetadata{ - Kind: p.GetKind().String(), - Name: p.GetName(), - Source: p.GetManifestSource(), - }, validation.RulesForField[string]( "metadata.name", func() string { return p.Metadata.Name }, ). - // If predicate has been included just for the demonstration. - If(func() bool { return p.Spec.Description != "lol" }). With( validation.StringRequired(), validation.StringIsDNSSubdomain()), @@ -37,5 +30,8 @@ func validate(p Project) error { ). With(validation.StringDescription()), ) - return v.Validate() + if errs := v.Validate(); len(errs) > 0 { + return v1alpha.NewObjectError(p, errs) + } + return nil } diff --git a/manifest/v1alpha/validation/test_data/object_error.txt b/manifest/v1alpha/test_data/errors/object_error.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/object_error.txt rename to manifest/v1alpha/test_data/errors/object_error.txt diff --git a/manifest/v1alpha/validation/test_data/object_error_project_scoped.txt b/manifest/v1alpha/test_data/errors/object_error_project_scoped.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/object_error_project_scoped.txt rename to manifest/v1alpha/test_data/errors/object_error_project_scoped.txt diff --git a/manifest/v1alpha/validation/doc.go b/manifest/v1alpha/validation/doc.go deleted file mode 100644 index 8fce6ec4..00000000 --- a/manifest/v1alpha/validation/doc.go +++ /dev/null @@ -1,2 +0,0 @@ -// Package validation implements a functional API for validating any manifest.Object. -package validation diff --git a/manifest/v1alpha/validation/errors.go b/manifest/v1alpha/validation/errors.go deleted file mode 100644 index f73032fb..00000000 --- a/manifest/v1alpha/validation/errors.go +++ /dev/null @@ -1,99 +0,0 @@ -package validation - -import ( - "encoding/json" - "fmt" - "reflect" - "strings" -) - -type ObjectError struct { - Object ObjectMetadata `json:"object"` - Errors []error `json:"errors"` -} - -type ObjectMetadata struct { - IsProjectScoped bool `json:"isProjectScoped"` - Kind string `json:"kind"` - Name string `json:"name"` - Project string `json:"project"` - Source string `json:"source"` -} - -func (e ObjectError) Error() string { - b := new(strings.Builder) - b.WriteString(fmt.Sprintf("Validation for %s '%s'", e.Object.Kind, e.Object.Name)) - if e.Object.IsProjectScoped { - b.WriteString(" in project '" + e.Object.Project + "'") - } - b.WriteString(" has failed for the following fields:\n") - joinErrors(b, e.Errors, strings.Repeat(" ", 2)) - if e.Object.Source != "" { - b.WriteString("\nManifest source: ") - b.WriteString(e.Object.Source) - } - return b.String() -} - -type FieldError struct { - FieldPath string `json:"fieldPath"` - FieldValue interface{} `json:"value"` - Errors []error `json:"errors"` -} - -func (e FieldError) Error() string { - b := new(strings.Builder) - b.WriteString(fmt.Sprintf("'%s' with value '%s':\n", e.FieldPath, e.ValueString())) - joinErrors(b, e.Errors, strings.Repeat(" ", 2)) - return b.String() -} - -func (e FieldError) ValueString() string { - ft := reflect.TypeOf(e.FieldValue) - if ft.Kind() == reflect.Pointer { - ft = ft.Elem() - } - var s string - switch ft.Kind() { - case reflect.Interface, reflect.Map, reflect.Slice, reflect.Struct: - raw, _ := json.Marshal(e.FieldValue) - s = string(raw) - default: - s = fmt.Sprint(e.FieldValue) - } - return limitString(s, 100) -} - -// multiRuleError is a container for transferring multiple errors reported by MultiRule. -type multiRuleError []error - -// Error is only implemented to satisfy the error interface. -func (m multiRuleError) Error() string { - b := new(strings.Builder) - joinErrors(b, m, "") - return b.String() -} - -const listPoint = "- " - -func joinErrors(b *strings.Builder, errs []error, indent string) { - for i, e := range errs { - b.WriteString(indent) - b.WriteString(listPoint) - // Remove the first list point characters if the error contained them. - errMsg := strings.TrimLeft(e.Error(), listPoint) - // Indent the whole error message. - errMsg = strings.ReplaceAll(errMsg, "\n", "\n"+indent) - b.WriteString(errMsg) - if i < len(errs)-1 { - b.WriteString("\n") - } - } -} - -func limitString(s string, limit int) string { - if len(s) > limit { - return s[:limit] + "..." - } - return s -} diff --git a/manifest/v1alpha/validation/errors_test.go b/manifest/v1alpha/validation/errors_test.go deleted file mode 100644 index 2c623c22..00000000 --- a/manifest/v1alpha/validation/errors_test.go +++ /dev/null @@ -1,99 +0,0 @@ -package validation - -import ( - "embed" - "fmt" - "path/filepath" - "testing" - - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -//go:embed test_data -var errorsTestData embed.FS - -func TestMultiRuleError(t *testing.T) { - err := multiRuleError{ - errors.New("this is just a test!"), - errors.New("another error..."), - errors.New("that is just fatal."), - } - assert.EqualError(t, err, expectedErrorOutput(t, "multi_error.txt")) -} - -func TestFieldError(t *testing.T) { - for typ, value := range map[string]interface{}{ - "string": "default", - "slice": []string{"this", "that"}, - "map": map[string]string{"this": "that"}, - "struct": struct { - This string `json:"this"` - That string `json:"THAT"` - }{This: "this", That: "that"}, - } { - t.Run(typ, func(t *testing.T) { - err := FieldError{ - FieldPath: "metadata.name", - FieldValue: value, - Errors: []error{ - multiRuleError{ - errors.New("what a shame this happened"), - errors.New("this is outrageous..."), - }, - errors.New("here's another error"), - }, - } - assert.EqualError(t, err, expectedErrorOutput(t, fmt.Sprintf("field_error_%s.txt", typ))) - }) - } -} - -func TestObjectError(t *testing.T) { - errs := []error{ - FieldError{ - FieldPath: "metadata.name", - FieldValue: "default", - Errors: []error{errors.New("here's an error")}, - }, - FieldError{ - FieldPath: "spec.description", - FieldValue: "some long description", - Errors: []error{errors.New("here's another error")}, - }, - } - - t.Run("non project scoped object", func(t *testing.T) { - err := ObjectError{ - Object: ObjectMetadata{ - Kind: "Project", - Name: "default", - Source: "/home/me/project.yaml", - }, - Errors: errs, - } - assert.EqualError(t, err, expectedErrorOutput(t, "object_error.txt")) - }) - - t.Run("project scoped object", func(t *testing.T) { - err := ObjectError{ - Object: ObjectMetadata{ - IsProjectScoped: true, - Kind: "Service", - Name: "my-service", - Project: "default", - Source: "/home/me/service.yaml", - }, - Errors: errs, - } - assert.EqualError(t, err, expectedErrorOutput(t, "object_error_project_scoped.txt")) - }) -} - -func expectedErrorOutput(t *testing.T, name string) string { - t.Helper() - data, err := errorsTestData.ReadFile(filepath.Join("test_data", name)) - require.NoError(t, err) - return string(data) -} diff --git a/validation/doc.go b/validation/doc.go new file mode 100644 index 00000000..9f97f899 --- /dev/null +++ b/validation/doc.go @@ -0,0 +1,2 @@ +// Package validation implements a functional API for consistent struct level validation. +package validation diff --git a/validation/errors.go b/validation/errors.go new file mode 100644 index 00000000..a79ce389 --- /dev/null +++ b/validation/errors.go @@ -0,0 +1,101 @@ +package validation + +import ( + "encoding/json" + "fmt" + "reflect" + "strings" +) + +func NewFieldError(fieldPath string, fieldValue interface{}, errs []error) *FieldError { + errorMessages := make([]string, 0, len(errs)) + for _, err := range errs { + if mErrs, ok := err.(multiRuleError); ok { + for _, mErr := range mErrs { + errorMessages = append(errorMessages, mErr.Error()) + } + } else { + errorMessages = append(errorMessages, err.Error()) + } + } + return &FieldError{ + FieldPath: fieldPath, + FieldValue: fieldValue, + Errors: errorMessages, + } +} + +type FieldError struct { + FieldPath string `json:"fieldPath"` + FieldValue interface{} `json:"value"` + Errors []string `json:"errors"` +} + +func (e FieldError) Error() string { + b := new(strings.Builder) + b.WriteString(fmt.Sprintf("'%s' with value '%s':\n", e.FieldPath, e.ValueString())) + joinErrorMessages(b, e.Errors, strings.Repeat(" ", 2)) + return b.String() +} + +func (e FieldError) ValueString() string { + ft := reflect.TypeOf(e.FieldValue) + if ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + var s string + switch ft.Kind() { + case reflect.Interface, reflect.Map, reflect.Slice, reflect.Struct: + raw, _ := json.Marshal(e.FieldValue) + s = string(raw) + default: + s = fmt.Sprint(e.FieldValue) + } + return limitString(s, 100) +} + +// multiRuleError is a container for transferring multiple errors reported by MultiRule. +type multiRuleError []error + +func (m multiRuleError) Error() string { + b := new(strings.Builder) + JoinErrors(b, m, "") + return b.String() +} + +const listPoint = "- " + +func JoinErrors(b *strings.Builder, errs []error, indent string) { + for i, err := range errs { + buildErrorMessage(b, err.Error(), indent) + if i < len(errs)-1 { + b.WriteString("\n") + } + } +} + +func joinErrorMessages(b *strings.Builder, msgs []string, indent string) { + for i, msg := range msgs { + buildErrorMessage(b, msg, indent) + if i < len(msgs)-1 { + b.WriteString("\n") + } + } +} + +func buildErrorMessage(b *strings.Builder, errMsg, indent string) { + b.WriteString(indent) + b.WriteString(listPoint) + // Remove the first list point characters if the error contained them. + errMsg = strings.TrimLeft(errMsg, listPoint) + // Indent the whole error message. + errMsg = strings.ReplaceAll(errMsg, "\n", "\n"+indent) + b.WriteString(errMsg) +} + +func limitString(s string, limit int) string { + if len(s) > limit { + return s[:limit] + "..." + } + return s +} diff --git a/validation/errors_test.go b/validation/errors_test.go new file mode 100644 index 00000000..ef262ebe --- /dev/null +++ b/validation/errors_test.go @@ -0,0 +1,56 @@ +package validation + +import ( + "embed" + "fmt" + "path/filepath" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +//go:embed test_data +var errorsTestData embed.FS + +func TestMultiRuleError(t *testing.T) { + err := multiRuleError{ + errors.New("this is just a test!"), + errors.New("another error..."), + errors.New("that is just fatal."), + } + assert.EqualError(t, err, expectedErrorOutput(t, "multi_error.txt")) +} + +func TestFieldError(t *testing.T) { + for typ, value := range map[string]interface{}{ + "string": "default", + "slice": []string{"this", "that"}, + "map": map[string]string{"this": "that"}, + "struct": struct { + This string `json:"this"` + That string `json:"THAT"` + }{This: "this", That: "that"}, + } { + t.Run(typ, func(t *testing.T) { + err := FieldError{ + FieldPath: "metadata.name", + FieldValue: value, + Errors: []string{ + "what a shame this happened", + "this is outrageous...", + "here's another error", + }, + } + assert.EqualError(t, err, expectedErrorOutput(t, fmt.Sprintf("field_error_%s.txt", typ))) + }) + } +} + +func expectedErrorOutput(t *testing.T, name string) string { + t.Helper() + data, err := errorsTestData.ReadFile(filepath.Join("test_data", name)) + require.NoError(t, err) + return string(data) +} diff --git a/manifest/v1alpha/validation/rule.go b/validation/rule.go similarity index 100% rename from manifest/v1alpha/validation/rule.go rename to validation/rule.go diff --git a/manifest/v1alpha/validation/rules.go b/validation/rules.go similarity index 72% rename from manifest/v1alpha/validation/rules.go rename to validation/rules.go index 66081f06..e1a4bdf2 100644 --- a/manifest/v1alpha/validation/rules.go +++ b/validation/rules.go @@ -4,32 +4,22 @@ type fieldRules interface { Validate() error } -func RulesForObject(objectMetadata ObjectMetadata, rules ...fieldRules) ObjectRules { - return ObjectRules{ - objectMetadata: objectMetadata, - fieldRules: rules, - } +func RulesForObject(rules ...fieldRules) ObjectRules { + return ObjectRules{fieldRules: rules} } type ObjectRules struct { - objectMetadata ObjectMetadata - fieldRules []fieldRules + fieldRules []fieldRules } -func (r ObjectRules) Validate() error { +func (r ObjectRules) Validate() []error { var errors []error for _, field := range r.fieldRules { if err := field.Validate(); err != nil { errors = append(errors, err) } } - if len(errors) > 0 { - return &ObjectError{ - Object: r.objectMetadata, - Errors: errors, - } - } - return nil + return errors } // RulesForField creates a typed FieldRules instance for the field which access is defined through getter function. @@ -59,11 +49,7 @@ func (r FieldRules[T]) Validate() error { } } if len(errors) > 0 { - return &FieldError{ - FieldPath: r.fieldPath, - FieldValue: fv, - Errors: errors, - } + return NewFieldError(r.fieldPath, fv, errors) } return nil } diff --git a/manifest/v1alpha/validation/rules_test.go b/validation/rules_test.go similarity index 78% rename from manifest/v1alpha/validation/rules_test.go rename to validation/rules_test.go index e55de471..ca4b7b18 100644 --- a/manifest/v1alpha/validation/rules_test.go +++ b/validation/rules_test.go @@ -11,23 +11,17 @@ import ( func TestRulesForObject(t *testing.T) { t.Run("no errors", func(t *testing.T) { r := RulesForObject( - ObjectMetadata{}, RulesForField[string]("test", func() string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), ) - err := r.Validate() - assert.NoError(t, err) + errs := r.Validate() + assert.Empty(t, errs) }) t.Run("errors", func(t *testing.T) { err1 := errors.New("1") err2 := errors.New("2") r := RulesForObject( - ObjectMetadata{ - Kind: "Project", - Name: "default", - Source: "/home/me/project.yaml", - }, RulesForField[string]("test", func() string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), RulesForField[string]("test.name", func() string { return "name" }). @@ -35,27 +29,20 @@ func TestRulesForObject(t *testing.T) { RulesForField[string]("test.display", func() string { return "display" }). With(SingleRule[string](func(v string) error { return err2 })), ) - err := r.Validate() - require.Error(t, err) - assert.Equal(t, ObjectError{ - Object: ObjectMetadata{ - Kind: "Project", - Name: "default", - Source: "/home/me/project.yaml", + errs := r.Validate() + require.Len(t, errs, 2) + assert.Equal(t, []error{ + &FieldError{ + FieldPath: "test.name", + FieldValue: "name", + Errors: []string{err1.Error()}, }, - Errors: []error{ - &FieldError{ - FieldPath: "test.name", - FieldValue: "name", - Errors: []error{err1}, - }, - &FieldError{ - FieldPath: "test.display", - FieldValue: "display", - Errors: []error{err2}, - }, + &FieldError{ + FieldPath: "test.display", + FieldValue: "display", + Errors: []string{err2.Error()}, }, - }, *err.(*ObjectError)) + }, errs) }) } @@ -76,7 +63,7 @@ func TestRulesForField(t *testing.T) { assert.Equal(t, FieldError{ FieldPath: "test.path", FieldValue: "path", - Errors: []error{expectedErr}, + Errors: []string{expectedErr.Error()}, }, *err.(*FieldError)) }) @@ -103,7 +90,7 @@ func TestRulesForField(t *testing.T) { assert.Equal(t, FieldError{ FieldPath: "test.path", FieldValue: "value", - Errors: []error{err1, err2}, + Errors: []string{err1.Error(), err2.Error()}, }, *err.(*FieldError)) }) } diff --git a/manifest/v1alpha/validation/string.go b/validation/string.go similarity index 100% rename from manifest/v1alpha/validation/string.go rename to validation/string.go diff --git a/manifest/v1alpha/validation/string_test.go b/validation/string_test.go similarity index 100% rename from manifest/v1alpha/validation/string_test.go rename to validation/string_test.go diff --git a/manifest/v1alpha/validation/test_data/field_error_map.txt b/validation/test_data/field_error_map.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/field_error_map.txt rename to validation/test_data/field_error_map.txt diff --git a/manifest/v1alpha/validation/test_data/field_error_slice.txt b/validation/test_data/field_error_slice.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/field_error_slice.txt rename to validation/test_data/field_error_slice.txt diff --git a/manifest/v1alpha/validation/test_data/field_error_string.txt b/validation/test_data/field_error_string.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/field_error_string.txt rename to validation/test_data/field_error_string.txt diff --git a/manifest/v1alpha/validation/test_data/field_error_struct.txt b/validation/test_data/field_error_struct.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/field_error_struct.txt rename to validation/test_data/field_error_struct.txt diff --git a/manifest/v1alpha/validation/test_data/multi_error.txt b/validation/test_data/multi_error.txt similarity index 100% rename from manifest/v1alpha/validation/test_data/multi_error.txt rename to validation/test_data/multi_error.txt From 760636ccd91e4c755fb93232d3bc578a733b7fdd Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Mon, 2 Oct 2023 20:00:51 +0200 Subject: [PATCH 12/20] rename object validation to struct --- manifest/v1alpha/project/validation.go | 2 +- validation/rules.go | 10 +++++----- validation/rules_test.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index e0abe168..b34746fa 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -6,7 +6,7 @@ import ( ) func validate(p Project) error { - v := validation.RulesForObject( + v := validation.RulesForStruct( validation.RulesForField[string]( "metadata.name", func() string { return p.Metadata.Name }, diff --git a/validation/rules.go b/validation/rules.go index e1a4bdf2..e4b04154 100644 --- a/validation/rules.go +++ b/validation/rules.go @@ -4,17 +4,17 @@ type fieldRules interface { Validate() error } -func RulesForObject(rules ...fieldRules) ObjectRules { - return ObjectRules{fieldRules: rules} +func RulesForStruct(rules ...fieldRules) StructRules { + return StructRules{fieldRules: rules} } -type ObjectRules struct { +type StructRules struct { fieldRules []fieldRules } -func (r ObjectRules) Validate() []error { +func (s StructRules) Validate() []error { var errors []error - for _, field := range r.fieldRules { + for _, field := range s.fieldRules { if err := field.Validate(); err != nil { errors = append(errors, err) } diff --git a/validation/rules_test.go b/validation/rules_test.go index ca4b7b18..cbf345dc 100644 --- a/validation/rules_test.go +++ b/validation/rules_test.go @@ -8,9 +8,9 @@ import ( "github.com/stretchr/testify/require" ) -func TestRulesForObject(t *testing.T) { +func TestRulesForStruct(t *testing.T) { t.Run("no errors", func(t *testing.T) { - r := RulesForObject( + r := RulesForStruct( RulesForField[string]("test", func() string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), ) @@ -21,7 +21,7 @@ func TestRulesForObject(t *testing.T) { t.Run("errors", func(t *testing.T) { err1 := errors.New("1") err2 := errors.New("2") - r := RulesForObject( + r := RulesForStruct( RulesForField[string]("test", func() string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), RulesForField[string]("test.name", func() string { return "name" }). From 0fadc494b115b1e377380a34db36da42893fe198 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Tue, 3 Oct 2023 10:05:41 +0200 Subject: [PATCH 13/20] fix error handling --- manifest/v1alpha/errors.go | 2 +- manifest/v1alpha/errors_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/manifest/v1alpha/errors.go b/manifest/v1alpha/errors.go index 4412c7af..85389444 100644 --- a/manifest/v1alpha/errors.go +++ b/manifest/v1alpha/errors.go @@ -61,7 +61,7 @@ func (o *ObjectError) MarshalJSON() ([]byte, error) { var errs []json.RawMessage for _, oErr := range o.Errors { switch v := oErr.(type) { - case validation.FieldError: + case validation.FieldError, *validation.FieldError: data, err := json.Marshal(v) if err != nil { return nil, err diff --git a/manifest/v1alpha/errors_test.go b/manifest/v1alpha/errors_test.go index 17759ed7..2d6ebf31 100644 --- a/manifest/v1alpha/errors_test.go +++ b/manifest/v1alpha/errors_test.go @@ -74,6 +74,10 @@ func TestObjectError_UnmarshalJSON(t *testing.T) { Errors: []string{"nested"}, }, errors.New("some error"), + &validation.FieldError{ + FieldPath: "metadata.name", + FieldValue: "my-project", + }, }, } data, err := json.Marshal(expected) @@ -86,6 +90,7 @@ func TestObjectError_UnmarshalJSON(t *testing.T) { assert.Equal(t, expected.Object, actual.Object) assert.Equal(t, expected.Errors[0], actual.Errors[0]) assert.Equal(t, expected.Errors[1].Error(), actual.Errors[1].Error()) + assert.Equal(t, *expected.Errors[2].(*validation.FieldError), actual.Errors[2]) } func expectedErrorOutput(t *testing.T, name string) string { From 4dd7c8ae1b0bee415c906723819921f43d7d0658 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Tue, 3 Oct 2023 13:00:45 +0200 Subject: [PATCH 14/20] change labels validation rule name --- manifest/v1alpha/labels.go | 2 +- manifest/v1alpha/project/validation.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifest/v1alpha/labels.go b/manifest/v1alpha/labels.go index baaefeea..32c2dbd8 100644 --- a/manifest/v1alpha/labels.go +++ b/manifest/v1alpha/labels.go @@ -16,7 +16,7 @@ type ( Value = string ) -func ValidationRule() validation.SingleRule[Labels] { +func ValidationRuleLabels() validation.SingleRule[Labels] { return func(v Labels) error { return v.Validate() } } diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index b34746fa..5f5418fb 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -23,7 +23,7 @@ func validate(p Project) error { "metadata.labels", func() v1alpha.Labels { return p.Metadata.Labels }, ). - With(v1alpha.ValidationRule()), + With(v1alpha.ValidationRuleLabels()), validation.RulesForField[string]( "spec.description", func() string { return p.Spec.Description }, From 907d1f9f28e884adccd070f2268e4c4df2f54821 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Wed, 4 Oct 2023 13:03:59 +0200 Subject: [PATCH 15/20] let compiler infer the type --- manifest/v1alpha/project/validation.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index 5f5418fb..911c41bd 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -7,24 +7,24 @@ import ( func validate(p Project) error { v := validation.RulesForStruct( - validation.RulesForField[string]( + validation.RulesForField( "metadata.name", func() string { return p.Metadata.Name }, ). With( validation.StringRequired(), validation.StringIsDNSSubdomain()), - validation.RulesForField[string]( + validation.RulesForField( "metadata.displayName", func() string { return p.Metadata.DisplayName }, ). With(validation.StringLength(0, 63)), - validation.RulesForField[v1alpha.Labels]( + validation.RulesForField( "metadata.labels", func() v1alpha.Labels { return p.Metadata.Labels }, ). With(v1alpha.ValidationRuleLabels()), - validation.RulesForField[string]( + validation.RulesForField( "spec.description", func() string { return p.Spec.Description }, ). From 3104c3de96b5e16be6f9567994b3708ac47a323c Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Wed, 4 Oct 2023 15:08:42 +0200 Subject: [PATCH 16/20] declare the function once --- manifest/v1alpha/project/validation.go | 50 +++++++++++++------------- validation/rules.go | 34 +++++++++--------- validation/rules_test.go | 34 +++++++++--------- 3 files changed, 60 insertions(+), 58 deletions(-) diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index 911c41bd..f97ce012 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -5,32 +5,32 @@ import ( "github.com/nobl9/nobl9-go/validation" ) +var validateProject = validation.RulesForStruct[Project]( + validation.RulesForField( + "metadata.name", + func(p Project) string { return p.Metadata.Name }, + ).With( + validation.StringRequired(), + validation.StringIsDNSSubdomain()), + validation.RulesForField( + "metadata.displayName", + func(p Project) string { return p.Metadata.DisplayName }, + ).With( + validation.StringLength(0, 63)), + validation.RulesForField( + "metadata.labels", + func(p Project) v1alpha.Labels { return p.Metadata.Labels }, + ).With( + v1alpha.ValidationRuleLabels()), + validation.RulesForField( + "spec.description", + func(p Project) string { return p.Spec.Description }, + ).With( + validation.StringDescription()), +).Validate + func validate(p Project) error { - v := validation.RulesForStruct( - validation.RulesForField( - "metadata.name", - func() string { return p.Metadata.Name }, - ). - With( - validation.StringRequired(), - validation.StringIsDNSSubdomain()), - validation.RulesForField( - "metadata.displayName", - func() string { return p.Metadata.DisplayName }, - ). - With(validation.StringLength(0, 63)), - validation.RulesForField( - "metadata.labels", - func() v1alpha.Labels { return p.Metadata.Labels }, - ). - With(v1alpha.ValidationRuleLabels()), - validation.RulesForField( - "spec.description", - func() string { return p.Spec.Description }, - ). - With(validation.StringDescription()), - ) - if errs := v.Validate(); len(errs) > 0 { + if errs := validateProject(p); len(errs) > 0 { return v1alpha.NewObjectError(p, errs) } return nil diff --git a/validation/rules.go b/validation/rules.go index e4b04154..33ffb6fd 100644 --- a/validation/rules.go +++ b/validation/rules.go @@ -1,21 +1,21 @@ package validation -type fieldRules interface { - Validate() error +type fieldRules[S any] interface { + Validate(s S) error } -func RulesForStruct(rules ...fieldRules) StructRules { - return StructRules{fieldRules: rules} +func RulesForStruct[S any](rules ...fieldRules[S]) StructRules[S] { + return StructRules[S]{fieldRules: rules} } -type StructRules struct { - fieldRules []fieldRules +type StructRules[S any] struct { + fieldRules []fieldRules[S] } -func (s StructRules) Validate() []error { +func (r StructRules[S]) Validate(st S) []error { var errors []error - for _, field := range s.fieldRules { - if err := field.Validate(); err != nil { + for _, field := range r.fieldRules { + if err := field.Validate(st); err != nil { errors = append(errors, err) } } @@ -23,25 +23,25 @@ func (s StructRules) Validate() []error { } // RulesForField creates a typed FieldRules instance for the field which access is defined through getter function. -func RulesForField[T any](fieldPath string, getter func() T) FieldRules[T] { - return FieldRules[T]{fieldPath: fieldPath, getter: getter} +func RulesForField[T, S any](fieldPath string, getter func(S) T) FieldRules[T, S] { + return FieldRules[T, S]{fieldPath: fieldPath, getter: getter} } // FieldRules is responsible for validating a single struct field. -type FieldRules[T any] struct { +type FieldRules[T, S any] struct { fieldPath string - getter func() T + getter func(S) T rules []Rule[T] predicates []func() bool } -func (r FieldRules[T]) Validate() error { +func (r FieldRules[T, S]) Validate(st S) error { for _, pred := range r.predicates { if pred != nil && !pred() { return nil } } - fv := r.getter() + fv := r.getter(st) var errors []error for i := range r.rules { if err := r.rules[i].Validate(fv); err != nil { @@ -54,12 +54,12 @@ func (r FieldRules[T]) Validate() error { return nil } -func (r FieldRules[T]) If(predicate func() bool) FieldRules[T] { +func (r FieldRules[T, S]) If(predicate func() bool) FieldRules[T, S] { r.predicates = append(r.predicates, predicate) return r } -func (r FieldRules[T]) With(rules ...Rule[T]) FieldRules[T] { +func (r FieldRules[T, S]) With(rules ...Rule[T]) FieldRules[T, S] { r.rules = append(r.rules, rules...) return r } diff --git a/validation/rules_test.go b/validation/rules_test.go index cbf345dc..4950acfc 100644 --- a/validation/rules_test.go +++ b/validation/rules_test.go @@ -10,26 +10,26 @@ import ( func TestRulesForStruct(t *testing.T) { t.Run("no errors", func(t *testing.T) { - r := RulesForStruct( - RulesForField[string]("test", func() string { return "test" }). + r := RulesForStruct[mockStruct]( + RulesForField[string]("test", func(m mockStruct) string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), ) - errs := r.Validate() + errs := r.Validate(mockStruct{}) assert.Empty(t, errs) }) t.Run("errors", func(t *testing.T) { err1 := errors.New("1") err2 := errors.New("2") - r := RulesForStruct( - RulesForField[string]("test", func() string { return "test" }). + r := RulesForStruct[mockStruct]( + RulesForField[string]("test", func(m mockStruct) string { return "test" }). With(SingleRule[string](func(v string) error { return nil })), - RulesForField[string]("test.name", func() string { return "name" }). + RulesForField[string]("test.name", func(m mockStruct) string { return "name" }). With(SingleRule[string](func(v string) error { return err1 })), - RulesForField[string]("test.display", func() string { return "display" }). + RulesForField[string]("test.display", func(m mockStruct) string { return "display" }). With(SingleRule[string](func(v string) error { return err2 })), ) - errs := r.Validate() + errs := r.Validate(mockStruct{}) require.Len(t, errs, 2) assert.Equal(t, []error{ &FieldError{ @@ -48,17 +48,17 @@ func TestRulesForStruct(t *testing.T) { func TestRulesForField(t *testing.T) { t.Run("no predicates, no error", func(t *testing.T) { - r := RulesForField[string]("test.path", func() string { return "path" }). + r := RulesForField[string]("test.path", func(m mockStruct) string { return "path" }). With(SingleRule[string](func(v string) error { return nil })) - err := r.Validate() + err := r.Validate(mockStruct{}) assert.NoError(t, err) }) t.Run("no predicates, validate", func(t *testing.T) { expectedErr := errors.New("ops!") - r := RulesForField[string]("test.path", func() string { return "path" }). + r := RulesForField[string]("test.path", func(m mockStruct) string { return "path" }). With(SingleRule[string](func(v string) error { return expectedErr })) - err := r.Validate() + err := r.Validate(mockStruct{}) require.Error(t, err) assert.Equal(t, FieldError{ FieldPath: "test.path", @@ -68,24 +68,24 @@ func TestRulesForField(t *testing.T) { }) t.Run("predicate matches, don't validate", func(t *testing.T) { - r := RulesForField[string]("test.path", func() string { return "value" }). + r := RulesForField[string]("test.path", func(m mockStruct) string { return "value" }). If(func() bool { return true }). If(func() bool { return true }). If(func() bool { return false }). With(SingleRule[string](func(v string) error { return errors.New("ops!") })) - err := r.Validate() + err := r.Validate(mockStruct{}) assert.NoError(t, err) }) t.Run("multiple rules", func(t *testing.T) { err1 := errors.New("oh no!") err2 := errors.New("ops!") - r := RulesForField[string]("test.path", func() string { return "value" }). + r := RulesForField[string]("test.path", func(m mockStruct) string { return "value" }). With(SingleRule[string](func(v string) error { return nil })). With(SingleRule[string](func(v string) error { return err1 })). With(SingleRule[string](func(v string) error { return nil })). With(SingleRule[string](func(v string) error { return err2 })) - err := r.Validate() + err := r.Validate(mockStruct{}) require.Error(t, err) assert.Equal(t, FieldError{ FieldPath: "test.path", @@ -94,3 +94,5 @@ func TestRulesForField(t *testing.T) { }, *err.(*FieldError)) }) } + +type mockStruct struct{} From e2de9a519e4ff871d60eef2069ecf2742b995ffe Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Tue, 3 Oct 2023 12:18:01 +0200 Subject: [PATCH 17/20] move service to separate pkg --- manifest/v1alpha/alert.go | 17 ++++-- manifest/v1alpha/parser/parser.go | 3 +- manifest/v1alpha/project/project.go | 20 +++---- manifest/v1alpha/service.go | 42 --------------- manifest/v1alpha/service/doc.go | 2 + manifest/v1alpha/service/example.yaml | 11 ++++ manifest/v1alpha/service/example_test.go | 53 +++++++++++++++++++ manifest/v1alpha/service/service.go | 49 +++++++++++++++++ .../v1alpha/{ => service}/service_object.go | 5 +- .../service/test_data/expected_error.txt | 14 +++++ manifest/v1alpha/service/validation.go | 44 +++++++++++++++ manifest/v1alpha/service/validation_test.go | 34 ++++++++++++ manifest/v1alpha/validator.go | 1 + sdk/client_test.go | 17 +++--- sdk/parser_test.go | 6 +-- 15 files changed, 247 insertions(+), 71 deletions(-) delete mode 100644 manifest/v1alpha/service.go create mode 100644 manifest/v1alpha/service/doc.go create mode 100644 manifest/v1alpha/service/example.yaml create mode 100644 manifest/v1alpha/service/example_test.go create mode 100644 manifest/v1alpha/service/service.go rename manifest/v1alpha/{ => service}/service_object.go (92%) create mode 100644 manifest/v1alpha/service/test_data/expected_error.txt create mode 100644 manifest/v1alpha/service/validation.go create mode 100644 manifest/v1alpha/service/validation_test.go diff --git a/manifest/v1alpha/alert.go b/manifest/v1alpha/alert.go index 48230fe2..9193fba8 100644 --- a/manifest/v1alpha/alert.go +++ b/manifest/v1alpha/alert.go @@ -1,6 +1,8 @@ package v1alpha -import "github.com/nobl9/nobl9-go/manifest" +import ( + "github.com/nobl9/nobl9-go/manifest" +) //go:generate go run ../../scripts/generate-object-impl.go Alert @@ -22,9 +24,9 @@ type AlertMetadata struct { // AlertSpec represents content of Alert's Spec type AlertSpec struct { - AlertPolicy AlertPolicyMetadata `json:"alertPolicy"` - SLO SLOMetadata `json:"slo"` - Service ServiceMetadata `json:"service"` + AlertPolicy AlertObjectMetadata `json:"alertPolicy"` + SLO AlertObjectMetadata `json:"slo"` + Service AlertObjectMetadata `json:"service"` Objective AlertObjective `json:"objective"` Severity string `json:"severity" validate:"required,severity" example:"High"` Status string `json:"status" example:"Resolved"` @@ -41,3 +43,10 @@ type AlertObjective struct { Name string `json:"name" validate:"omitempty"` DisplayName string `json:"displayName" validate:"omitempty"` } + +type AlertObjectMetadata struct { + Name string `json:"name"` + DisplayName string `json:"displayName,omitempty"` + Project string `json:"project,omitempty"` + Labels Labels `json:"labels,omitempty"` +} diff --git a/manifest/v1alpha/parser/parser.go b/manifest/v1alpha/parser/parser.go index 42199231..1afe7b7f 100644 --- a/manifest/v1alpha/parser/parser.go +++ b/manifest/v1alpha/parser/parser.go @@ -11,6 +11,7 @@ import ( "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/project" + "github.com/nobl9/nobl9-go/manifest/v1alpha/service" ) type unmarshalFunc func(v interface{}) error @@ -45,7 +46,7 @@ func parseObject(kind manifest.Kind, unmarshal unmarshalFunc) (manifest.Object, //exhaustive:enforce switch kind { case manifest.KindService: - return genericParseObject[v1alpha.Service](unmarshal) + return genericParseObject[service.Service](unmarshal) case manifest.KindSLO: return genericParseObject[v1alpha.SLO](unmarshal) case manifest.KindProject: diff --git a/manifest/v1alpha/project/project.go b/manifest/v1alpha/project/project.go index 0d1b75a2..90bb09fe 100644 --- a/manifest/v1alpha/project/project.go +++ b/manifest/v1alpha/project/project.go @@ -7,6 +7,16 @@ import ( //go:generate go run ../../../scripts/generate-object-impl.go Project +// New creates a new Project based on provided Metadata nad Spec. +func New(metadata Metadata, spec Spec) Project { + return Project{ + APIVersion: manifest.VersionV1alpha.String(), + Kind: manifest.KindProject, + Metadata: metadata, + Spec: spec, + } +} + // Project is the primary grouping primitive for manifest.Object. // Most objects are scoped to a certain Project. type Project struct { @@ -19,16 +29,6 @@ type Project struct { ManifestSource string `json:"manifestSrc,omitempty"` } -// New creates a new Project based on provided Metadata nad Spec. -func New(metadata Metadata, spec Spec) Project { - return Project{ - APIVersion: manifest.VersionV1alpha.String(), - Kind: manifest.KindProject, - Metadata: metadata, - Spec: spec, - } -} - // Metadata provides identity information for Project. type Metadata struct { Name string `json:"name" validate:"required,objectName" example:"name"` diff --git a/manifest/v1alpha/service.go b/manifest/v1alpha/service.go deleted file mode 100644 index b8fa3b3c..00000000 --- a/manifest/v1alpha/service.go +++ /dev/null @@ -1,42 +0,0 @@ -package v1alpha - -import ( - "github.com/nobl9/nobl9-go/manifest" -) - -//go:generate go run ../../scripts/generate-object-impl.go Service - -// Service struct which mapped one to one with kind: service yaml definition -type Service struct { - APIVersion string `json:"apiVersion"` - Kind manifest.Kind `json:"kind"` - Metadata ServiceMetadata `json:"metadata"` - Spec ServiceSpec `json:"spec"` - Status *ServiceStatus `json:"status,omitempty"` - - Organization string `json:"organization,omitempty"` - ManifestSource string `json:"manifestSrc,omitempty"` -} - -type ServiceMetadata struct { - Name string `json:"name" validate:"required,objectName"` - DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` - Project string `json:"project,omitempty" validate:"objectName"` - Labels Labels `json:"labels,omitempty" validate:"omitempty,labels"` -} - -// ServiceStatus represents content of Status optional for Service Object. -type ServiceStatus struct { - SloCount int `json:"sloCount"` -} - -// ServiceSpec represents content of Spec typical for Service Object. -type ServiceSpec struct { - Description string `json:"description" validate:"description" example:"Bleeding edge web app"` -} - -// ServiceWithSLOs struct which mapped one to one with kind: service and slo yaml definition. -type ServiceWithSLOs struct { - Service Service `json:"service"` - SLOs []SLO `json:"slos"` -} diff --git a/manifest/v1alpha/service/doc.go b/manifest/v1alpha/service/doc.go new file mode 100644 index 00000000..1e72836d --- /dev/null +++ b/manifest/v1alpha/service/doc.go @@ -0,0 +1,2 @@ +// Package service defines Service object definitions. +package service diff --git a/manifest/v1alpha/service/example.yaml b/manifest/v1alpha/service/example.yaml new file mode 100644 index 00000000..78acb517 --- /dev/null +++ b/manifest/v1alpha/service/example.yaml @@ -0,0 +1,11 @@ +apiVersion: n9/v1alpha +kind: Service +metadata: + name: my-service + displayName: My Service + project: default + labels: + team: [ green, orange ] + region: [ eu-central-1 ] +spec: + description: Example Service diff --git a/manifest/v1alpha/service/example_test.go b/manifest/v1alpha/service/example_test.go new file mode 100644 index 00000000..064fcce2 --- /dev/null +++ b/manifest/v1alpha/service/example_test.go @@ -0,0 +1,53 @@ +package service_test + +import ( + "context" + "log" + + "github.com/nobl9/nobl9-go/internal/examples" + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/manifest/v1alpha/service" +) + +func ExampleService() { + // Create the object: + myService := service.New( + service.Metadata{ + Name: "my-service", + DisplayName: "My Service", + Project: "default", + Labels: v1alpha.Labels{ + "team": []string{"green", "orange"}, + "region": []string{"eu-central-1"}, + }, + }, + service.Spec{ + Description: "Example service", + }, + ) + // Verify the object: + if err := myService.Validate(); err != nil { + log.Fatal("service validation failed, err: %w", err) + } + // Apply the object: + client := examples.GetOfflineEchoClient() + if err := client.ApplyObjects(context.Background(), []manifest.Object{myService}, false); err != nil { + log.Fatal("failed to apply service, err: %w", err) + } + // Output: + // apiVersion: n9/v1alpha + // kind: Service + // metadata: + // name: my-service + // displayName: My Service + // project: default + // labels: + // region: + // - eu-central-1 + // team: + // - green + // - orange + // spec: + // description: Example service +} diff --git a/manifest/v1alpha/service/service.go b/manifest/v1alpha/service/service.go new file mode 100644 index 00000000..fd13e9bf --- /dev/null +++ b/manifest/v1alpha/service/service.go @@ -0,0 +1,49 @@ +package service + +import ( + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" +) + +//go:generate go run ../../../scripts/generate-object-impl.go Service + +// New creates a new Service based on provided Metadata nad Spec. +func New(metadata Metadata, spec Spec) Service { + return Service{ + APIVersion: manifest.VersionV1alpha.String(), + Kind: manifest.KindService, + Metadata: metadata, + Spec: spec, + } +} + +// Service struct which mapped one to one with kind: service yaml definition +type Service struct { + APIVersion string `json:"apiVersion"` + Kind manifest.Kind `json:"kind"` + Metadata Metadata `json:"metadata"` + Spec Spec `json:"spec"` + Status *Status `json:"status,omitempty"` + + Organization string `json:"organization,omitempty"` + ManifestSource string `json:"manifestSrc,omitempty"` +} + +// Metadata provides identity information for Service. +type Metadata struct { + Name string `json:"name" validate:"required,objectName"` + DisplayName string `json:"displayName,omitempty" validate:"omitempty,min=0,max=63"` + Project string `json:"project,omitempty" validate:"objectName"` + Labels v1alpha.Labels `json:"labels,omitempty" validate:"omitempty,labels"` +} + +// Status holds dynamic fields returned when the Service is fetched from Nobl9 platform. +// Status is not part of the static object definition. +type Status struct { + SloCount int `json:"sloCount"` +} + +// Spec holds detailed information specific to Service. +type Spec struct { + Description string `json:"description" validate:"description" example:"Bleeding edge web app"` +} diff --git a/manifest/v1alpha/service_object.go b/manifest/v1alpha/service/service_object.go similarity index 92% rename from manifest/v1alpha/service_object.go rename to manifest/v1alpha/service/service_object.go index 77e93368..3d941d3d 100644 --- a/manifest/v1alpha/service_object.go +++ b/manifest/v1alpha/service/service_object.go @@ -1,13 +1,12 @@ // Code generated by "generate-object-impl Service"; DO NOT EDIT. -package v1alpha +package service import "github.com/nobl9/nobl9-go/manifest" // Ensure interfaces are implemented. var _ manifest.Object = Service{} var _ manifest.ProjectScopedObject = Service{} -var _ ObjectContext = Service{} func (s Service) GetVersion() string { return s.APIVersion @@ -22,7 +21,7 @@ func (s Service) GetName() string { } func (s Service) Validate() error { - return validator.Check(s) + return validate(s) } func (s Service) GetProject() string { diff --git a/manifest/v1alpha/service/test_data/expected_error.txt b/manifest/v1alpha/service/test_data/expected_error.txt new file mode 100644 index 00000000..326049c1 --- /dev/null +++ b/manifest/v1alpha/service/test_data/expected_error.txt @@ -0,0 +1,14 @@ +Validation for Service 'MY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICE' in project 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT' has failed for the following fields: + - 'metadata.name' with value 'MY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICEMY SERVICE...': + - length must be between 1 and 63 + - a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') + - 'metadata.displayName' with value 'my-servicemy-servicemy-servicemy-servicemy-servicemy-servicemy-servicemy-servicemy-servicemy-service': + - length must be between 0 and 63 + - 'metadata.project' with value 'MY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECTMY PROJECT...': + - length must be between 1 and 63 + - a DNS-1123 compliant name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') + - 'metadata.labels' with value '{"L O L":["dip","dip"]}': + - label key 'L O L' does not match the regex: ^\p{L}([_\-0-9\p{L}]*[0-9\p{L}])?$ + - 'spec.description' with value 'llllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll...': + - length must be between 0 and 1050 +Manifest source: /home/me/service.yaml \ No newline at end of file diff --git a/manifest/v1alpha/service/validation.go b/manifest/v1alpha/service/validation.go new file mode 100644 index 00000000..04153a31 --- /dev/null +++ b/manifest/v1alpha/service/validation.go @@ -0,0 +1,44 @@ +package service + +import ( + "github.com/nobl9/nobl9-go/manifest/v1alpha" + "github.com/nobl9/nobl9-go/validation" +) + +func validate(s Service) error { + v := validation.RulesForStruct( + validation.RulesForField[string]( + "metadata.name", + func() string { return s.Metadata.Name }, + ). + With( + validation.StringRequired(), + validation.StringIsDNSSubdomain()), + validation.RulesForField[string]( + "metadata.displayName", + func() string { return s.Metadata.DisplayName }, + ). + With(validation.StringLength(0, 63)), + validation.RulesForField[string]( + "metadata.project", + func() string { return s.Metadata.Project }, + ). + With( + validation.StringRequired(), + validation.StringIsDNSSubdomain()), + validation.RulesForField[v1alpha.Labels]( + "metadata.labels", + func() v1alpha.Labels { return s.Metadata.Labels }, + ). + With(v1alpha.ValidationRule()), + validation.RulesForField[string]( + "spec.description", + func() string { return s.Spec.Description }, + ). + With(validation.StringDescription()), + ) + if errs := v.Validate(); len(errs) > 0 { + return v1alpha.NewObjectError(s, errs) + } + return nil +} diff --git a/manifest/v1alpha/service/validation_test.go b/manifest/v1alpha/service/validation_test.go new file mode 100644 index 00000000..199934de --- /dev/null +++ b/manifest/v1alpha/service/validation_test.go @@ -0,0 +1,34 @@ +package service + +import ( + _ "embed" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" +) + +//go:embed test_data/expected_error.txt +var expectedError string + +func TestValidate_AllErrors(t *testing.T) { + err := validate(Service{ + Kind: manifest.KindService, + Metadata: Metadata{ + Name: strings.Repeat("MY SERVICE", 20), + DisplayName: strings.Repeat("my-service", 10), + Project: strings.Repeat("MY PROJECT", 20), + Labels: v1alpha.Labels{ + "L O L": []string{"dip", "dip"}, + }, + }, + Spec: Spec{ + Description: strings.Repeat("l", 2000), + }, + ManifestSource: "/home/me/service.yaml", + }) + assert.Equal(t, expectedError, err.Error()) +} diff --git a/manifest/v1alpha/validator.go b/manifest/v1alpha/validator.go index 485b852d..a1951044 100644 --- a/manifest/v1alpha/validator.go +++ b/manifest/v1alpha/validator.go @@ -1404,6 +1404,7 @@ func areSumoLogicTimesliceValuesEqual(sloSpec SLOSpec) bool { // haveAzureMonitorCountMetricSpecTheSameResourceIDAndMetricNamespace checks if good/bad query has the same resourceID // and metricNamespace as total query +// nolint: gocognit func haveAzureMonitorCountMetricSpecTheSameResourceIDAndMetricNamespace(sloSpec SLOSpec) bool { for _, objective := range sloSpec.Objectives { if objective.CountMetrics == nil { diff --git a/sdk/client_test.go b/sdk/client_test.go index b0ba36f0..a37307ea 100644 --- a/sdk/client_test.go +++ b/sdk/client_test.go @@ -24,22 +24,23 @@ import ( "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha" + v1alphaService "github.com/nobl9/nobl9-go/manifest/v1alpha/service" ) func TestClient_GetObjects(t *testing.T) { responsePayload := []manifest.Object{ - v1alpha.Service{ + v1alphaService.Service{ APIVersion: v1alpha.APIVersion, Kind: manifest.KindService, - Metadata: v1alpha.ServiceMetadata{ + Metadata: v1alphaService.Metadata{ Name: "service1", Project: "default", }, }, - v1alpha.Service{ + v1alphaService.Service{ APIVersion: v1alpha.APIVersion, Kind: manifest.KindService, - Metadata: v1alpha.ServiceMetadata{ + Metadata: v1alphaService.Metadata{ Name: "service2", Project: "default", }, @@ -146,10 +147,10 @@ func TestClient_GetObjects_UserGroupsEndpoint(t *testing.T) { func TestClient_ApplyObjects(t *testing.T) { requestPayload := []manifest.Object{ - v1alpha.Service{ + v1alphaService.Service{ APIVersion: v1alpha.APIVersion, Kind: manifest.KindService, - Metadata: v1alpha.ServiceMetadata{ + Metadata: v1alphaService.Metadata{ Name: "service1", Project: "default", }, @@ -186,10 +187,10 @@ func TestClient_ApplyObjects(t *testing.T) { func TestClient_DeleteObjects(t *testing.T) { requestPayload := []manifest.Object{ - v1alpha.Service{ + v1alphaService.Service{ APIVersion: v1alpha.APIVersion, Kind: manifest.KindService, - Metadata: v1alpha.ServiceMetadata{ + Metadata: v1alphaService.Metadata{ Name: "service1", Project: "default", }, diff --git a/sdk/parser_test.go b/sdk/parser_test.go index 769d4e83..0d8c808c 100644 --- a/sdk/parser_test.go +++ b/sdk/parser_test.go @@ -9,8 +9,8 @@ import ( "github.com/stretchr/testify/require" "github.com/nobl9/nobl9-go/manifest" - "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/project" + v1alphaService "github.com/nobl9/nobl9-go/manifest/v1alpha/service" ) //go:embed test_data/parser @@ -132,9 +132,9 @@ func TestDecodeSingle(t *testing.T) { }) t.Run("invalid type, return error", func(t *testing.T) { - _, err := DecodeObject[v1alpha.Service](readInputFile(t, "single_project.yaml")) + _, err := DecodeObject[v1alphaService.Service](readInputFile(t, "single_project.yaml")) require.Error(t, err) - assert.EqualError(t, err, "object of type project.Project is not of type v1alpha.Service") + assert.EqualError(t, err, "object of type project.Project is not of type service.Service") }) } From 0c4abe57a06bfa9ef1418b50eb01f792e12d36db Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Tue, 3 Oct 2023 12:37:06 +0200 Subject: [PATCH 18/20] extract common validation --- manifest/v1alpha/validation.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 manifest/v1alpha/validation.go diff --git a/manifest/v1alpha/validation.go b/manifest/v1alpha/validation.go new file mode 100644 index 00000000..f9bbac81 --- /dev/null +++ b/manifest/v1alpha/validation.go @@ -0,0 +1,28 @@ +package v1alpha + +import "github.com/nobl9/nobl9-go/validation" + +func FieldRuleMetdataName(getter func() string) validation.FieldRules[string] { + return validation.RulesForField[string]("metadata.name", getter). + With(validation.StringRequired(), validation.StringIsDNSSubdomain()) +} + +func FieldRuleMetadataDisplayName(getter func() string) validation.FieldRules[string] { + return validation.RulesForField[string]("metadata.displayName", getter). + With(validation.StringLength(0, 63)) +} + +func FieldRuleMetdataProject(getter func() string) validation.FieldRules[string] { + return validation.RulesForField[string]("metadata.project", getter). + With(validation.StringRequired(), validation.StringIsDNSSubdomain()) +} + +func FieldRuleMetadataLabels(getter func() Labels) validation.FieldRules[Labels] { + return validation.RulesForField[Labels]("metadata.labels", getter). + With(ValidationRule()) +} + +func FieldRuleMetadataDescription(getter func() string) validation.FieldRules[string] { + return validation.RulesForField[string]("spec.description", getter). + With(validation.StringDescription()) +} From 1ccc0477908f97874eb7dab6e70cce2629466891 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Wed, 4 Oct 2023 15:13:20 +0200 Subject: [PATCH 19/20] use common validators --- manifest/v1alpha/project/validation.go | 25 +++------------- manifest/v1alpha/service/validation.go | 41 ++++++-------------------- manifest/v1alpha/validation.go | 12 ++++---- 3 files changed, 19 insertions(+), 59 deletions(-) diff --git a/manifest/v1alpha/project/validation.go b/manifest/v1alpha/project/validation.go index f97ce012..345ee1f6 100644 --- a/manifest/v1alpha/project/validation.go +++ b/manifest/v1alpha/project/validation.go @@ -6,27 +6,10 @@ import ( ) var validateProject = validation.RulesForStruct[Project]( - validation.RulesForField( - "metadata.name", - func(p Project) string { return p.Metadata.Name }, - ).With( - validation.StringRequired(), - validation.StringIsDNSSubdomain()), - validation.RulesForField( - "metadata.displayName", - func(p Project) string { return p.Metadata.DisplayName }, - ).With( - validation.StringLength(0, 63)), - validation.RulesForField( - "metadata.labels", - func(p Project) v1alpha.Labels { return p.Metadata.Labels }, - ).With( - v1alpha.ValidationRuleLabels()), - validation.RulesForField( - "spec.description", - func(p Project) string { return p.Spec.Description }, - ).With( - validation.StringDescription()), + v1alpha.FieldRuleMetadataName(func(p Project) string { return p.Metadata.Name }), + v1alpha.FieldRuleMetadataDisplayName(func(p Project) string { return p.Metadata.DisplayName }), + v1alpha.FieldRuleMetadataLabels(func(p Project) v1alpha.Labels { return p.Metadata.Labels }), + v1alpha.FieldRuleSpecDescription(func(p Project) string { return p.Spec.Description }), ).Validate func validate(p Project) error { diff --git a/manifest/v1alpha/service/validation.go b/manifest/v1alpha/service/validation.go index 04153a31..1189d422 100644 --- a/manifest/v1alpha/service/validation.go +++ b/manifest/v1alpha/service/validation.go @@ -5,39 +5,16 @@ import ( "github.com/nobl9/nobl9-go/validation" ) +var validateService = validation.RulesForStruct[Service]( + v1alpha.FieldRuleMetadataName(func(s Service) string { return s.Metadata.Name }), + v1alpha.FieldRuleMetadataDisplayName(func(s Service) string { return s.Metadata.DisplayName }), + v1alpha.FieldRuleMetadataProject(func(s Service) string { return s.Metadata.Project }), + v1alpha.FieldRuleMetadataLabels(func(s Service) v1alpha.Labels { return s.Metadata.Labels }), + v1alpha.FieldRuleSpecDescription(func(s Service) string { return s.Spec.Description }), +).Validate + func validate(s Service) error { - v := validation.RulesForStruct( - validation.RulesForField[string]( - "metadata.name", - func() string { return s.Metadata.Name }, - ). - With( - validation.StringRequired(), - validation.StringIsDNSSubdomain()), - validation.RulesForField[string]( - "metadata.displayName", - func() string { return s.Metadata.DisplayName }, - ). - With(validation.StringLength(0, 63)), - validation.RulesForField[string]( - "metadata.project", - func() string { return s.Metadata.Project }, - ). - With( - validation.StringRequired(), - validation.StringIsDNSSubdomain()), - validation.RulesForField[v1alpha.Labels]( - "metadata.labels", - func() v1alpha.Labels { return s.Metadata.Labels }, - ). - With(v1alpha.ValidationRule()), - validation.RulesForField[string]( - "spec.description", - func() string { return s.Spec.Description }, - ). - With(validation.StringDescription()), - ) - if errs := v.Validate(); len(errs) > 0 { + if errs := validateService(s); len(errs) > 0 { return v1alpha.NewObjectError(s, errs) } return nil diff --git a/manifest/v1alpha/validation.go b/manifest/v1alpha/validation.go index f9bbac81..950a867f 100644 --- a/manifest/v1alpha/validation.go +++ b/manifest/v1alpha/validation.go @@ -2,27 +2,27 @@ package v1alpha import "github.com/nobl9/nobl9-go/validation" -func FieldRuleMetdataName(getter func() string) validation.FieldRules[string] { +func FieldRuleMetadataName[S any](getter func(S) string) validation.FieldRules[string, S] { return validation.RulesForField[string]("metadata.name", getter). With(validation.StringRequired(), validation.StringIsDNSSubdomain()) } -func FieldRuleMetadataDisplayName(getter func() string) validation.FieldRules[string] { +func FieldRuleMetadataDisplayName[S any](getter func(S) string) validation.FieldRules[string, S] { return validation.RulesForField[string]("metadata.displayName", getter). With(validation.StringLength(0, 63)) } -func FieldRuleMetdataProject(getter func() string) validation.FieldRules[string] { +func FieldRuleMetadataProject[S any](getter func(S) string) validation.FieldRules[string, S] { return validation.RulesForField[string]("metadata.project", getter). With(validation.StringRequired(), validation.StringIsDNSSubdomain()) } -func FieldRuleMetadataLabels(getter func() Labels) validation.FieldRules[Labels] { +func FieldRuleMetadataLabels[S any](getter func(S) Labels) validation.FieldRules[Labels, S] { return validation.RulesForField[Labels]("metadata.labels", getter). - With(ValidationRule()) + With(ValidationRuleLabels()) } -func FieldRuleMetadataDescription(getter func() string) validation.FieldRules[string] { +func FieldRuleSpecDescription[S any](getter func(S) string) validation.FieldRules[string, S] { return validation.RulesForField[string]("spec.description", getter). With(validation.StringDescription()) } From 956b8c4028f771656d80e41d9b8172e32f3b81f4 Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus Date: Wed, 4 Oct 2023 20:23:40 +0200 Subject: [PATCH 20/20] revert changes --- manifest/v1alpha/alert.go | 4 +--- manifest/v1alpha/alert_policy.go | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/manifest/v1alpha/alert.go b/manifest/v1alpha/alert.go index 9193fba8..34b3bcb6 100644 --- a/manifest/v1alpha/alert.go +++ b/manifest/v1alpha/alert.go @@ -1,8 +1,6 @@ package v1alpha -import ( - "github.com/nobl9/nobl9-go/manifest" -) +import "github.com/nobl9/nobl9-go/manifest" //go:generate go run ../../scripts/generate-object-impl.go Alert diff --git a/manifest/v1alpha/alert_policy.go b/manifest/v1alpha/alert_policy.go index bceaac55..b1fdd403 100644 --- a/manifest/v1alpha/alert_policy.go +++ b/manifest/v1alpha/alert_policy.go @@ -26,7 +26,7 @@ type AlertPolicyMetadata struct { // AlertPolicySpec represents content of AlertPolicy's Spec. type AlertPolicySpec struct { - Description string `json:"description" validate:"description" example:"Message budget is at risk"` + Description string `json:"description" validate:"description" example:"Error budget is at risk"` Severity string `json:"severity" validate:"required,severity" example:"High"` CoolDownDuration string `json:"coolDown,omitempty" validate:"omitempty,validDuration,nonNegativeDuration,durationAtLeast=5m" example:"5m"` //nolint:lll Conditions []AlertCondition `json:"conditions" validate:"required,min=1,dive"`