refactor: common missing dependency errors

Signed-off-by: KevFan <[email protected]>

controllers/auth_policies_validator.go

@@ -17,6 +17,7 @@ import (
 
 type AuthPolicyValidator struct {
 	isGatewayAPIInstalled        bool
+	isGatewayProviderInstalled   bool
 	isAuthorinoOperatorInstalled bool
 }
 
@@ -45,12 +46,17 @@ func (r *AuthPolicyValidator) Validate(ctx context.Context, _ []controller.Resou
 
 	state.Store(StateAuthPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) {
 		if !r.isGatewayAPIInstalled {
-			return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API")
+			return policy.GetLocator(), kuadrant.MissingGatewayAPIError()
+		}
+
+		if !r.isGatewayProviderInstalled {
+			return policy.GetLocator(), kuadrant.MissingGatewayProviderError()
 		}
 
 		if !r.isAuthorinoOperatorInstalled {
-			return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Authorino Operator")
+			return policy.GetLocator(), kuadrant.MissingAuthorinoOperatorError()
 		}
+
 		var err error
 		if len(policy.GetTargetRefs()) > 0 && len(topology.Targetables().Children(policy)) == 0 {
 			ref := policy.GetTargetRefs()[0]

controllers/data_plane_policies_workflow.go

@@ -55,10 +55,11 @@ var (
 //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update
 
 func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isGatewayAPInstalled, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *controller.Workflow {
+	isGatewayProviderInstalled := isIstioInstalled || isEnvoyGatewayInstalled
 	dataPlanePoliciesValidation := &controller.Workflow{
 		Tasks: []controller.ReconcileFunc{
-			(&AuthPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled}).Subscription().Reconcile,
-			(&RateLimitPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled}).Subscription().Reconcile,
+			(&AuthPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled, isGatewayProviderInstalled: isGatewayProviderInstalled}).Subscription().Reconcile,
+			(&RateLimitPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled, isGatewayProviderInstalled: isGatewayProviderInstalled}).Subscription().Reconcile,
 		},
 	}
 

controllers/dnspolicies_validator.go

@@ -52,11 +52,11 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso
 
 	state.Store(StateDNSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) {
 		if !r.isGatewayAPIInstalled {
-			return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API")
+			return p.GetLocator(), kuadrant.MissingGatewayAPIError()
 		}
 
 		if !r.isDNSOperatorInstalled {
-			return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("DNS Operator")
+			return p.GetLocator(), kuadrant.MissingDNSOperatorError()
 		}
 
 		policy := p.(*kuadrantv1.DNSPolicy)

controllers/kuadrant_status_updater.go

@@ -29,7 +29,7 @@ const (
 type KuadrantStatusUpdater struct {
 	Client                       *dynamic.DynamicClient
 	isGatwayAPIInstalled         bool
-	HasGateway                   bool
+	isGatewayProviderInstalled   bool
 	isLimitadorOperatorInstalled bool
 	isAuthorinoOperatorInstalled bool
 }
@@ -38,7 +38,7 @@ func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isGatewayAPIInstall
 	return &KuadrantStatusUpdater{
 		Client:                       client,
 		isGatwayAPIInstalled:         isGatewayAPIInstalled,
-		HasGateway:                   isGatewayProviderInstalled,
+		isGatewayProviderInstalled:   isGatewayProviderInstalled,
 		isLimitadorOperatorInstalled: isLimitadorOperatorInstalled,
 		isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled,
 	}
@@ -129,23 +129,34 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log
 	}
 
 	if !r.isGatwayAPIInstalled {
+		err := kuadrant.MissingGatewayAPIError()
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "GatewayAPI"
-		cond.Message = kuadrant.NewErrDependencyNotInstalled("Gateway API").Error()
+		cond.Reason = string(err.Reason())
+		cond.Message = err.Error()
 		return cond
 	}
 
-	if !r.HasGateway {
+	if !r.isGatewayProviderInstalled {
+		err := kuadrant.MissingGatewayProviderError()
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "GatewayAPIProviderNotFound"
-		cond.Message = kuadrant.NewErrDependencyNotInstalled("GatewayAPI provider").Error()
+		cond.Reason = string(err.Reason())
+		cond.Message = err.Error()
 		return cond
 	}
 
 	if !r.isLimitadorOperatorInstalled {
+		err := kuadrant.MissingLimitadorOperatorError()
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "LimitadorAPIProviderNotFound"
-		cond.Message = kuadrant.NewErrDependencyNotInstalled("Limitador Operator").Error()
+		cond.Reason = string(err.Reason())
+		cond.Message = err.Error()
+		return cond
+	}
+
+	if !r.isAuthorinoOperatorInstalled {
+		err := kuadrant.MissingAuthorinoOperatorError()
+		cond.Status = metav1.ConditionFalse
+		cond.Reason = string(err.Reason())
+		cond.Message = err.Error()
 		return cond
 	}
 
@@ -173,7 +184,7 @@ func checkLimitadorReady(topology *machinery.Topology, logger logr.Logger) *stri
 		return ptr.To("limitador resoure not in topology")
 	}
 
-	statusConditionReady := meta.FindStatusCondition(limitadorObj.Status.Conditions, "Ready")
+	statusConditionReady := meta.FindStatusCondition(limitadorObj.Status.Conditions, limitadorv1alpha1.StatusConditionReady)
 	if statusConditionReady == nil {
 		return ptr.To("Ready condition not found")
 	}
@@ -191,7 +202,7 @@ func checkAuthorinoAvailable(topology *machinery.Topology, logger logr.Logger) *
 		return ptr.To("authorino resoure not in topology")
 	}
 
-	readyCondition := authorino.FindAuthorinoStatusCondition(authorinoObj.Status.Conditions, limitadorv1alpha1.StatusConditionReady)
+	readyCondition := authorino.FindAuthorinoStatusCondition(authorinoObj.Status.Conditions, "Ready")
 	if readyCondition == nil {
 		return ptr.To("Ready condition not found")
 	}

controllers/ratelimit_policies_validator.go

@@ -18,6 +18,7 @@ import (
 type RateLimitPolicyValidator struct {
 	isLimitadorOperatorInstalled bool
 	isGatewayAPIInstalled        bool
+	isGatewayProviderInstalled   bool
 }
 
 // RateLimitPolicyValidator subscribes to events with potential to flip the validity of rate limit policies
@@ -45,11 +46,15 @@ func (r *RateLimitPolicyValidator) Validate(ctx context.Context, _ []controller.
 
 	state.Store(StateRateLimitPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) {
 		if !r.isGatewayAPIInstalled {
-			return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API")
+			return policy.GetLocator(), kuadrant.MissingGatewayAPIError()
+		}
+
+		if !r.isGatewayProviderInstalled {
+			return policy.GetLocator(), kuadrant.MissingGatewayProviderError()
 		}
 
 		if !r.isLimitadorOperatorInstalled {
-			return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Limitador Operator")
+			return policy.GetLocator(), kuadrant.MissingLimitadorOperatorError()
 		}
 
 		var err error

controllers/tlspolicies_validator.go

@@ -52,11 +52,11 @@ func (t *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.Reso
 
 	state.Store(TLSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) {
 		if !t.isGatewayAPIInstalled {
-			return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API")
+			return p.GetLocator(), kuadrant.MissingGatewayAPIError()
 		}
 
 		if !t.isCertManagerInstalled {
-			return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Cert Manager")
+			return p.GetLocator(), kuadrant.MissingCertManagerError()
 		}
 
 		policy := p.(*kuadrantv1.TLSPolicy)

pkg/kuadrant/errors.go

@@ -258,3 +258,29 @@ func (e ErrSystemResource) Error() string {
 func (e ErrSystemResource) Reason() gatewayapiv1alpha2.PolicyConditionReason {
 	return PolicyReasonMissingResource
 }
+
+// Common ErrDependencyNotInstalled errors
+
+func MissingGatewayAPIError() PolicyError {
+	return NewErrDependencyNotInstalled("Gateway API")
+}
+
+func MissingGatewayProviderError() PolicyError {
+	return NewErrDependencyNotInstalled("Gateway API provider (istio / envoy gateway)")
+}
+
+func MissingAuthorinoOperatorError() PolicyError {
+	return NewErrDependencyNotInstalled("Authorino Operator")
+}
+
+func MissingLimitadorOperatorError() PolicyError {
+	return NewErrDependencyNotInstalled("Limitador Operator")
+}
+
+func MissingDNSOperatorError() PolicyError {
+	return NewErrDependencyNotInstalled("DNS Operator")
+}
+
+func MissingCertManagerError() PolicyError {
+	return NewErrDependencyNotInstalled("Cert Manager")
+}

tests/bare_k8s/kuadrant_controller_test.go

@@ -55,7 +55,7 @@ var _ = Describe("Kuadrant controller is disabled", func() {
 				cond := meta.FindStatusCondition(kuadrantCR.Status.Conditions, controllers.ReadyConditionType)
 				g.Expect(cond).ToNot(BeNil())
 				g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
-				g.Expect(cond.Reason).To(Equal("GatewayAPI"))
+				g.Expect(cond.Reason).To(Equal("MissingDependency"))
 				g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart pod once dependency has been installed"))
 			}).WithContext(ctx).Should(Succeed())
 		}, testTimeOut)

tests/gatewayapi/kuadrant_controller_test.go

@@ -10,7 +10,10 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
+	kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1"
 	kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
 	"github.com/kuadrant/kuadrant-operator/controllers"
 	"github.com/kuadrant/kuadrant-operator/tests"
@@ -19,8 +22,8 @@ import (
 var _ = Describe("Kuadrant controller when gateway provider is missing", func() {
 	var (
 		testNamespace    string
-		kuadrantName     string = "local"
-		afterEachTimeOut        = NodeTimeout(3 * time.Minute)
+		testTimeOut      = SpecTimeout(15 * time.Second)
+		afterEachTimeOut = NodeTimeout(3 * time.Minute)
 	)
 
 	BeforeEach(func(ctx SpecContext) {
@@ -32,29 +35,93 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func()
 	}, afterEachTimeOut)
 
 	Context("when default kuadrant CR is created", func() {
-		It("Status reports error", func(ctx SpecContext) {
+		It("Status reports missing Gateway API provider (istio / envoy gateway)", func(ctx SpecContext) {
 			kuadrantCR := &kuadrantv1beta1.Kuadrant{
 				TypeMeta: metav1.TypeMeta{
 					Kind:       "Kuadrant",
 					APIVersion: kuadrantv1beta1.GroupVersion.String(),
 				},
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      kuadrantName,
+					Name:      "local",
 					Namespace: testNamespace,
 				},
 			}
 			Expect(testClient().Create(ctx, kuadrantCR)).ToNot(HaveOccurred())
 
 			Eventually(func(g Gomega) {
-				kObj := &kuadrantv1beta1.Kuadrant{}
-				err := testClient().Get(ctx, client.ObjectKeyFromObject(kuadrantCR), kObj)
+				g.Expect(testClient().Get(ctx, client.ObjectKeyFromObject(kuadrantCR), kuadrantCR)).ToNot(HaveOccurred())
+				cond := meta.FindStatusCondition(kuadrantCR.Status.Conditions, controllers.ReadyConditionType)
+				g.Expect(cond).ToNot(BeNil())
+				g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+				g.Expect(cond.Reason).To(Equal("MissingDependency"))
+				g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart pod once dependency has been installed"))
+			}).WithContext(ctx).Should(Succeed())
+		}, testTimeOut)
+	})
+
+	Context("when rate limit policy is created", func() {
+		It("Status is populated with missing Gateway API provider (istio / envoy gateway)", func(ctx SpecContext) {
+			policy := &kuadrantv1.RateLimitPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "rlp",
+					Namespace: testNamespace,
+				},
+				Spec: kuadrantv1.RateLimitPolicySpec{
+					TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReferenceWithSectionName{
+						LocalPolicyTargetReference: gatewayapiv1alpha2.LocalPolicyTargetReference{
+							Kind:  "Gateway",
+							Group: gatewayapiv1.GroupName,
+							Name:  "test",
+						},
+					},
+				},
+			}
+
+			Expect(testClient().Create(ctx, policy)).To(Succeed())
+
+			Eventually(func(g Gomega) {
+				err := testClient().Get(ctx, client.ObjectKeyFromObject(policy), policy)
 				g.Expect(err).ToNot(HaveOccurred())
-				cond := meta.FindStatusCondition(kObj.Status.Conditions, string(controllers.ReadyConditionType))
+
+				cond := meta.FindStatusCondition(policy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
+				g.Expect(cond).ToNot(BeNil())
+				g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+				g.Expect(cond.Reason).To(Equal("MissingDependency"))
+				g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart pod once dependency has been installed"))
+			}).WithContext(ctx).Should(Succeed())
+		}, testTimeOut)
+	})
+
+	Context("when auth policy is created", func() {
+		It("Status is populated with missing Gateway API provider (istio / envoy gateway)", func(ctx SpecContext) {
+			policy := &kuadrantv1.AuthPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "auth",
+					Namespace: testNamespace,
+				},
+				Spec: kuadrantv1.AuthPolicySpec{
+					TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReferenceWithSectionName{
+						LocalPolicyTargetReference: gatewayapiv1alpha2.LocalPolicyTargetReference{
+							Kind:  "Gateway",
+							Group: gatewayapiv1.GroupName,
+							Name:  "test",
+						},
+					},
+				},
+			}
+
+			Expect(testClient().Create(ctx, policy)).To(Succeed())
+
+			Eventually(func(g Gomega) {
+				err := testClient().Get(ctx, client.ObjectKeyFromObject(policy), policy)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				cond := meta.FindStatusCondition(policy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
 				g.Expect(cond).ToNot(BeNil())
 				g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
-				g.Expect(cond.Reason).To(Equal("GatewayAPIProviderNotFound"))
-				g.Expect(cond.Message).To(Equal("GatewayAPI provider is not installed, please restart pod once dependency has been installed"))
-			}, time.Minute, 15*time.Second).WithContext(ctx).Should(Succeed())
-		})
+				g.Expect(cond.Reason).To(Equal("MissingDependency"))
+				g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart pod once dependency has been installed"))
+			}).WithContext(ctx).Should(Succeed())
+		}, testTimeOut)
 	})
 })