diff --git a/metrics-operator/api/v1alpha3/keptnmetric_types.go b/metrics-operator/api/v1alpha3/keptnmetric_types.go index da1f05d049..85f1b3628c 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_types.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_types.go @@ -58,6 +58,8 @@ type RangeSpec struct { // Interval specifies the duration of the time interval for the data query // +kubebuilder:default:="5m" Interval string `json:"interval,omitempty"` + // Step represents the query resolution step width for the data query + Step string `json:"step,omitempty"` } // +kubebuilder:object:root=true diff --git a/metrics-operator/api/v1alpha3/keptnmetric_webhook.go b/metrics-operator/api/v1alpha3/keptnmetric_webhook.go index eeaba7df38..c3d3baa174 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_webhook.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_webhook.go @@ -64,15 +64,17 @@ func (r *KeptnMetric) ValidateDelete() error { } func (s *KeptnMetric) validateKeptnMetric() error { - var allErrs field.ErrorList //defined as a list to allow returning multiple validation errors + var allErrs field.ErrorList // defined as a list to allow returning multiple validation errors var err *field.Error if err = s.validateRangeInterval(); err != nil { allErrs = append(allErrs, err) } + if err = s.validateRangeStep(); err != nil { + allErrs = append(allErrs, err) + } if len(allErrs) == 0 { return nil } - return apierrors.NewInvalid( schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, s.Name, @@ -93,3 +95,18 @@ func (s *KeptnMetric) validateRangeInterval() *field.Error { } return nil } + +func (s *KeptnMetric) validateRangeStep() *field.Error { + if s.Spec.Range == nil || s.Spec.Range.Step == "" { + return nil + } + _, err := time.ParseDuration(s.Spec.Range.Step) + if err != nil { + return field.Invalid( + field.NewPath("spec").Child("range").Child("step"), + s.Spec.Range.Step, + errors.New("Forbidden! The time interval cannot be parsed. Please check for suitable conventions").Error(), + ) + } + return nil +} diff --git a/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go b/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go index 6561dc0f16..9866c28109 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go @@ -141,3 +141,184 @@ func TestKeptnMetric_validateRangeInterval(t *testing.T) { }) } } + +func TestKeptnMetric_validateRangeStep(t *testing.T) { + + tests := []struct { + name string + verb string + Spec KeptnMetricSpec + want error + oldSpec runtime.Object + }{ + { + name: "create-with-wrong-step", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "create-with-wrong-step", + field.ErrorList{ + field.Invalid( + field.NewPath("spec").Child("range").Child("step"), + "1mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + }, + ), + }, + { + name: "create-with-empty-step", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "", + }, + }, + }, + { + name: "create-with-right-step", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + }, + }, + }, + { + name: "create-with-wrong-interval-right-step", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5mins", + Step: "1m", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "create-with-wrong-interval-right-step", + field.ErrorList{ + field.Invalid( + field.NewPath("spec").Child("range").Child("interval"), + "5mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + }, + ), + }, + { + name: "create-with-wrong-interval-wrong-step", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5mins", + Step: "1mins", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "create-with-wrong-interval-wrong-step", + field.ErrorList{ + field.Invalid( + field.NewPath("spec").Child("range").Child("interval"), + "5mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + field.Invalid( + field.NewPath("spec").Child("range").Child("step"), + "1mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + }, + ), + }, + { + name: "update-with-right-step", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + }, + }, + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + }, + }, + }, + }, + { + name: "update-with-wrong-step", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "update-with-wrong-step", + field.ErrorList{ + field.Invalid( + field.NewPath("spec").Child("range").Child("step"), + "1mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + }, + ), + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + }, + }, + }, + }, + { + name: "delete", + verb: "delete", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var s *KeptnMetric + if tt.Spec.Range == nil { + s = &KeptnMetric{ + ObjectMeta: metav1.ObjectMeta{Name: tt.name}, + Spec: KeptnMetricSpec{Range: tt.Spec.Range}, + } + } else { + s = &KeptnMetric{ + ObjectMeta: metav1.ObjectMeta{Name: tt.name}, + Spec: KeptnMetricSpec{Range: &RangeSpec{Interval: tt.Spec.Range.Interval, Step: tt.Spec.Range.Step}}, + } + } + var err error + switch tt.verb { + case "create": + err = s.ValidateCreate() + case "update": + err = s.ValidateUpdate(tt.oldSpec) + case "delete": + err = s.ValidateDelete() + } + if tt.want == nil { + require.Nil(t, err) + } else { + require.Equal(t, tt.want, err) + } + }) + } +} diff --git a/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml b/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml index fafde197b0..d06326ce49 100644 --- a/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml +++ b/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml @@ -224,6 +224,10 @@ spec: description: Interval specifies the duration of the time interval for the data query type: string + step: + description: Step represents the query resolution step width for + the data query + type: string type: object required: - fetchIntervalSeconds diff --git a/test/integration/metrics/01-teststep-install.yaml b/test/integration/metrics/01-teststep-install.yaml index 63bd101959..df18fd2531 100644 --- a/test/integration/metrics/01-teststep-install.yaml +++ b/test/integration/metrics/01-teststep-install.yaml @@ -5,6 +5,7 @@ apply: - goodmetric2.yaml - goodmetric3.yaml - goodmetric4.yaml + - goodmetric5.yaml commands: - - command: kubectl apply -f badmetric.yaml + - command: kubectl apply -f badmetric1.yaml badmetric2.yaml ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test diff --git a/test/integration/metrics/02-teststep-assert.yaml b/test/integration/metrics/02-teststep-assert.yaml index f81d486c75..800b17487b 100644 --- a/test/integration/metrics/02-teststep-assert.yaml +++ b/test/integration/metrics/02-teststep-assert.yaml @@ -1,9 +1,11 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep error: # this checks that kubectl get resource fails, AKA bad CRD not added - - badmetric.yaml + - badmetric1.yaml + - badmetric2.yaml assert: # this checks that kubectl get resource succeeds - goodmetric1.yaml - goodmetric2.yaml - goodmetric3.yaml - goodmetric4.yaml + - goodmetric5.yaml diff --git a/test/integration/metrics/03-teststep-retrieve-metrics-via-api.yaml b/test/integration/metrics/03-teststep-retrieve-metrics-via-api.yaml index 5041c54e85..6f140506ec 100644 --- a/test/integration/metrics/03-teststep-retrieve-metrics-via-api.yaml +++ b/test/integration/metrics/03-teststep-retrieve-metrics-via-api.yaml @@ -5,3 +5,4 @@ commands: - script: ./retrieve-metrics.sh "podtato-head2" - script: ./retrieve-metrics.sh "podtato-head3" - script: ./retrieve-metrics.sh "podtato-head4" + - script: ./retrieve-metrics.sh "podtato-head5" diff --git a/test/integration/metrics/badmetric.yaml b/test/integration/metrics/badmetric1.yaml similarity index 100% rename from test/integration/metrics/badmetric.yaml rename to test/integration/metrics/badmetric1.yaml diff --git a/test/integration/metrics/badmetric2.yaml b/test/integration/metrics/badmetric2.yaml new file mode 100644 index 0000000000..05d6088d65 --- /dev/null +++ b/test/integration/metrics/badmetric2.yaml @@ -0,0 +1,12 @@ +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetric +metadata: + name: podtato-head1 +spec: + provider: + name: "my-provider" + query: "sum(kube_pod_container_resource_limits{resource='cpu'})" + fetchIntervalSeconds: 5 + range: + interval: "5m" + step: "1mins" diff --git a/test/integration/metrics/goodmetric5.yaml b/test/integration/metrics/goodmetric5.yaml new file mode 100644 index 0000000000..c75927e64c --- /dev/null +++ b/test/integration/metrics/goodmetric5.yaml @@ -0,0 +1,12 @@ +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetric +metadata: + name: podtato-head5 +spec: + provider: + name: "my-provider2" + query: "sum(kube_pod_container_resource_limits{resource='cpu'})" + fetchIntervalSeconds: 5 + range: + interval: "5m" + step: "1m"