diff --git a/api/v1/ratelimitpolicy_types.go b/api/v1/ratelimitpolicy_types.go index 4c47f9d66..aadc7c3f8 100644 --- a/api/v1/ratelimitpolicy_types.go +++ b/api/v1/ratelimitpolicy_types.go @@ -153,6 +153,9 @@ func (p *RateLimitPolicy) Kind() string { // +kubebuilder:validation:XValidation:rule="!(has(self.defaults) && has(self.limits))",message="Implicit and explicit defaults are mutually exclusive" // +kubebuilder:validation:XValidation:rule="!(has(self.defaults) && has(self.overrides))",message="Overrides and explicit defaults are mutually exclusive" // +kubebuilder:validation:XValidation:rule="!(has(self.overrides) && has(self.limits))",message="Overrides and implicit defaults are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.overrides) || has(self.defaults)) ? has(self.limits) && size(self.limits) > 0 : true",message="At least one spec.limits must be defined" +// +kubebuilder:validation:XValidation:rule="has(self.overrides) ? has(self.overrides.limits) && size(self.overrides.limits) > 0 : true",message="At least one spec.overrides.limits must be defined" +// +kubebuilder:validation:XValidation:rule="has(self.defaults) ? has(self.defaults.limits) && size(self.defaults.limits) > 0 : true",message="At least one spec.defaults.limits must be defined" type RateLimitPolicySpec struct { // Reference to the object to which this policy applies. // +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'" diff --git a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml index d58540342..6e82e5b49 100644 --- a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml +++ b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml @@ -109,7 +109,7 @@ metadata: capabilities: Basic Install categories: Integration & Delivery containerImage: quay.io/kuadrant/kuadrant-operator:latest - createdAt: "2024-11-13T16:31:14Z" + createdAt: "2024-11-18T10:54:46Z" description: A Kubernetes Operator to manage the lifecycle of the Kuadrant system operators.operatorframework.io/builder: operator-sdk-v1.32.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml b/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml index a35db04e4..e1b4e85af 100644 --- a/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml +++ b/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml @@ -392,6 +392,15 @@ spec: rule: '!(has(self.defaults) && has(self.overrides))' - message: Overrides and implicit defaults are mutually exclusive rule: '!(has(self.overrides) && has(self.limits))' + - message: At least one spec.limits must be defined + rule: '!(has(self.overrides) || has(self.defaults)) ? has(self.limits) + && size(self.limits) > 0 : true' + - message: At least one spec.overrides.limits must be defined + rule: 'has(self.overrides) ? has(self.overrides.limits) && size(self.overrides.limits) + > 0 : true' + - message: At least one spec.defaults.limits must be defined + rule: 'has(self.defaults) ? has(self.defaults.limits) && size(self.defaults.limits) + > 0 : true' status: properties: conditions: diff --git a/charts/kuadrant-operator/templates/manifests.yaml b/charts/kuadrant-operator/templates/manifests.yaml index db39b9336..36a1019f0 100644 --- a/charts/kuadrant-operator/templates/manifests.yaml +++ b/charts/kuadrant-operator/templates/manifests.yaml @@ -7994,6 +7994,15 @@ spec: rule: '!(has(self.defaults) && has(self.overrides))' - message: Overrides and implicit defaults are mutually exclusive rule: '!(has(self.overrides) && has(self.limits))' + - message: At least one spec.limits must be defined + rule: '!(has(self.overrides) || has(self.defaults)) ? has(self.limits) + && size(self.limits) > 0 : true' + - message: At least one spec.overrides.limits must be defined + rule: 'has(self.overrides) ? has(self.overrides.limits) && size(self.overrides.limits) + > 0 : true' + - message: At least one spec.defaults.limits must be defined + rule: 'has(self.defaults) ? has(self.defaults.limits) && size(self.defaults.limits) + > 0 : true' status: properties: conditions: diff --git a/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml b/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml index 9dbab91d8..4f11d07fe 100644 --- a/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml +++ b/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml @@ -391,6 +391,15 @@ spec: rule: '!(has(self.defaults) && has(self.overrides))' - message: Overrides and implicit defaults are mutually exclusive rule: '!(has(self.overrides) && has(self.limits))' + - message: At least one spec.limits must be defined + rule: '!(has(self.overrides) || has(self.defaults)) ? has(self.limits) + && size(self.limits) > 0 : true' + - message: At least one spec.overrides.limits must be defined + rule: 'has(self.overrides) ? has(self.overrides.limits) && size(self.overrides.limits) + > 0 : true' + - message: At least one spec.defaults.limits must be defined + rule: 'has(self.defaults) ? has(self.defaults.limits) && size(self.defaults.limits) + > 0 : true' status: properties: conditions: diff --git a/tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go b/tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go index 7ba2a736d..55d905470 100644 --- a/tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go +++ b/tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go @@ -920,7 +920,13 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { Context("Spec TargetRef Validations", func() { It("Valid policy targeting HTTPRoute", func(ctx SpecContext) { - policy := policyFactory() + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = map[string]kuadrantv1.Limit{ + "implicit": { + Rates: []kuadrantv1.Rate{{Limit: 2, Window: kuadrantv1.Duration("20s")}}, + }, + } + }) err := k8sClient.Create(ctx, policy) Expect(err).To(BeNil()) }, testTimeOut) @@ -928,6 +934,11 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { It("Valid policy targeting Gateway", func(ctx SpecContext) { policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.Limits = map[string]kuadrantv1.Limit{ + "implicit": { + Rates: []kuadrantv1.Rate{{Limit: 2, Window: kuadrantv1.Duration("20s")}}, + }, + } }) err := k8sClient.Create(ctx, policy) Expect(err).To(BeNil()) @@ -952,6 +963,85 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { }, testTimeOut) }) + Context("Limits missing from configuration", func() { + It("Missing limits object", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = nil + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.limits must be defined")).To(BeTrue()) + }, testTimeOut) + + It("Empty limits object created", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = map[string]kuadrantv1.Limit{} + + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.limits must be defined")).To(BeTrue()) + }, testTimeOut) + + It("Missing defaults.limits object", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = nil + policy.Spec.Defaults = &kuadrantv1.MergeableRateLimitPolicySpec{ + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: nil, + }, + } + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.defaults.limits must be defined")).To(BeTrue()) + }, testTimeOut) + + It("Empty defaults.limits object created", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = nil + policy.Spec.Defaults = &kuadrantv1.MergeableRateLimitPolicySpec{ + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: map[string]kuadrantv1.Limit{}, + }, + } + + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.defaults.limits must be defined")).To(BeTrue()) + }, testTimeOut) + + It("Missing overrides.limits object", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = nil + policy.Spec.Overrides = &kuadrantv1.MergeableRateLimitPolicySpec{ + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: nil, + }, + } + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.overrides.limits must be defined")).To(BeTrue()) + }, testTimeOut) + + It("Empty overrides.limits object created", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) { + policy.Spec.Limits = nil + policy.Spec.Overrides = &kuadrantv1.MergeableRateLimitPolicySpec{ + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: map[string]kuadrantv1.Limit{}, + }, + } + + }) + err := k8sClient.Create(ctx, policy) + Expect(err).To(Not(BeNil())) + Expect(strings.Contains(err.Error(), "At least one spec.overrides.limits must be defined")).To(BeTrue()) + }, testTimeOut) + }) + Context("Defaults / Override validation", func() { It("Valid - only implicit defaults defined", func(ctx SpecContext) { policy := policyFactory(func(policy *kuadrantv1.RateLimitPolicy) {