From bada5f6ba07fcc313aba52a133c4d8022e58009f Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 8 Nov 2024 13:30:59 +0000 Subject: [PATCH 01/12] feat: validate dns operator crd is installed for dnspolicy status Signed-off-by: KevFan --- controllers/dns_workflow.go | 4 ++-- controllers/dnspolicies_validator.go | 14 +++++++++--- controllers/state_of_the_world.go | 34 +++++++++++++++++----------- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/controllers/dns_workflow.go b/controllers/dns_workflow.go index 6ec58ff06..aea3242c0 100644 --- a/controllers/dns_workflow.go +++ b/controllers/dns_workflow.go @@ -46,9 +46,9 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords/status,verbs=get -func NewDNSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme) *controller.Workflow { +func NewDNSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isDNSOperatorInstalled bool) *controller.Workflow { return &controller.Workflow{ - Precondition: NewDNSPoliciesValidator().Subscription().Reconcile, + Precondition: NewDNSPoliciesValidator(isDNSOperatorInstalled).Subscription().Reconcile, Tasks: []controller.ReconcileFunc{ NewEffectiveDNSPoliciesReconciler(client, scheme).Subscription().Reconcile, }, diff --git a/controllers/dnspolicies_validator.go b/controllers/dnspolicies_validator.go index 776d98b4d..261a2ff56 100644 --- a/controllers/dnspolicies_validator.go +++ b/controllers/dnspolicies_validator.go @@ -18,11 +18,15 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) -func NewDNSPoliciesValidator() *DNSPoliciesValidator { - return &DNSPoliciesValidator{} +func NewDNSPoliciesValidator(isDNSOperatorInstalled bool) *DNSPoliciesValidator { + return &DNSPoliciesValidator{ + isDNSOperatorInstalled: isDNSOperatorInstalled, + } } -type DNSPoliciesValidator struct{} +type DNSPoliciesValidator struct { + isDNSOperatorInstalled bool +} func (r *DNSPoliciesValidator) Subscription() controller.Subscription { return controller.Subscription{ @@ -45,6 +49,10 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso logger.V(1).Info("validating dns policies", "policies", len(policies)) state.Store(StateDNSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { + if !r.isDNSOperatorInstalled { + return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("DNS Operator") + } + policy := p.(*kuadrantv1.DNSPolicy) if err := isTargetRefsFound(topology, policy); err != nil { diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index a12a0863f..5c7019e54 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -38,6 +38,7 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/openshift" "github.com/kuadrant/kuadrant-operator/pkg/openshift/consoleplugin" + "github.com/kuadrant/kuadrant-operator/pkg/utils" ) var ( @@ -177,6 +178,7 @@ type BootOptionsBuilder struct { isIstioInstalled bool isCertManagerInstalled bool isConsolePluginInstalled bool + isDNSOperatorInstalled bool } func (b *BootOptionsBuilder) getOptions() []controller.ControllerOption { @@ -323,18 +325,24 @@ func (b *BootOptionsBuilder) getConsolePluginOptions() []controller.ControllerOp func (b *BootOptionsBuilder) getDNSOperatorOptions() []controller.ControllerOption { var opts []controller.ControllerOption - opts = append(opts, - controller.WithRunnable("dnsrecord watcher", controller.Watch( - &kuadrantdnsv1alpha1.DNSRecord{}, DNSRecordResource, metav1.NamespaceAll, - controller.FilterResourcesByLabel[*kuadrantdnsv1alpha1.DNSRecord](fmt.Sprintf("%s=%s", AppLabelKey, AppLabelValue)))), - controller.WithObjectKinds( - DNSRecordGroupKind, - ), - controller.WithObjectLinks( - LinkListenerToDNSRecord, - LinkDNSPolicyToDNSRecord, - ), - ) + var err error + b.isDNSOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantdnsv1alpha1.GroupVersion.Group, "DNSRecord", kuadrantdnsv1alpha1.GroupVersion.Version) + if err != nil || !b.isDNSOperatorInstalled { + b.logger.Info("dns operator is not installed, skipping related watches and reconcilers", "err", err) + } else { + opts = append(opts, + controller.WithRunnable("dnsrecord watcher", controller.Watch( + &kuadrantdnsv1alpha1.DNSRecord{}, DNSRecordResource, metav1.NamespaceAll, + controller.FilterResourcesByLabel[*kuadrantdnsv1alpha1.DNSRecord](fmt.Sprintf("%s=%s", AppLabelKey, AppLabelValue)))), + controller.WithObjectKinds( + DNSRecordGroupKind, + ), + controller.WithObjectLinks( + LinkListenerToDNSRecord, + LinkDNSPolicyToDNSRecord, + ), + ) + } return opts } @@ -345,7 +353,7 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { Tasks: []controller.ReconcileFunc{ NewAuthorinoReconciler(b.client).Subscription().Reconcile, NewLimitadorReconciler(b.client).Subscription().Reconcile, - NewDNSWorkflow(b.client, b.manager.GetScheme()).Run, + NewDNSWorkflow(b.client, b.manager.GetScheme(), b.isDNSOperatorInstalled).Run, NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, NewDataPlanePoliciesWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Run, NewKuadrantStatusUpdater(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Subscription().Reconcile, From 1ad64d17b0d559a0e8c0266c9658acef9db151d1 Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 8 Nov 2024 13:41:45 +0000 Subject: [PATCH 02/12] feat: validate limitador operator crd is installed for rate limit policy status Signed-off-by: KevFan --- controllers/data_plane_policies_workflow.go | 4 ++-- controllers/ratelimit_policies_validator.go | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/controllers/data_plane_policies_workflow.go b/controllers/data_plane_policies_workflow.go index 53a0b0fd5..05029db55 100644 --- a/controllers/data_plane_policies_workflow.go +++ b/controllers/data_plane_policies_workflow.go @@ -59,11 +59,11 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update -func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled bool) *controller.Workflow { +func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled bool) *controller.Workflow { dataPlanePoliciesValidation := &controller.Workflow{ Tasks: []controller.ReconcileFunc{ (&AuthPolicyValidator{}).Subscription().Reconcile, - (&RateLimitPolicyValidator{}).Subscription().Reconcile, + (&RateLimitPolicyValidator{isLimitadorOperatorInstalled: isLimitadorOperatorInstalled}).Subscription().Reconcile, }, } diff --git a/controllers/ratelimit_policies_validator.go b/controllers/ratelimit_policies_validator.go index 4992950a0..b502013a5 100644 --- a/controllers/ratelimit_policies_validator.go +++ b/controllers/ratelimit_policies_validator.go @@ -15,7 +15,9 @@ import ( kuadrant "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) -type RateLimitPolicyValidator struct{} +type RateLimitPolicyValidator struct { + isLimitadorOperatorInstalled bool +} // RateLimitPolicyValidator subscribes to events with potential to flip the validity of rate limit policies func (r *RateLimitPolicyValidator) Subscription() controller.Subscription { @@ -41,6 +43,10 @@ func (r *RateLimitPolicyValidator) Validate(ctx context.Context, _ []controller. defer logger.V(1).Info("finished validating rate limit policies") state.Store(StateRateLimitPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { + if !r.isLimitadorOperatorInstalled { + return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Limitador Operator") + } + var err error if len(policy.GetTargetRefs()) > 0 && len(topology.Targetables().Children(policy)) == 0 { ref := policy.GetTargetRefs()[0] From 7cea6f0e0642dcf844d22ca3186a507bd224a0d8 Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 8 Nov 2024 13:55:52 +0000 Subject: [PATCH 03/12] feat: validate authorino operator crd is installed for auth policy status Signed-off-by: KevFan --- controllers/auth_policies_validator.go | 7 +- controllers/data_plane_policies_workflow.go | 4 +- controllers/state_of_the_world.go | 110 ++++++++++++++------ 3 files changed, 85 insertions(+), 36 deletions(-) diff --git a/controllers/auth_policies_validator.go b/controllers/auth_policies_validator.go index ac232735b..58e54cd9f 100644 --- a/controllers/auth_policies_validator.go +++ b/controllers/auth_policies_validator.go @@ -15,7 +15,9 @@ import ( kuadrant "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) -type AuthPolicyValidator struct{} +type AuthPolicyValidator struct { + isAuthorinoOperatorInstalled bool +} // AuthPolicyValidator subscribes to events with potential to flip the validity of auth policies func (r *AuthPolicyValidator) Subscription() controller.Subscription { @@ -41,6 +43,9 @@ func (r *AuthPolicyValidator) Validate(ctx context.Context, _ []controller.Resou defer logger.V(1).Info("finished validating auth policies") state.Store(StateAuthPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { + if !r.isAuthorinoOperatorInstalled { + return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Authorino Operator") + } var err error if len(policy.GetTargetRefs()) > 0 && len(topology.Targetables().Children(policy)) == 0 { ref := policy.GetTargetRefs()[0] diff --git a/controllers/data_plane_policies_workflow.go b/controllers/data_plane_policies_workflow.go index 05029db55..d0e730e5c 100644 --- a/controllers/data_plane_policies_workflow.go +++ b/controllers/data_plane_policies_workflow.go @@ -59,10 +59,10 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update -func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled bool) *controller.Workflow { +func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *controller.Workflow { dataPlanePoliciesValidation := &controller.Workflow{ Tasks: []controller.ReconcileFunc{ - (&AuthPolicyValidator{}).Subscription().Reconcile, + (&AuthPolicyValidator{isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled}).Subscription().Reconcile, (&RateLimitPolicyValidator{isLimitadorOperatorInstalled: isLimitadorOperatorInstalled}).Subscription().Reconcile, }, } diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 5c7019e54..c3fb746e2 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -111,23 +111,6 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D controller.WithPredicates(&ctrlruntimepredicate.TypedGenerationChangedPredicate[*corev1.ConfigMap]{}), controller.FilterResourcesByLabel[*corev1.ConfigMap](fmt.Sprintf("%s=true", kuadrant.TopologyLabel)), )), - // TODO: Move as boot options for Limitador and Authorino as there can be a possibility that the operators are not installed - controller.WithRunnable("limitador watcher", controller.Watch( - &limitadorv1alpha1.Limitador{}, - kuadrantv1beta1.LimitadorsResource, - metav1.NamespaceAll, - )), - controller.WithRunnable("authorino watcher", controller.Watch( - &authorinooperatorv1beta1.Authorino{}, - kuadrantv1beta1.AuthorinosResource, - metav1.NamespaceAll, - )), - controller.WithRunnable("authconfig watcher", controller.Watch( - &authorinov1beta3.AuthConfig{}, - authorino.AuthConfigsResource, - metav1.NamespaceAll, - controller.FilterResourcesByLabel[*authorinov1beta3.AuthConfig](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), - )), controller.WithPolicyKinds( kuadrantv1.DNSPolicyGroupKind, kuadrantv1.TLSPolicyGroupKind, @@ -137,15 +120,9 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D controller.WithObjectKinds( kuadrantv1beta1.KuadrantGroupKind, ConfigMapGroupKind, - kuadrantv1beta1.LimitadorGroupKind, - kuadrantv1beta1.AuthorinoGroupKind, - authorino.AuthConfigGroupKind, ), controller.WithObjectLinks( kuadrantv1beta1.LinkKuadrantToGatewayClasses, - kuadrantv1beta1.LinkKuadrantToLimitador, - kuadrantv1beta1.LinkKuadrantToAuthorino, - authorino.LinkHTTPRouteRuleToAuthConfig, ), } @@ -173,12 +150,14 @@ type BootOptionsBuilder struct { client *dynamic.DynamicClient // Internal configurations - isGatewayAPIInstalled bool - isEnvoyGatewayInstalled bool - isIstioInstalled bool - isCertManagerInstalled bool - isConsolePluginInstalled bool - isDNSOperatorInstalled bool + isGatewayAPIInstalled bool + isEnvoyGatewayInstalled bool + isIstioInstalled bool + isCertManagerInstalled bool + isConsolePluginInstalled bool + isDNSOperatorInstalled bool + isLimitadorOperatorInstalled bool + isAuthorinoOperatorInstalled bool } func (b *BootOptionsBuilder) getOptions() []controller.ControllerOption { @@ -189,6 +168,8 @@ func (b *BootOptionsBuilder) getOptions() []controller.ControllerOption { opts = append(opts, b.getCertManagerOptions()...) opts = append(opts, b.getConsolePluginOptions()...) opts = append(opts, b.getDNSOperatorOptions()...) + opts = append(opts, b.getLimitadorOperatorOptions()...) + opts = append(opts, b.getAuthorinoOperatorOptions()...) return opts } @@ -251,7 +232,6 @@ func (b *BootOptionsBuilder) getEnvoyGatewayOptions() []controller.ControllerOpt envoygateway.LinkGatewayToEnvoyExtensionPolicy, ), ) - // TODO: add specific tasks to workflow } return opts @@ -286,7 +266,6 @@ func (b *BootOptionsBuilder) getIstioOptions() []controller.ControllerOption { istio.LinkGatewayToWasmPlugin, ), ) - // TODO: add istio specific tasks to workflow } return opts @@ -326,7 +305,7 @@ func (b *BootOptionsBuilder) getConsolePluginOptions() []controller.ControllerOp func (b *BootOptionsBuilder) getDNSOperatorOptions() []controller.ControllerOption { var opts []controller.ControllerOption var err error - b.isDNSOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantdnsv1alpha1.GroupVersion.Group, "DNSRecord", kuadrantdnsv1alpha1.GroupVersion.Version) + b.isDNSOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), DNSRecordGroupKind.Group, DNSRecordGroupKind.Kind, kuadrantdnsv1alpha1.GroupVersion.Version) if err != nil || !b.isDNSOperatorInstalled { b.logger.Info("dns operator is not installed, skipping related watches and reconcilers", "err", err) } else { @@ -347,6 +326,71 @@ func (b *BootOptionsBuilder) getDNSOperatorOptions() []controller.ControllerOpti return opts } +func (b *BootOptionsBuilder) getLimitadorOperatorOptions() []controller.ControllerOption { + var opts []controller.ControllerOption + var err error + b.isLimitadorOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantv1beta1.LimitadorGroupKind.Group, kuadrantv1beta1.LimitadorGroupKind.Kind, limitadorv1alpha1.GroupVersion.Version) + if err != nil || !b.isLimitadorOperatorInstalled { + b.logger.Info("limitador operator is not installed, skipping related watches and reconcilers", "err", err) + } else { + opts = append(opts, + controller.WithRunnable("limitador watcher", controller.Watch( + &limitadorv1alpha1.Limitador{}, + kuadrantv1beta1.LimitadorsResource, + metav1.NamespaceAll, + )), + controller.WithObjectKinds( + kuadrantv1beta1.LimitadorGroupKind, + ), + controller.WithObjectLinks( + kuadrantv1beta1.LinkKuadrantToLimitador, + ), + ) + } + + return opts +} + +func (b *BootOptionsBuilder) getAuthorinoOperatorOptions() []controller.ControllerOption { + var opts []controller.ControllerOption + var err error + b.isAuthorinoOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantv1beta1.AuthorinoGroupKind.Group, kuadrantv1beta1.AuthorinoGroupKind.Kind, authorinooperatorv1beta1.GroupVersion.Version) + if err != nil || !b.isAuthorinoOperatorInstalled { + b.logger.Info("authorino operator is not installed, skipping related watches and reconcilers", "err", err) + return opts + } + + b.isAuthorinoOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), authorino.AuthConfigGroupKind.Group, authorino.AuthConfigGroupKind.Kind, authorinov1beta3.GroupVersion.Version) + if err != nil || !b.isAuthorinoOperatorInstalled { + b.logger.Info("authorino operator is not installed, skipping related watches and reconcilers", "err", err) + return opts + } + + opts = append(opts, + controller.WithRunnable("authorino watcher", controller.Watch( + &authorinooperatorv1beta1.Authorino{}, + kuadrantv1beta1.AuthorinosResource, + metav1.NamespaceAll, + )), + controller.WithRunnable("authconfig watcher", controller.Watch( + &authorinov1beta3.AuthConfig{}, + authorino.AuthConfigsResource, + metav1.NamespaceAll, + controller.FilterResourcesByLabel[*authorinov1beta3.AuthConfig](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), + )), + controller.WithObjectKinds( + kuadrantv1beta1.AuthorinoGroupKind, + authorino.AuthConfigGroupKind, + ), + controller.WithObjectLinks( + kuadrantv1beta1.LinkKuadrantToAuthorino, + authorino.LinkHTTPRouteRuleToAuthConfig, + ), + ) + + return opts +} + func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { mainWorkflow := &controller.Workflow{ Precondition: initWorkflow(b.client).Run, @@ -355,7 +399,7 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { NewLimitadorReconciler(b.client).Subscription().Reconcile, NewDNSWorkflow(b.client, b.manager.GetScheme(), b.isDNSOperatorInstalled).Run, NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, - NewDataPlanePoliciesWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Run, + NewDataPlanePoliciesWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Run, NewKuadrantStatusUpdater(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Subscription().Reconcile, }, Postcondition: finalStepsWorkflow(b.client, b.isIstioInstalled, b.isGatewayAPIInstalled).Run, From b8e66dc1c57432648fe7a03646a605ba4c7e38b0 Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 8 Nov 2024 16:34:15 +0000 Subject: [PATCH 04/12] feat: report missing Limitator/Authorino Operator to kuadrant status Signed-off-by: KevFan --- controllers/kuadrant_status_updater.go | 23 +++++++++++++++----- controllers/state_of_the_world.go | 15 ++++++++++--- tests/gatewayapi/kuadrant_controller_test.go | 2 +- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/controllers/kuadrant_status_updater.go b/controllers/kuadrant_status_updater.go index 4f96068b6..3a0803d8f 100644 --- a/controllers/kuadrant_status_updater.go +++ b/controllers/kuadrant_status_updater.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/go-logr/logr" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" corev1 "k8s.io/api/core/v1" @@ -18,6 +19,7 @@ import ( kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/authorino" + "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) const ( @@ -25,12 +27,14 @@ const ( ) type KuadrantStatusUpdater struct { - Client *dynamic.DynamicClient - HasGateway bool + Client *dynamic.DynamicClient + HasGateway bool + isLimitadorOperatorInstalled bool + isAuthorinoOperatorInstalled bool } -func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled bool) *KuadrantStatusUpdater { - return &KuadrantStatusUpdater{Client: client, HasGateway: isIstioInstalled || isEnvoyGatewayInstalled} +func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *KuadrantStatusUpdater { + return &KuadrantStatusUpdater{Client: client, HasGateway: isIstioInstalled || isEnvoyGatewayInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled} } func (r *KuadrantStatusUpdater) Subscription() *controller.Subscription { @@ -120,7 +124,14 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log if !r.HasGateway { cond.Status = metav1.ConditionFalse cond.Reason = "GatewayAPIProviderNotFound" - cond.Message = "GatewayAPI provider not found" + cond.Message = kuadrant.NewErrDependencyNotInstalled("GatewayAPI provider").Error() + return cond + } + + if !r.isLimitadorOperatorInstalled { + cond.Status = metav1.ConditionFalse + cond.Reason = "LimitadorAPIProviderNotFound" + cond.Message = kuadrant.NewErrDependencyNotInstalled("Limitador Operator").Error() return cond } @@ -166,7 +177,7 @@ func checkAuthorinoAvailable(topology *machinery.Topology, logger logr.Logger) * return ptr.To("authorino resoure not in topology") } - readyCondition := authorino.FindAuthorinoStatusCondition(authorinoObj.Status.Conditions, "Ready") + readyCondition := authorino.FindAuthorinoStatusCondition(authorinoObj.Status.Conditions, limitadorv1alpha1.StatusConditionReady) if readyCondition == nil { return ptr.To("Ready condition not found") } diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index c3fb746e2..33de3e998 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -395,12 +395,10 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { mainWorkflow := &controller.Workflow{ Precondition: initWorkflow(b.client).Run, Tasks: []controller.ReconcileFunc{ - NewAuthorinoReconciler(b.client).Subscription().Reconcile, - NewLimitadorReconciler(b.client).Subscription().Reconcile, NewDNSWorkflow(b.client, b.manager.GetScheme(), b.isDNSOperatorInstalled).Run, NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, NewDataPlanePoliciesWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Run, - NewKuadrantStatusUpdater(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Subscription().Reconcile, + NewKuadrantStatusUpdater(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Subscription().Reconcile, }, Postcondition: finalStepsWorkflow(b.client, b.isIstioInstalled, b.isGatewayAPIInstalled).Run, } @@ -411,6 +409,17 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { ) } + if b.isLimitadorOperatorInstalled { + mainWorkflow.Tasks = append(mainWorkflow.Tasks, + NewLimitadorReconciler(b.client).Subscription().Reconcile, + ) + } + + if b.isAuthorinoOperatorInstalled { + mainWorkflow.Tasks = append(mainWorkflow.Tasks, + NewAuthorinoReconciler(b.client).Subscription().Reconcile) + } + return mainWorkflow.Run } diff --git a/tests/gatewayapi/kuadrant_controller_test.go b/tests/gatewayapi/kuadrant_controller_test.go index fdf101b13..ec02ddf05 100644 --- a/tests/gatewayapi/kuadrant_controller_test.go +++ b/tests/gatewayapi/kuadrant_controller_test.go @@ -53,7 +53,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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 not found")) + 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()) }) }) From cce96305822a5332124e43772bc36e4a748dcb32 Mon Sep 17 00:00:00 2001 From: KevFan Date: Mon, 11 Nov 2024 11:57:09 +0000 Subject: [PATCH 05/12] feat: gateway api detection on kuadrant and policy status Signed-off-by: KevFan --- controllers/auth_policies_validator.go | 7 +- controllers/data_plane_policies_workflow.go | 6 +- controllers/dns_workflow.go | 4 +- controllers/dnspolicies_validator.go | 8 +- controllers/kuadrant_status_updater.go | 18 ++- controllers/ratelimit_policies_validator.go | 7 +- controllers/state_of_the_world.go | 24 ++-- controllers/tls_workflow.go | 4 +- controllers/tlspolicies_validator.go | 47 ++++---- tests/bare_k8s/kuadrant_controller_test.go | 119 +++++++++++++++++++- 10 files changed, 194 insertions(+), 50 deletions(-) diff --git a/controllers/auth_policies_validator.go b/controllers/auth_policies_validator.go index 58e54cd9f..2de5372dc 100644 --- a/controllers/auth_policies_validator.go +++ b/controllers/auth_policies_validator.go @@ -12,10 +12,11 @@ import ( "k8s.io/utils/ptr" kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1" - kuadrant "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) type AuthPolicyValidator struct { + isGatewayAPIInstalled bool isAuthorinoOperatorInstalled bool } @@ -43,6 +44,10 @@ func (r *AuthPolicyValidator) Validate(ctx context.Context, _ []controller.Resou defer logger.V(1).Info("finished validating auth policies") state.Store(StateAuthPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { + if !r.isGatewayAPIInstalled { + return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API") + } + if !r.isAuthorinoOperatorInstalled { return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Authorino Operator") } diff --git a/controllers/data_plane_policies_workflow.go b/controllers/data_plane_policies_workflow.go index d0e730e5c..f3bacb615 100644 --- a/controllers/data_plane_policies_workflow.go +++ b/controllers/data_plane_policies_workflow.go @@ -59,11 +59,11 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update -func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *controller.Workflow { +func NewDataPlanePoliciesWorkflow(client *dynamic.DynamicClient, isGatewayAPInstalled, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *controller.Workflow { dataPlanePoliciesValidation := &controller.Workflow{ Tasks: []controller.ReconcileFunc{ - (&AuthPolicyValidator{isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled}).Subscription().Reconcile, - (&RateLimitPolicyValidator{isLimitadorOperatorInstalled: isLimitadorOperatorInstalled}).Subscription().Reconcile, + (&AuthPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled}).Subscription().Reconcile, + (&RateLimitPolicyValidator{isGatewayAPIInstalled: isGatewayAPInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled}).Subscription().Reconcile, }, } diff --git a/controllers/dns_workflow.go b/controllers/dns_workflow.go index aea3242c0..91e40d468 100644 --- a/controllers/dns_workflow.go +++ b/controllers/dns_workflow.go @@ -46,9 +46,9 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords/status,verbs=get -func NewDNSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isDNSOperatorInstalled bool) *controller.Workflow { +func NewDNSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isGatewayAPIInstalled, isDNSOperatorInstalled bool) *controller.Workflow { return &controller.Workflow{ - Precondition: NewDNSPoliciesValidator(isDNSOperatorInstalled).Subscription().Reconcile, + Precondition: NewDNSPoliciesValidator(isGatewayAPIInstalled, isDNSOperatorInstalled).Subscription().Reconcile, Tasks: []controller.ReconcileFunc{ NewEffectiveDNSPoliciesReconciler(client, scheme).Subscription().Reconcile, }, diff --git a/controllers/dnspolicies_validator.go b/controllers/dnspolicies_validator.go index 261a2ff56..20321f0de 100644 --- a/controllers/dnspolicies_validator.go +++ b/controllers/dnspolicies_validator.go @@ -18,13 +18,15 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) -func NewDNSPoliciesValidator(isDNSOperatorInstalled bool) *DNSPoliciesValidator { +func NewDNSPoliciesValidator(isGatewayAPIInstalled, isDNSOperatorInstalled bool) *DNSPoliciesValidator { return &DNSPoliciesValidator{ + isGatewayAPIInstalled: isGatewayAPIInstalled, isDNSOperatorInstalled: isDNSOperatorInstalled, } } type DNSPoliciesValidator struct { + isGatewayAPIInstalled bool isDNSOperatorInstalled bool } @@ -49,6 +51,10 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso logger.V(1).Info("validating dns policies", "policies", len(policies)) state.Store(StateDNSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { + if !r.isGatewayAPIInstalled { + return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API") + } + if !r.isDNSOperatorInstalled { return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("DNS Operator") } diff --git a/controllers/kuadrant_status_updater.go b/controllers/kuadrant_status_updater.go index 3a0803d8f..cc1ebac3c 100644 --- a/controllers/kuadrant_status_updater.go +++ b/controllers/kuadrant_status_updater.go @@ -28,13 +28,20 @@ const ( type KuadrantStatusUpdater struct { Client *dynamic.DynamicClient + isGatwayAPIInstalled bool HasGateway bool isLimitadorOperatorInstalled bool isAuthorinoOperatorInstalled bool } -func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *KuadrantStatusUpdater { - return &KuadrantStatusUpdater{Client: client, HasGateway: isIstioInstalled || isEnvoyGatewayInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled} +func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isGatewayAPIInstalled, isGatewayProviderInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *KuadrantStatusUpdater { + return &KuadrantStatusUpdater{ + Client: client, + isGatwayAPIInstalled: isGatewayAPIInstalled, + HasGateway: isGatewayProviderInstalled, + isLimitadorOperatorInstalled: isLimitadorOperatorInstalled, + isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled, + } } func (r *KuadrantStatusUpdater) Subscription() *controller.Subscription { @@ -121,6 +128,13 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log Message: "Kuadrant is ready", } + if !r.isGatwayAPIInstalled { + cond.Status = metav1.ConditionFalse + cond.Reason = "GatewayAPI" + cond.Message = kuadrant.NewErrDependencyNotInstalled("Gateway API").Error() + return cond + } + if !r.HasGateway { cond.Status = metav1.ConditionFalse cond.Reason = "GatewayAPIProviderNotFound" diff --git a/controllers/ratelimit_policies_validator.go b/controllers/ratelimit_policies_validator.go index b502013a5..f648b649e 100644 --- a/controllers/ratelimit_policies_validator.go +++ b/controllers/ratelimit_policies_validator.go @@ -12,11 +12,12 @@ import ( "k8s.io/utils/ptr" kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1" - kuadrant "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) type RateLimitPolicyValidator struct { isLimitadorOperatorInstalled bool + isGatewayAPIInstalled bool } // RateLimitPolicyValidator subscribes to events with potential to flip the validity of rate limit policies @@ -43,6 +44,10 @@ func (r *RateLimitPolicyValidator) Validate(ctx context.Context, _ []controller. defer logger.V(1).Info("finished validating rate limit policies") state.Store(StateRateLimitPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { + if !r.isGatewayAPIInstalled { + return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API") + } + if !r.isLimitadorOperatorInstalled { return policy.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Limitador Operator") } diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 33de3e998..1bd0a7664 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -391,16 +391,20 @@ func (b *BootOptionsBuilder) getAuthorinoOperatorOptions() []controller.Controll return opts } +func (b *BootOptionsBuilder) isGatewayProviderInstalled() bool { + return b.isIstioInstalled || b.isEnvoyGatewayInstalled +} + func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { mainWorkflow := &controller.Workflow{ Precondition: initWorkflow(b.client).Run, Tasks: []controller.ReconcileFunc{ - NewDNSWorkflow(b.client, b.manager.GetScheme(), b.isDNSOperatorInstalled).Run, - NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, - NewDataPlanePoliciesWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Run, - NewKuadrantStatusUpdater(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Subscription().Reconcile, + NewDNSWorkflow(b.client, b.manager.GetScheme(), b.isGatewayAPIInstalled, b.isDNSOperatorInstalled).Run, + NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isGatewayAPIInstalled, b.isCertManagerInstalled).Run, + NewDataPlanePoliciesWorkflow(b.client, b.isGatewayAPIInstalled, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Run, + NewKuadrantStatusUpdater(b.client, b.isGatewayAPIInstalled, b.isGatewayProviderInstalled(), b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Subscription().Reconcile, }, - Postcondition: finalStepsWorkflow(b.client, b.isIstioInstalled, b.isGatewayAPIInstalled).Run, + Postcondition: finalStepsWorkflow(b.client, b.isGatewayAPIInstalled, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Run, } if b.isConsolePluginInstalled { @@ -477,12 +481,16 @@ func initWorkflow(client *dynamic.DynamicClient) *controller.Workflow { } } -func finalStepsWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoyGatewayInstalled bool) *controller.Workflow { +func finalStepsWorkflow(client *dynamic.DynamicClient, isGatewayAPIInstalled, isIstioInstalled, isEnvoyGatewayInstalled bool) *controller.Workflow { workflow := &controller.Workflow{ - Tasks: []controller.ReconcileFunc{ + Tasks: []controller.ReconcileFunc{}, + } + + if isGatewayAPIInstalled { + workflow.Tasks = append(workflow.Tasks, NewGatewayPolicyDiscoverabilityReconciler(client).Subscription().Reconcile, NewHTTPRoutePolicyDiscoverabilityReconciler(client).Subscription().Reconcile, - }, + ) } if isIstioInstalled { diff --git a/controllers/tls_workflow.go b/controllers/tls_workflow.go index 7665e77c7..bb64993ba 100644 --- a/controllers/tls_workflow.go +++ b/controllers/tls_workflow.go @@ -40,9 +40,9 @@ var ( //+kubebuilder:rbac:groups="cert-manager.io",resources=clusterissuers,verbs=get;list;watch; //+kubebuilder:rbac:groups="cert-manager.io",resources=certificates,verbs=get;list;watch;create;update;patch;delete -func NewTLSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isCertManagerInstalled bool) *controller.Workflow { +func NewTLSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isGatewayAPIInstalled, isCertManagerInstalled bool) *controller.Workflow { return &controller.Workflow{ - Precondition: NewTLSPoliciesValidator(isCertManagerInstalled).Subscription().Reconcile, + Precondition: NewTLSPoliciesValidator(isGatewayAPIInstalled, isCertManagerInstalled).Subscription().Reconcile, Tasks: []controller.ReconcileFunc{ NewEffectiveTLSPoliciesReconciler(client, scheme).Subscription().Reconcile, }, diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go index 61e0df16c..630e86d2c 100644 --- a/controllers/tlspolicies_validator.go +++ b/controllers/tlspolicies_validator.go @@ -19,13 +19,15 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/kuadrant" ) -func NewTLSPoliciesValidator(isCertManagerInstalled bool) *TLSPoliciesValidator { +func NewTLSPoliciesValidator(isGatewayAPIInstalled, isCertManagerInstalled bool) *TLSPoliciesValidator { return &TLSPoliciesValidator{ + isGatewayAPIInstalled: isGatewayAPIInstalled, isCertManagerInstalled: isCertManagerInstalled, } } type TLSPoliciesValidator struct { + isGatewayAPIInstalled bool isCertManagerInstalled bool } @@ -42,53 +44,46 @@ func (t *TLSPoliciesValidator) Subscription() *controller.Subscription { } } -func (t *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { +func (t *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { logger := controller.LoggerFromContext(ctx).WithName("TLSPoliciesValidator").WithName("Validate") policies := lo.Filter(topology.Policies().Items(), filterForTLSPolicies) + logger.V(1).Info("validating tls policies", "policies", len(policies)) - isPolicyValidErrorMap := make(map[string]error, len(policies)) - - for _, policy := range policies { - p := policy.(*kuadrantv1.TLSPolicy) - if p.DeletionTimestamp != nil { - logger.V(1).Info("tls policy is marked for deletion, skipping", "name", p.Name, "namespace", p.Namespace) - continue + state.Store(TLSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { + if !t.isGatewayAPIInstalled { + return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Gateway API") } if !t.isCertManagerInstalled { - isPolicyValidErrorMap[p.GetLocator()] = kuadrant.NewErrDependencyNotInstalled("Cert Manager") - continue + return p.GetLocator(), kuadrant.NewErrDependencyNotInstalled("Cert Manager") } + policy := p.(*kuadrantv1.TLSPolicy) // Validate target ref - if err := t.isTargetRefsFound(topology, p); err != nil { - isPolicyValidErrorMap[p.GetLocator()] = err - continue + if err := t.isTargetRefsFound(topology, policy); err != nil { + return p.GetLocator(), err } // Validate if there's a conflicting policy - if err := t.isConflict(policies, p); err != nil { - isPolicyValidErrorMap[p.GetLocator()] = err - continue + if err := t.isConflict(policies, policy); err != nil { + return p.GetLocator(), err } // Validate IssuerRef kind is correct - if err := t.isValidIssuerKind(p); err != nil { - isPolicyValidErrorMap[p.GetLocator()] = err - continue + if err := t.isValidIssuerKind(policy); err != nil { + return p.GetLocator(), err } // Validate Issuer is present on cluster through the topology - if err := t.isIssuerFound(topology, p); err != nil { - isPolicyValidErrorMap[p.GetLocator()] = err - continue + if err := t.isIssuerFound(topology, policy); err != nil { + return p.GetLocator(), err } - isPolicyValidErrorMap[p.GetLocator()] = nil - } + return p.GetLocator(), nil + })) - s.Store(TLSPolicyAcceptedKey, isPolicyValidErrorMap) + logger.V(1).Info("finished validating tls policies") return nil } diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go index b9cf6461c..164195352 100644 --- a/tests/bare_k8s/kuadrant_controller_test.go +++ b/tests/bare_k8s/kuadrant_controller_test.go @@ -5,18 +5,22 @@ package bare_k8s_test import ( "time" + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "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" ) -var _ = Describe("Controller", func() { +var _ = Describe("Kuadrant controller is disabled", func() { var ( testNamespace string testTimeOut = SpecTimeout(15 * time.Second) @@ -32,7 +36,7 @@ var _ = Describe("Controller", func() { }, afterEachTimeOut) Context("when default kuadrant CR is created", func() { - It("Status is populated with missing GatewayProvide", func(ctx SpecContext) { + It("Status is populated with missing Gateway API", func(ctx SpecContext) { kuadrantCR := &kuadrantv1beta1.Kuadrant{ TypeMeta: metav1.TypeMeta{ Kind: "Kuadrant", @@ -51,8 +55,115 @@ var _ = Describe("Controller", 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("GatewayAPIProviderNotFound")) - g.Expect(cond.Message).To(Equal("GatewayAPI provider not found")) + g.Expect(cond.Reason).To(Equal("GatewayAPI")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart pod once dependency has been installed")) + }).WithContext(ctx).Should(Succeed()) + }, testTimeOut) + }) + + Context("when dns policy is created", func() { + It("Status is populated with missing Gateway API", func(ctx SpecContext) { + policy := kuadrantv1.NewDNSPolicy("dns", testNamespace).WithTargetGateway("test") + policy.Spec.ProviderRefs = append(policy.Spec.ProviderRefs, kuadrantdnsv1alpha1.ProviderRef{ + Name: "dnsProviderSecret", + }) + 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("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) + }) + + Context("when tls policy is created", func() { + It("Status is populated with missing Gateway API", func(ctx SpecContext) { + policy := kuadrantv1.NewTLSPolicy("tls", testNamespace). + WithTargetGateway("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("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) + }) + + Context("when rate limit policy is created", func() { + It("Status is populated with missing Gateway API", 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(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 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", 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("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) }) From 5a544e9e595e770f62e5ea4b8dac05b5fcdcf6f5 Mon Sep 17 00:00:00 2001 From: KevFan Date: Mon, 11 Nov 2024 15:26:27 +0000 Subject: [PATCH 06/12] refactor: common missing dependency errors Signed-off-by: KevFan --- controllers/auth_policies_validator.go | 10 ++- controllers/data_plane_policies_workflow.go | 5 +- controllers/dnspolicies_validator.go | 4 +- controllers/kuadrant_status_updater.go | 33 +++++--- controllers/ratelimit_policies_validator.go | 9 +- controllers/tlspolicies_validator.go | 4 +- pkg/kuadrant/errors.go | 26 ++++++ tests/bare_k8s/kuadrant_controller_test.go | 2 +- tests/gatewayapi/kuadrant_controller_test.go | 89 +++++++++++++++++--- 9 files changed, 149 insertions(+), 33 deletions(-) diff --git a/controllers/auth_policies_validator.go b/controllers/auth_policies_validator.go index 2de5372dc..8f66dec2b 100644 --- a/controllers/auth_policies_validator.go +++ b/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] diff --git a/controllers/data_plane_policies_workflow.go b/controllers/data_plane_policies_workflow.go index f3bacb615..ed166a8f8 100644 --- a/controllers/data_plane_policies_workflow.go +++ b/controllers/data_plane_policies_workflow.go @@ -60,10 +60,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, }, } diff --git a/controllers/dnspolicies_validator.go b/controllers/dnspolicies_validator.go index 20321f0de..a84dccdbb 100644 --- a/controllers/dnspolicies_validator.go +++ b/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) diff --git a/controllers/kuadrant_status_updater.go b/controllers/kuadrant_status_updater.go index cc1ebac3c..7fae5b3e4 100644 --- a/controllers/kuadrant_status_updater.go +++ b/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") } diff --git a/controllers/ratelimit_policies_validator.go b/controllers/ratelimit_policies_validator.go index f648b649e..9657c9b12 100644 --- a/controllers/ratelimit_policies_validator.go +++ b/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 diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go index 630e86d2c..3432c584f 100644 --- a/controllers/tlspolicies_validator.go +++ b/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) diff --git a/pkg/kuadrant/errors.go b/pkg/kuadrant/errors.go index 3c3229101..7c28a4249 100644 --- a/pkg/kuadrant/errors.go +++ b/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") +} diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go index 164195352..e36d854ba 100644 --- a/tests/bare_k8s/kuadrant_controller_test.go +++ b/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) diff --git a/tests/gatewayapi/kuadrant_controller_test.go b/tests/gatewayapi/kuadrant_controller_test.go index ec02ddf05..c30ca7fa1 100644 --- a/tests/gatewayapi/kuadrant_controller_test.go +++ b/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) }) }) From 495da298aa50c598a5dbbe2a5c06c329bde85654 Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 15 Nov 2024 09:55:41 +0000 Subject: [PATCH 07/12] refactor: IsAuthorinoOperatorInstalled function Signed-off-by: KevFan --- controllers/state_of_the_world.go | 10 +--------- pkg/authorino/conditions.go | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 1bd0a7664..a8c660852 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -354,13 +354,7 @@ func (b *BootOptionsBuilder) getLimitadorOperatorOptions() []controller.Controll func (b *BootOptionsBuilder) getAuthorinoOperatorOptions() []controller.ControllerOption { var opts []controller.ControllerOption var err error - b.isAuthorinoOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantv1beta1.AuthorinoGroupKind.Group, kuadrantv1beta1.AuthorinoGroupKind.Kind, authorinooperatorv1beta1.GroupVersion.Version) - if err != nil || !b.isAuthorinoOperatorInstalled { - b.logger.Info("authorino operator is not installed, skipping related watches and reconcilers", "err", err) - return opts - } - - b.isAuthorinoOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), authorino.AuthConfigGroupKind.Group, authorino.AuthConfigGroupKind.Kind, authorinov1beta3.GroupVersion.Version) + b.isAuthorinoOperatorInstalled, err = authorino.IsAuthorinoOperatorInstalled(b.manager.GetRESTMapper(), b.logger) if err != nil || !b.isAuthorinoOperatorInstalled { b.logger.Info("authorino operator is not installed, skipping related watches and reconcilers", "err", err) return opts @@ -504,8 +498,6 @@ func finalStepsWorkflow(client *dynamic.DynamicClient, isGatewayAPIInstalled, is return workflow } -var ErrMissingKuadrant = fmt.Errorf("missing kuadrant object in topology") - func GetKuadrantFromTopology(topology *machinery.Topology) *kuadrantv1beta1.Kuadrant { kuadrants := lo.FilterMap(topology.Objects().Roots(), func(root machinery.Object, _ int) (controller.Object, bool) { o, isSortable := root.(controller.Object) diff --git a/pkg/authorino/conditions.go b/pkg/authorino/conditions.go index 49010b476..fd8a2ca5c 100644 --- a/pkg/authorino/conditions.go +++ b/pkg/authorino/conditions.go @@ -1,15 +1,36 @@ package authorino import ( - authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + "github.com/go-logr/logr" + authorinooperatorv1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + authorinov1beta3 "github.com/kuadrant/authorino/api/v1beta3" + "k8s.io/apimachinery/pkg/api/meta" + + "github.com/kuadrant/kuadrant-operator/pkg/utils" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" ) -func FindAuthorinoStatusCondition(conditions []authorinov1beta1.Condition, conditionType string) *authorinov1beta1.Condition { +func FindAuthorinoStatusCondition(conditions []authorinooperatorv1beta1.Condition, conditionType string) *authorinooperatorv1beta1.Condition { for i := range conditions { - if conditions[i].Type == authorinov1beta1.ConditionType(conditionType) { + if conditions[i].Type == authorinooperatorv1beta1.ConditionType(conditionType) { return &conditions[i] } } return nil } + +func IsAuthorinoOperatorInstalled(restMapper meta.RESTMapper, logger logr.Logger) (bool, error) { + if ok, err := utils.IsCRDInstalled(restMapper, kuadrantv1beta1.AuthorinoGroupKind.Group, kuadrantv1beta1.AuthorinoGroupKind.Kind, authorinooperatorv1beta1.GroupVersion.Version); !ok || err != nil { + logger.V(1).Error(err, "Authorino Operator CRD was not installed", "group", kuadrantv1beta1.AuthorinoGroupKind.Group, "kind", kuadrantv1beta1.AuthorinoGroupKind.Kind, "version", authorinooperatorv1beta1.GroupVersion.Version) + return false, err + } + + if ok, err := utils.IsCRDInstalled(restMapper, AuthConfigGroupKind.Group, AuthConfigGroupKind.Kind, authorinov1beta3.GroupVersion.Version); !ok || err != nil { + logger.V(1).Error(err, "Authorino Operator CRD was not installed", "group", AuthConfigGroupKind.Group, "kind", AuthConfigGroupKind.Kind, "version", authorinov1beta3.GroupVersion.Version) + return false, err + } + + return true, nil +} From c5302249562be4478466912272f1227f31b4c79c Mon Sep 17 00:00:00 2001 From: KevFan Date: Mon, 2 Dec 2024 11:29:10 +0000 Subject: [PATCH 08/12] fix: integration tests after rebase Signed-off-by: KevFan --- tests/bare_k8s/kuadrant_controller_test.go | 28 +++++++++++++++++++- tests/gatewayapi/kuadrant_controller_test.go | 26 ++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go index e36d854ba..7d42b9c53 100644 --- a/tests/bare_k8s/kuadrant_controller_test.go +++ b/tests/bare_k8s/kuadrant_controller_test.go @@ -5,6 +5,7 @@ package bare_k8s_test import ( "time" + authorinoapi "github.com/kuadrant/authorino/api/v1beta3" kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -20,7 +21,7 @@ import ( "github.com/kuadrant/kuadrant-operator/tests" ) -var _ = Describe("Kuadrant controller is disabled", func() { +var _ = Describe("Kuadrant controller when Gateway API is missing", func() { var ( testNamespace string testTimeOut = SpecTimeout(15 * time.Second) @@ -117,6 +118,18 @@ var _ = Describe("Kuadrant controller is disabled", func() { Name: "test", }, }, + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: map[string]kuadrantv1.Limit{ + "test": { + Rates: []kuadrantv1.Rate{ + { + Limit: 10, + Window: "10s", + }, + }, + }, + }, + }, }, } @@ -150,6 +163,19 @@ var _ = Describe("Kuadrant controller is disabled", func() { Name: "test", }, }, + AuthPolicySpecProper: kuadrantv1.AuthPolicySpecProper{ + AuthScheme: &kuadrantv1.AuthSchemeSpec{ + Authentication: map[string]kuadrantv1.MergeableAuthenticationSpec{ + "anyonmous": { + AuthenticationSpec: authorinoapi.AuthenticationSpec{ + AuthenticationMethodSpec: authorinoapi.AuthenticationMethodSpec{ + AnonymousAccess: &authorinoapi.AnonymousAccessSpec{}, + }, + }, + }, + }, + }, + }, }, } diff --git a/tests/gatewayapi/kuadrant_controller_test.go b/tests/gatewayapi/kuadrant_controller_test.go index c30ca7fa1..38e8ce94d 100644 --- a/tests/gatewayapi/kuadrant_controller_test.go +++ b/tests/gatewayapi/kuadrant_controller_test.go @@ -5,6 +5,7 @@ package gatewayapi_test import ( "time" + authorinoapi "github.com/kuadrant/authorino/api/v1beta3" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -74,6 +75,18 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() Name: "test", }, }, + RateLimitPolicySpecProper: kuadrantv1.RateLimitPolicySpecProper{ + Limits: map[string]kuadrantv1.Limit{ + "test": { + Rates: []kuadrantv1.Rate{ + { + Limit: 10, + Window: "10s", + }, + }, + }, + }, + }, }, } @@ -107,6 +120,19 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() Name: "test", }, }, + AuthPolicySpecProper: kuadrantv1.AuthPolicySpecProper{ + AuthScheme: &kuadrantv1.AuthSchemeSpec{ + Authentication: map[string]kuadrantv1.MergeableAuthenticationSpec{ + "anyonmous": { + AuthenticationSpec: authorinoapi.AuthenticationSpec{ + AuthenticationMethodSpec: authorinoapi.AuthenticationMethodSpec{ + AnonymousAccess: &authorinoapi.AnonymousAccessSpec{}, + }, + }, + }, + }, + }, + }, }, } From e1f802c1fd2f7a8708f01a74c543a64ba5d5e6f4 Mon Sep 17 00:00:00 2001 From: KevFan Date: Mon, 13 Jan 2025 15:12:39 +0000 Subject: [PATCH 09/12] fixup: specify kuadrant operator pod in status message Signed-off-by: KevFan --- pkg/kuadrant/errors.go | 2 +- tests/bare_k8s/kuadrant_controller_test.go | 10 +++++----- tests/gatewayapi/kuadrant_controller_test.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/kuadrant/errors.go b/pkg/kuadrant/errors.go index 7c28a4249..4aed50a3b 100644 --- a/pkg/kuadrant/errors.go +++ b/pkg/kuadrant/errors.go @@ -232,7 +232,7 @@ type ErrDependencyNotInstalled struct { } func (e ErrDependencyNotInstalled) Error() string { - return fmt.Sprintf("%s is not installed, please restart pod once dependency has been installed", e.dependencyName) + return fmt.Sprintf("%s is not installed, please restart Kuadrant Operator pod once dependency has been installed", e.dependencyName) } func (e ErrDependencyNotInstalled) Reason() gatewayapiv1alpha2.PolicyConditionReason { diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go index 7d42b9c53..7e9220d6a 100644 --- a/tests/bare_k8s/kuadrant_controller_test.go +++ b/tests/bare_k8s/kuadrant_controller_test.go @@ -57,7 +57,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -78,7 +78,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -98,7 +98,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -143,7 +143,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -189,7 +189,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("Gateway API is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) diff --git a/tests/gatewayapi/kuadrant_controller_test.go b/tests/gatewayapi/kuadrant_controller_test.go index 38e8ce94d..fd9b32b8a 100644 --- a/tests/gatewayapi/kuadrant_controller_test.go +++ b/tests/gatewayapi/kuadrant_controller_test.go @@ -55,7 +55,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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")) + g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -100,7 +100,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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")) + g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -146,7 +146,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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")) + g.Expect(cond.Message).To(Equal("Gateway API provider (istio / envoy gateway) is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) From b446956fa7c55593caffe32ed7f90a63da7866e7 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 14 Jan 2025 16:46:01 +0000 Subject: [PATCH 10/12] refactor: list missing dependencies in status message Signed-off-by: KevFan --- controllers/auth_policies_validator.go | 36 ++++++++---- controllers/dnspolicies_validator.go | 28 ++++++++-- controllers/kuadrant_status_updater.go | 59 ++++++++++---------- controllers/ratelimit_policies_validator.go | 36 ++++++++---- controllers/tlspolicies_validator.go | 50 +++++++++++------ pkg/kuadrant/errors.go | 33 ++--------- tests/bare_k8s/kuadrant_controller_test.go | 10 ++-- tests/gatewayapi/kuadrant_controller_test.go | 6 +- 8 files changed, 150 insertions(+), 108 deletions(-) diff --git a/controllers/auth_policies_validator.go b/controllers/auth_policies_validator.go index 8f66dec2b..85af8c064 100644 --- a/controllers/auth_policies_validator.go +++ b/controllers/auth_policies_validator.go @@ -45,16 +45,8 @@ func (r *AuthPolicyValidator) Validate(ctx context.Context, _ []controller.Resou defer logger.V(1).Info("finished validating auth policies") state.Store(StateAuthPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { - if !r.isGatewayAPIInstalled { - return policy.GetLocator(), kuadrant.MissingGatewayAPIError() - } - - if !r.isGatewayProviderInstalled { - return policy.GetLocator(), kuadrant.MissingGatewayProviderError() - } - - if !r.isAuthorinoOperatorInstalled { - return policy.GetLocator(), kuadrant.MissingAuthorinoOperatorError() + if err := r.isMissingDependency(); err != nil { + return policy.GetLocator(), err } var err error @@ -74,3 +66,27 @@ func (r *AuthPolicyValidator) Validate(ctx context.Context, _ []controller.Resou return nil } + +func (r *AuthPolicyValidator) isMissingDependency() error { + isMissingDependency := false + var missingDependencies []string + + if !r.isGatewayAPIInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API") + } + if !r.isGatewayProviderInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API provider (istio / envoy gateway)") + } + if !r.isAuthorinoOperatorInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Authorino Operator") + } + + if isMissingDependency { + return kuadrant.NewErrDependencyNotInstalled(missingDependencies...) + } + + return nil +} diff --git a/controllers/dnspolicies_validator.go b/controllers/dnspolicies_validator.go index a84dccdbb..399760955 100644 --- a/controllers/dnspolicies_validator.go +++ b/controllers/dnspolicies_validator.go @@ -51,12 +51,8 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso logger.V(1).Info("validating dns policies", "policies", len(policies)) state.Store(StateDNSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { - if !r.isGatewayAPIInstalled { - return p.GetLocator(), kuadrant.MissingGatewayAPIError() - } - - if !r.isDNSOperatorInstalled { - return p.GetLocator(), kuadrant.MissingDNSOperatorError() + if err := r.isMissingDependency(); err != nil { + return p.GetLocator(), err } policy := p.(*kuadrantv1.DNSPolicy) @@ -77,6 +73,26 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso return nil } +func (r *DNSPoliciesValidator) isMissingDependency() error { + isMissingDependency := false + var missingDependencies []string + + if !r.isGatewayAPIInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API") + } + if !r.isDNSOperatorInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "DNS Operator") + } + + if isMissingDependency { + return kuadrant.NewErrDependencyNotInstalled(missingDependencies...) + } + + return nil +} + // isTargetRefsFound Policies are already linked to their targets. // If the target ref length and length of targetables by this policy is not the same, then the policy could not find the target. func isTargetRefsFound(topology *machinery.Topology, p *kuadrantv1.DNSPolicy) error { diff --git a/controllers/kuadrant_status_updater.go b/controllers/kuadrant_status_updater.go index 7fae5b3e4..fa6985e15 100644 --- a/controllers/kuadrant_status_updater.go +++ b/controllers/kuadrant_status_updater.go @@ -28,7 +28,7 @@ const ( type KuadrantStatusUpdater struct { Client *dynamic.DynamicClient - isGatwayAPIInstalled bool + isGatewayAPIInstalled bool isGatewayProviderInstalled bool isLimitadorOperatorInstalled bool isAuthorinoOperatorInstalled bool @@ -37,7 +37,7 @@ type KuadrantStatusUpdater struct { func NewKuadrantStatusUpdater(client *dynamic.DynamicClient, isGatewayAPIInstalled, isGatewayProviderInstalled, isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled bool) *KuadrantStatusUpdater { return &KuadrantStatusUpdater{ Client: client, - isGatwayAPIInstalled: isGatewayAPIInstalled, + isGatewayAPIInstalled: isGatewayAPIInstalled, isGatewayProviderInstalled: isGatewayProviderInstalled, isLimitadorOperatorInstalled: isLimitadorOperatorInstalled, isAuthorinoOperatorInstalled: isAuthorinoOperatorInstalled, @@ -128,32 +128,7 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log Message: "Kuadrant is ready", } - if !r.isGatwayAPIInstalled { - err := kuadrant.MissingGatewayAPIError() - cond.Status = metav1.ConditionFalse - cond.Reason = string(err.Reason()) - cond.Message = err.Error() - return cond - } - - if !r.isGatewayProviderInstalled { - err := kuadrant.MissingGatewayProviderError() - cond.Status = metav1.ConditionFalse - cond.Reason = string(err.Reason()) - cond.Message = err.Error() - return cond - } - - if !r.isLimitadorOperatorInstalled { - err := kuadrant.MissingLimitadorOperatorError() - cond.Status = metav1.ConditionFalse - cond.Reason = string(err.Reason()) - cond.Message = err.Error() - return cond - } - - if !r.isAuthorinoOperatorInstalled { - err := kuadrant.MissingAuthorinoOperatorError() + if err := r.isMissingDependency(); err != nil { cond.Status = metav1.ConditionFalse cond.Reason = string(err.Reason()) cond.Message = err.Error() @@ -177,6 +152,34 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log return cond } +func (r *KuadrantStatusUpdater) isMissingDependency() kuadrant.PolicyError { + isMissingDependency := false + var missingDependencies []string + + if !r.isGatewayAPIInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API") + } + if !r.isGatewayProviderInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API provider (istio / envoy gateway)") + } + if !r.isAuthorinoOperatorInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Authorino Operator") + } + if !r.isLimitadorOperatorInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Limitador Operator") + } + + if isMissingDependency { + return kuadrant.NewErrDependencyNotInstalled(missingDependencies...) + } + + return nil +} + func checkLimitadorReady(topology *machinery.Topology, logger logr.Logger) *string { limitadorObj := GetLimitadorFromTopology(topology) if limitadorObj == nil { diff --git a/controllers/ratelimit_policies_validator.go b/controllers/ratelimit_policies_validator.go index 9657c9b12..3b8403b97 100644 --- a/controllers/ratelimit_policies_validator.go +++ b/controllers/ratelimit_policies_validator.go @@ -45,16 +45,8 @@ func (r *RateLimitPolicyValidator) Validate(ctx context.Context, _ []controller. defer logger.V(1).Info("finished validating rate limit policies") state.Store(StateRateLimitPolicyValid, lo.SliceToMap(policies, func(policy machinery.Policy) (string, error) { - if !r.isGatewayAPIInstalled { - return policy.GetLocator(), kuadrant.MissingGatewayAPIError() - } - - if !r.isGatewayProviderInstalled { - return policy.GetLocator(), kuadrant.MissingGatewayProviderError() - } - - if !r.isLimitadorOperatorInstalled { - return policy.GetLocator(), kuadrant.MissingLimitadorOperatorError() + if err := r.isMissingDependency(); err != nil { + return policy.GetLocator(), err } var err error @@ -74,3 +66,27 @@ func (r *RateLimitPolicyValidator) Validate(ctx context.Context, _ []controller. return nil } + +func (r *RateLimitPolicyValidator) isMissingDependency() error { + isMissingDependency := false + var missingDependencies []string + + if !r.isGatewayAPIInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API") + } + if !r.isGatewayProviderInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API provider (istio / envoy gateway)") + } + if !r.isLimitadorOperatorInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Limitador Operator") + } + + if isMissingDependency { + return kuadrant.NewErrDependencyNotInstalled(missingDependencies...) + } + + return nil +} diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go index 3432c584f..633178c55 100644 --- a/controllers/tlspolicies_validator.go +++ b/controllers/tlspolicies_validator.go @@ -31,7 +31,7 @@ type TLSPoliciesValidator struct { isCertManagerInstalled bool } -func (t *TLSPoliciesValidator) Subscription() *controller.Subscription { +func (r *TLSPoliciesValidator) Subscription() *controller.Subscription { return &controller.Subscription{ Events: []controller.ResourceEventMatcher{ {Kind: &machinery.GatewayGroupKind}, @@ -40,43 +40,39 @@ func (t *TLSPoliciesValidator) Subscription() *controller.Subscription { {Kind: &CertManagerIssuerKind}, {Kind: &CertManagerClusterIssuerKind}, }, - ReconcileFunc: t.Validate, + ReconcileFunc: r.Validate, } } -func (t *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { +func (r *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { logger := controller.LoggerFromContext(ctx).WithName("TLSPoliciesValidator").WithName("Validate") policies := lo.Filter(topology.Policies().Items(), filterForTLSPolicies) logger.V(1).Info("validating tls policies", "policies", len(policies)) state.Store(TLSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { - if !t.isGatewayAPIInstalled { - return p.GetLocator(), kuadrant.MissingGatewayAPIError() - } - - if !t.isCertManagerInstalled { - return p.GetLocator(), kuadrant.MissingCertManagerError() + if err := r.isMissingDependency(); err != nil { + return p.GetLocator(), err } policy := p.(*kuadrantv1.TLSPolicy) // Validate target ref - if err := t.isTargetRefsFound(topology, policy); err != nil { + if err := r.isTargetRefsFound(topology, policy); err != nil { return p.GetLocator(), err } // Validate if there's a conflicting policy - if err := t.isConflict(policies, policy); err != nil { + if err := r.isConflict(policies, policy); err != nil { return p.GetLocator(), err } // Validate IssuerRef kind is correct - if err := t.isValidIssuerKind(policy); err != nil { + if err := r.isValidIssuerKind(policy); err != nil { return p.GetLocator(), err } // Validate Issuer is present on cluster through the topology - if err := t.isIssuerFound(topology, policy); err != nil { + if err := r.isIssuerFound(topology, policy); err != nil { return p.GetLocator(), err } @@ -88,10 +84,30 @@ func (t *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.Reso return nil } +func (r *TLSPoliciesValidator) isMissingDependency() error { + isMissingDependency := false + var missingDependencies []string + + if !r.isGatewayAPIInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Gateway API") + } + if !r.isCertManagerInstalled { + isMissingDependency = true + missingDependencies = append(missingDependencies, "Cert Manager") + } + + if isMissingDependency { + return kuadrant.NewErrDependencyNotInstalled(missingDependencies...) + } + + return nil +} + // isTargetRefsFound Policies are already linked to their targets. If the target ref length and length of targetables by this policy is not the same, // then the policy could not find the target // TODO: What should happen if multiple target refs is supported in the future in terms of reporting in log and policy status? -func (t *TLSPoliciesValidator) isTargetRefsFound(topology *machinery.Topology, p *kuadrantv1.TLSPolicy) error { +func (r *TLSPoliciesValidator) isTargetRefsFound(topology *machinery.Topology, p *kuadrantv1.TLSPolicy) error { if len(p.GetTargetRefs()) != len(topology.Targetables().Children(p)) { return kuadrant.NewErrTargetNotFound(kuadrantv1.TLSPolicyGroupKind.Kind, p.Spec.TargetRef.LocalPolicyTargetReference, apierrors.NewNotFound(controller.GatewaysResource.GroupResource(), p.GetName())) } @@ -100,7 +116,7 @@ func (t *TLSPoliciesValidator) isTargetRefsFound(topology *machinery.Topology, p } // isConflict Validates if there's already an older policy with the same target ref -func (t *TLSPoliciesValidator) isConflict(policies []machinery.Policy, p *kuadrantv1.TLSPolicy) error { +func (r *TLSPoliciesValidator) isConflict(policies []machinery.Policy, p *kuadrantv1.TLSPolicy) error { conflictingP, ok := lo.Find(policies, func(item machinery.Policy) bool { conflictTLSPolicy := item.(*kuadrantv1.TLSPolicy) return p != conflictTLSPolicy && conflictTLSPolicy.DeletionTimestamp == nil && @@ -116,7 +132,7 @@ func (t *TLSPoliciesValidator) isConflict(policies []machinery.Policy, p *kuadra } // isValidIssuerKind Validates that the Issuer Ref kind is either empty, Issuer or ClusterIssuer -func (t *TLSPoliciesValidator) isValidIssuerKind(p *kuadrantv1.TLSPolicy) error { +func (r *TLSPoliciesValidator) isValidIssuerKind(p *kuadrantv1.TLSPolicy) error { if !lo.Contains([]string{"", certmanv1.IssuerKind, certmanv1.ClusterIssuerKind}, p.Spec.IssuerRef.Kind) { return kuadrant.NewErrInvalid(kuadrantv1.TLSPolicyGroupKind.Kind, fmt.Errorf(`invalid value %q for issuerRef.kind. Must be empty, %q or %q`, p.Spec.IssuerRef.Kind, certmanv1.IssuerKind, certmanv1.ClusterIssuerKind)) @@ -126,7 +142,7 @@ func (t *TLSPoliciesValidator) isValidIssuerKind(p *kuadrantv1.TLSPolicy) error } // isIssuerFound Validates that the Issuer specified can be found in the topology -func (t *TLSPoliciesValidator) isIssuerFound(topology *machinery.Topology, p *kuadrantv1.TLSPolicy) error { +func (r *TLSPoliciesValidator) isIssuerFound(topology *machinery.Topology, p *kuadrantv1.TLSPolicy) error { _, ok := lo.Find(topology.Objects().Children(p), func(item machinery.Object) bool { runtimeObj, ok := item.(*controller.RuntimeObject) if !ok { diff --git a/pkg/kuadrant/errors.go b/pkg/kuadrant/errors.go index 4aed50a3b..4547bbc44 100644 --- a/pkg/kuadrant/errors.go +++ b/pkg/kuadrant/errors.go @@ -3,6 +3,7 @@ package kuadrant import ( "errors" "fmt" + "strings" "github.com/kuadrant/policy-machinery/machinery" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -219,7 +220,7 @@ func reasonForError(err error) gatewayapiv1alpha2.PolicyConditionReason { return "" } -func NewErrDependencyNotInstalled(dependencyName string) ErrDependencyNotInstalled { +func NewErrDependencyNotInstalled(dependencyName ...string) ErrDependencyNotInstalled { return ErrDependencyNotInstalled{ dependencyName: dependencyName, } @@ -228,11 +229,11 @@ func NewErrDependencyNotInstalled(dependencyName string) ErrDependencyNotInstall var _ PolicyError = ErrDependencyNotInstalled{} type ErrDependencyNotInstalled struct { - dependencyName string + dependencyName []string } func (e ErrDependencyNotInstalled) Error() string { - return fmt.Sprintf("%s is not installed, please restart Kuadrant Operator pod once dependency has been installed", e.dependencyName) + return fmt.Sprintf("[%s] is not installed, please restart Kuadrant Operator pod once dependency has been installed", strings.Join(e.dependencyName, ", ")) } func (e ErrDependencyNotInstalled) Reason() gatewayapiv1alpha2.PolicyConditionReason { @@ -258,29 +259,3 @@ 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") -} diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go index 7e9220d6a..1bab992bc 100644 --- a/tests/bare_k8s/kuadrant_controller_test.go +++ b/tests/bare_k8s/kuadrant_controller_test.go @@ -57,7 +57,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API, Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -78,7 +78,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -98,7 +98,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -143,7 +143,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API, Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -189,7 +189,7 @@ var _ = Describe("Kuadrant controller when Gateway API is missing", func() { 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 is not installed, please restart Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API, Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) diff --git a/tests/gatewayapi/kuadrant_controller_test.go b/tests/gatewayapi/kuadrant_controller_test.go index fd9b32b8a..5c9140f13 100644 --- a/tests/gatewayapi/kuadrant_controller_test.go +++ b/tests/gatewayapi/kuadrant_controller_test.go @@ -55,7 +55,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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 Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -100,7 +100,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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 Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) @@ -146,7 +146,7 @@ var _ = Describe("Kuadrant controller when gateway provider is missing", func() 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 Kuadrant Operator pod once dependency has been installed")) + g.Expect(cond.Message).To(Equal("[Gateway API provider (istio / envoy gateway)] is not installed, please restart Kuadrant Operator pod once dependency has been installed")) }).WithContext(ctx).Should(Succeed()) }, testTimeOut) }) From efcaf7e6e1b7511408739e4102e7a5a0221f54a1 Mon Sep 17 00:00:00 2001 From: KevFan Date: Wed, 15 Jan 2025 16:52:53 +0000 Subject: [PATCH 11/12] refactor: remove else conditioning for returning opts Signed-off-by: KevFan --- controllers/state_of_the_world.go | 207 +++++++++++++++--------------- 1 file changed, 107 insertions(+), 100 deletions(-) diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index a8c660852..ea149a15c 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -180,26 +180,27 @@ func (b *BootOptionsBuilder) getGatewayAPIOptions() []controller.ControllerOptio b.isGatewayAPIInstalled, err = kuadrantgatewayapi.IsGatewayAPIInstalled(b.manager.GetRESTMapper()) if err != nil || !b.isGatewayAPIInstalled { b.logger.Info("gateway api is not installed, skipping watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("gatewayclass watcher", controller.Watch( - &gwapiv1.GatewayClass{}, - controller.GatewayClassesResource, - metav1.NamespaceAll, - )), - controller.WithRunnable("gateway watcher", controller.Watch( - &gwapiv1.Gateway{}, - controller.GatewaysResource, - metav1.NamespaceAll, - )), - controller.WithRunnable("httproute watcher", controller.Watch( - &gwapiv1.HTTPRoute{}, - controller.HTTPRoutesResource, - metav1.NamespaceAll, - )), - ) + return opts } + opts = append(opts, + controller.WithRunnable("gatewayclass watcher", controller.Watch( + &gwapiv1.GatewayClass{}, + controller.GatewayClassesResource, + metav1.NamespaceAll, + )), + controller.WithRunnable("gateway watcher", controller.Watch( + &gwapiv1.Gateway{}, + controller.GatewaysResource, + metav1.NamespaceAll, + )), + controller.WithRunnable("httproute watcher", controller.Watch( + &gwapiv1.HTTPRoute{}, + controller.HTTPRoutesResource, + metav1.NamespaceAll, + )), + ) + return opts } @@ -209,31 +210,32 @@ func (b *BootOptionsBuilder) getEnvoyGatewayOptions() []controller.ControllerOpt b.isEnvoyGatewayInstalled, err = envoygateway.IsEnvoyGatewayInstalled(b.manager.GetRESTMapper()) if err != nil || !b.isEnvoyGatewayInstalled { b.logger.Info("envoygateway is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("envoypatchpolicy watcher", controller.Watch( - &egv1alpha1.EnvoyPatchPolicy{}, - envoygateway.EnvoyPatchPoliciesResource, - metav1.NamespaceAll, - controller.FilterResourcesByLabel[*egv1alpha1.EnvoyPatchPolicy](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), - )), - controller.WithRunnable("envoyextensionpolicy watcher", controller.Watch( - &egv1alpha1.EnvoyExtensionPolicy{}, - envoygateway.EnvoyExtensionPoliciesResource, - metav1.NamespaceAll, - controller.FilterResourcesByLabel[*egv1alpha1.EnvoyExtensionPolicy](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), - )), - controller.WithObjectKinds( - envoygateway.EnvoyPatchPolicyGroupKind, - envoygateway.EnvoyExtensionPolicyGroupKind, - ), - controller.WithObjectLinks( - envoygateway.LinkGatewayToEnvoyPatchPolicy, - envoygateway.LinkGatewayToEnvoyExtensionPolicy, - ), - ) + return opts } + opts = append(opts, + controller.WithRunnable("envoypatchpolicy watcher", controller.Watch( + &egv1alpha1.EnvoyPatchPolicy{}, + envoygateway.EnvoyPatchPoliciesResource, + metav1.NamespaceAll, + controller.FilterResourcesByLabel[*egv1alpha1.EnvoyPatchPolicy](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), + )), + controller.WithRunnable("envoyextensionpolicy watcher", controller.Watch( + &egv1alpha1.EnvoyExtensionPolicy{}, + envoygateway.EnvoyExtensionPoliciesResource, + metav1.NamespaceAll, + controller.FilterResourcesByLabel[*egv1alpha1.EnvoyExtensionPolicy](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), + )), + controller.WithObjectKinds( + envoygateway.EnvoyPatchPolicyGroupKind, + envoygateway.EnvoyExtensionPolicyGroupKind, + ), + controller.WithObjectLinks( + envoygateway.LinkGatewayToEnvoyPatchPolicy, + envoygateway.LinkGatewayToEnvoyExtensionPolicy, + ), + ) + return opts } @@ -243,31 +245,32 @@ func (b *BootOptionsBuilder) getIstioOptions() []controller.ControllerOption { b.isIstioInstalled, err = istio.IsIstioInstalled(b.manager.GetRESTMapper()) if err != nil || !b.isIstioInstalled { b.logger.Info("istio is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("envoyfilter watcher", controller.Watch( - &istioclientnetworkingv1alpha3.EnvoyFilter{}, - istio.EnvoyFiltersResource, - metav1.NamespaceAll, - controller.FilterResourcesByLabel[*istioclientnetworkingv1alpha3.EnvoyFilter](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), - )), - controller.WithRunnable("wasmplugin watcher", controller.Watch( - &istioclientgoextensionv1alpha1.WasmPlugin{}, - istio.WasmPluginsResource, - metav1.NamespaceAll, - controller.FilterResourcesByLabel[*istioclientgoextensionv1alpha1.WasmPlugin](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), - )), - controller.WithObjectKinds( - istio.EnvoyFilterGroupKind, - istio.WasmPluginGroupKind, - ), - controller.WithObjectLinks( - istio.LinkGatewayToEnvoyFilter, - istio.LinkGatewayToWasmPlugin, - ), - ) + return opts } + opts = append(opts, + controller.WithRunnable("envoyfilter watcher", controller.Watch( + &istioclientnetworkingv1alpha3.EnvoyFilter{}, + istio.EnvoyFiltersResource, + metav1.NamespaceAll, + controller.FilterResourcesByLabel[*istioclientnetworkingv1alpha3.EnvoyFilter](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), + )), + controller.WithRunnable("wasmplugin watcher", controller.Watch( + &istioclientgoextensionv1alpha1.WasmPlugin{}, + istio.WasmPluginsResource, + metav1.NamespaceAll, + controller.FilterResourcesByLabel[*istioclientgoextensionv1alpha1.WasmPlugin](fmt.Sprintf("%s=true", kuadrantManagedLabelKey)), + )), + controller.WithObjectKinds( + istio.EnvoyFilterGroupKind, + istio.WasmPluginGroupKind, + ), + controller.WithObjectLinks( + istio.LinkGatewayToEnvoyFilter, + istio.LinkGatewayToWasmPlugin, + ), + ) + return opts } @@ -277,10 +280,11 @@ func (b *BootOptionsBuilder) getCertManagerOptions() []controller.ControllerOpti b.isCertManagerInstalled, err = kuadrantgatewayapi.IsCertManagerInstalled(b.manager.GetRESTMapper(), b.logger) if err != nil || !b.isCertManagerInstalled { b.logger.Info("cert manager is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, certManagerControllerOpts()...) + return opts } + opts = append(opts, certManagerControllerOpts()...) + return opts } @@ -290,15 +294,16 @@ func (b *BootOptionsBuilder) getConsolePluginOptions() []controller.ControllerOp b.isConsolePluginInstalled, err = openshift.IsConsolePluginInstalled(b.manager.GetRESTMapper()) if err != nil || !b.isConsolePluginInstalled { b.logger.Info("console plugin is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("consoleplugin watcher", controller.Watch( - &consolev1.ConsolePlugin{}, openshift.ConsolePluginsResource, metav1.NamespaceAll, - controller.FilterResourcesByLabel[*consolev1.ConsolePlugin](fmt.Sprintf("%s=%s", consoleplugin.AppLabelKey, consoleplugin.AppLabelValue)))), - controller.WithObjectKinds(openshift.ConsolePluginGVK.GroupKind()), - ) + return opts } + opts = append(opts, + controller.WithRunnable("consoleplugin watcher", controller.Watch( + &consolev1.ConsolePlugin{}, openshift.ConsolePluginsResource, metav1.NamespaceAll, + controller.FilterResourcesByLabel[*consolev1.ConsolePlugin](fmt.Sprintf("%s=%s", consoleplugin.AppLabelKey, consoleplugin.AppLabelValue)))), + controller.WithObjectKinds(openshift.ConsolePluginGVK.GroupKind()), + ) + return opts } @@ -308,21 +313,22 @@ func (b *BootOptionsBuilder) getDNSOperatorOptions() []controller.ControllerOpti b.isDNSOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), DNSRecordGroupKind.Group, DNSRecordGroupKind.Kind, kuadrantdnsv1alpha1.GroupVersion.Version) if err != nil || !b.isDNSOperatorInstalled { b.logger.Info("dns operator is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("dnsrecord watcher", controller.Watch( - &kuadrantdnsv1alpha1.DNSRecord{}, DNSRecordResource, metav1.NamespaceAll, - controller.FilterResourcesByLabel[*kuadrantdnsv1alpha1.DNSRecord](fmt.Sprintf("%s=%s", AppLabelKey, AppLabelValue)))), - controller.WithObjectKinds( - DNSRecordGroupKind, - ), - controller.WithObjectLinks( - LinkListenerToDNSRecord, - LinkDNSPolicyToDNSRecord, - ), - ) + return opts } + opts = append(opts, + controller.WithRunnable("dnsrecord watcher", controller.Watch( + &kuadrantdnsv1alpha1.DNSRecord{}, DNSRecordResource, metav1.NamespaceAll, + controller.FilterResourcesByLabel[*kuadrantdnsv1alpha1.DNSRecord](fmt.Sprintf("%s=%s", AppLabelKey, AppLabelValue)))), + controller.WithObjectKinds( + DNSRecordGroupKind, + ), + controller.WithObjectLinks( + LinkListenerToDNSRecord, + LinkDNSPolicyToDNSRecord, + ), + ) + return opts } @@ -332,22 +338,23 @@ func (b *BootOptionsBuilder) getLimitadorOperatorOptions() []controller.Controll b.isLimitadorOperatorInstalled, err = utils.IsCRDInstalled(b.manager.GetRESTMapper(), kuadrantv1beta1.LimitadorGroupKind.Group, kuadrantv1beta1.LimitadorGroupKind.Kind, limitadorv1alpha1.GroupVersion.Version) if err != nil || !b.isLimitadorOperatorInstalled { b.logger.Info("limitador operator is not installed, skipping related watches and reconcilers", "err", err) - } else { - opts = append(opts, - controller.WithRunnable("limitador watcher", controller.Watch( - &limitadorv1alpha1.Limitador{}, - kuadrantv1beta1.LimitadorsResource, - metav1.NamespaceAll, - )), - controller.WithObjectKinds( - kuadrantv1beta1.LimitadorGroupKind, - ), - controller.WithObjectLinks( - kuadrantv1beta1.LinkKuadrantToLimitador, - ), - ) + return opts } + opts = append(opts, + controller.WithRunnable("limitador watcher", controller.Watch( + &limitadorv1alpha1.Limitador{}, + kuadrantv1beta1.LimitadorsResource, + metav1.NamespaceAll, + )), + controller.WithObjectKinds( + kuadrantv1beta1.LimitadorGroupKind, + ), + controller.WithObjectLinks( + kuadrantv1beta1.LinkKuadrantToLimitador, + ), + ) + return opts } From e32bc81a954326595816189cb2def366691548fd Mon Sep 17 00:00:00 2001 From: KevFan Date: Wed, 15 Jan 2025 17:06:54 +0000 Subject: [PATCH 12/12] refactor: use defer for log Signed-off-by: KevFan --- controllers/tlspolicies_validator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go index 633178c55..694e7367e 100644 --- a/controllers/tlspolicies_validator.go +++ b/controllers/tlspolicies_validator.go @@ -49,6 +49,7 @@ func (r *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.Reso policies := lo.Filter(topology.Policies().Items(), filterForTLSPolicies) logger.V(1).Info("validating tls policies", "policies", len(policies)) + defer logger.V(1).Info("finished validating tls policies") state.Store(TLSPolicyAcceptedKey, lo.SliceToMap(policies, func(p machinery.Policy) (string, error) { if err := r.isMissingDependency(); err != nil { @@ -79,8 +80,6 @@ func (r *TLSPoliciesValidator) Validate(ctx context.Context, _ []controller.Reso return p.GetLocator(), nil })) - logger.V(1).Info("finished validating tls policies") - return nil }