From 5e2d5297cab410247ba673a22aa85521b8d50590 Mon Sep 17 00:00:00 2001 From: KevFan Date: Thu, 3 Oct 2024 16:42:12 +0100 Subject: [PATCH] refactor: align with some common conventions Signed-off-by: KevFan --- controllers/cert_manager_installed.go | 37 +++++ controllers/state_of_the_world.go | 5 +- .../{tlspolicy_links.go => tls_workflow.go} | 20 +++ controllers/tlspolicies_validator.go | 96 ++++++++++++ ...y_tasks.go => tlspolicy_status_updater.go} | 148 ++---------------- ...st.go => tlspolicy_status_updater_test.go} | 2 +- 6 files changed, 167 insertions(+), 141 deletions(-) create mode 100644 controllers/cert_manager_installed.go rename controllers/{tlspolicy_links.go => tls_workflow.go} (80%) create mode 100644 controllers/tlspolicies_validator.go rename controllers/{tlspolicy_tasks.go => tlspolicy_status_updater.go} (57%) rename controllers/{tlspolicy_tasks_test.go => tlspolicy_status_updater_test.go} (99%) diff --git a/controllers/cert_manager_installed.go b/controllers/cert_manager_installed.go new file mode 100644 index 000000000..87f030b57 --- /dev/null +++ b/controllers/cert_manager_installed.go @@ -0,0 +1,37 @@ +package controllers + +import ( + "context" + "sync" + + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + "k8s.io/apimachinery/pkg/api/meta" + + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" +) + +const IsCertManagerInstalledKey = "IsCertManagerInstalled" + +func NewIsCertManagerInstalledReconciler(restMapper meta.RESTMapper) IsCertManagerInstalledReconciler { + return IsCertManagerInstalledReconciler{ + restMapper: restMapper, + } +} + +type IsCertManagerInstalledReconciler struct { + restMapper meta.RESTMapper +} + +func (t IsCertManagerInstalledReconciler) Check(ctx context.Context, _ []controller.ResourceEvent, _ *machinery.Topology, _ error, s *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("IsCertManagerInstalledReconciler").WithName("Reconcile") + isCertManagerInstalled, err := kuadrantgatewayapi.IsCertManagerInstalled(t.restMapper, logger) + + if err != nil { + logger.Error(err, "error checking IsCertManagerInstalled") + } + + s.Store(IsCertManagerInstalledKey, isCertManagerInstalled) + + return nil +} diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 40a1050a7..d982196cf 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -104,6 +104,7 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D kuadrantv1beta1.LinkKuadrantToGatewayClasses, kuadrantv1beta1.LinkKuadrantToLimitador, ), + controller.WithReconcile(buildReconciler(client, manager.GetRESTMapper())), } ok, err := kuadrantgatewayapi.IsGatewayAPIInstalled(manager.GetRESTMapper()) @@ -203,8 +204,6 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D controllerOpts = append(controllerOpts, certManagerControllerOpts()...) } - controllerOpts = append(controllerOpts, controller.WithReconcile(buildReconciler(client, manager.GetRESTMapper()))) - return controller.NewController(controllerOpts...) } @@ -214,7 +213,7 @@ func buildReconciler(client *dynamic.DynamicClient, restMapper meta.RESTMapper) Precondition: NewEventLogger().Log, Tasks: []controller.ReconcileFunc{ NewTopologyFileReconciler(client, operatorNamespace).Reconcile, - NewIsCertManagerInstalledTask(restMapper).Reconcile, + NewIsCertManagerInstalledReconciler(restMapper).Check, }, }).Run, Tasks: []controller.ReconcileFunc{ diff --git a/controllers/tlspolicy_links.go b/controllers/tls_workflow.go similarity index 80% rename from controllers/tlspolicy_links.go rename to controllers/tls_workflow.go index 08389be07..4df60ae44 100644 --- a/controllers/tlspolicy_links.go +++ b/controllers/tls_workflow.go @@ -1,15 +1,35 @@ package controllers import ( + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" "github.com/samber/lo" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1" ) +var ( + CertManagerCertificatesResource = certmanagerv1.SchemeGroupVersion.WithResource("certificates") + CertManagerIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("issuers") + CertMangerClusterIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("clusterissuers") + + CertManagerCertificateKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.CertificateKind} + CertManagerIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.IssuerKind} + CertManagerClusterIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.ClusterIssuerKind} +) + +func NewTLSPolicyWorkflow(client *dynamic.DynamicClient) *controller.Workflow { + return &controller.Workflow{ + Precondition: NewValidateTLSPoliciesValidatorReconciler().Validate, + Postcondition: NewTLSPolicyStatusUpdaterReconciler(client).UpdateStatus, + } +} + func LinkGatewayToCertificateFunc(objs controller.Store) machinery.LinkFunc { gateways := lo.Map(objs.FilterByGroupKind(machinery.GatewayGroupKind), controller.ObjectAs[*gwapiv1.Gateway]) diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go new file mode 100644 index 000000000..f085cd7e5 --- /dev/null +++ b/controllers/tlspolicies_validator.go @@ -0,0 +1,96 @@ +package controllers + +import ( + "context" + "errors" + "sync" + + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + "github.com/samber/lo" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/ptr" + + kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" +) + +func NewValidateTLSPoliciesValidatorReconciler() *ValidateTLSPoliciesValidatorReconciler { + return &ValidateTLSPoliciesValidatorReconciler{} +} + +type ValidateTLSPoliciesValidatorReconciler struct{} + +func (t *ValidateTLSPoliciesValidatorReconciler) Subscription() *controller.Subscription { + return &controller.Subscription{ + Events: []controller.ResourceEventMatcher{ + {Kind: &machinery.GatewayGroupKind}, + {Kind: &kuadrantv1alpha1.TLSPolicyKind, EventType: ptr.To(controller.CreateEvent)}, + {Kind: &kuadrantv1alpha1.TLSPolicyKind, EventType: ptr.To(controller.UpdateEvent)}, + {Kind: &CertManagerCertificateKind}, + {Kind: &CertManagerIssuerKind}, + {Kind: &CertManagerClusterIssuerKind}, + }, + ReconcileFunc: t.Validate, + } +} + +func (t *ValidateTLSPoliciesValidatorReconciler) Validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("ValidateTLSPolicyTask").WithName("Reconcile") + + // Get all TLS Policies + policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.TLSPolicy, bool) { + p, ok := item.(*kuadrantv1alpha1.TLSPolicy) + return p, ok + }) + + // Get all gateways + gws := lo.FilterMap(topology.Targetables().Items(), func(item machinery.Targetable, index int) (*machinery.Gateway, bool) { + gw, ok := item.(*machinery.Gateway) + return gw, ok + }) + + isCertManagerInstalled := false + installed, ok := s.Load(IsCertManagerInstalledKey) + if ok { + isCertManagerInstalled = installed.(bool) + } else { + logger.V(1).Error(errors.New("isCertManagerInstalled was not found in sync map, defaulting to false"), "sync map error") + } + + for _, policy := range policies { + if policy.DeletionTimestamp != nil { + logger.V(1).Info("tls policy is marked for deletion, skipping", "name", policy.Name, "namespace", policy.Namespace) + continue + } + + if !isCertManagerInstalled { + s.Store(TLSPolicyAcceptedKey(policy.GetUID()), kuadrant.NewErrDependencyNotInstalled("Cert Manager")) + continue + } + + // TODO: This should be only one target ref for now, but what should happen if multiple target refs is supported in the future? + targetRefs := policy.GetTargetRefs() + for _, targetRef := range targetRefs { + // Find gateway defined by target ref + _, ok := lo.Find(gws, func(item *machinery.Gateway) bool { + if item.GetName() == targetRef.GetName() && item.GetNamespace() == targetRef.GetNamespace() { + return true + } + return false + }) + + // Can't find gateway target ref + if !ok { + logger.V(1).Info("tls policy cannot find target ref", "name", policy.Name, "namespace", policy.Namespace) + s.Store(TLSPolicyAcceptedKey(policy.GetUID()), kuadrant.NewErrTargetNotFound(policy.Kind(), policy.GetTargetRef(), apierrors.NewNotFound(kuadrantv1alpha1.TLSPoliciesResource.GroupResource(), policy.GetName()))) + continue + } + + logger.V(1).Info("tls policy found target ref", "name", policy.Name, "namespace", policy.Namespace) + s.Store(TLSPolicyAcceptedKey(policy.GetUID()), nil) + } + } + + return nil +} diff --git a/controllers/tlspolicy_tasks.go b/controllers/tlspolicy_status_updater.go similarity index 57% rename from controllers/tlspolicy_tasks.go rename to controllers/tlspolicy_status_updater.go index b18bcb8a5..4bb0e510e 100644 --- a/controllers/tlspolicy_tasks.go +++ b/controllers/tlspolicy_status_updater.go @@ -7,133 +7,32 @@ import ( "slices" "sync" - "github.com/cert-manager/cert-manager/pkg/apis/certmanager" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" "github.com/samber/lo" "k8s.io/apimachinery/pkg/api/equality" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" "k8s.io/utils/ptr" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -var ( - CertManagerCertificatesResource = certmanagerv1.SchemeGroupVersion.WithResource("certificates") - CertManagerIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("issuers") - CertMangerClusterIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("clusterissuers") - - CertManagerCertificateKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.CertificateKind} - CertManagerIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.IssuerKind} - CertManagerClusterIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.ClusterIssuerKind} -) - -func NewTLSPolicyWorkflow(client *dynamic.DynamicClient) *controller.Workflow { - return &controller.Workflow{ - Precondition: NewValidateTLSPolicyTask().Reconcile, - Postcondition: NewTLSPolicyStatusTask(client).Reconcile, - } -} - -type ValidateTLSPolicyTask struct{} - -func NewValidateTLSPolicyTask() *ValidateTLSPolicyTask { - return &ValidateTLSPolicyTask{} -} - -func (t *ValidateTLSPolicyTask) Subscription() *controller.Subscription { - return &controller.Subscription{ - Events: []controller.ResourceEventMatcher{ - {Kind: &machinery.GatewayGroupKind}, - {Kind: &kuadrantv1alpha1.TLSPolicyKind, EventType: ptr.To(controller.CreateEvent)}, - {Kind: &kuadrantv1alpha1.TLSPolicyKind, EventType: ptr.To(controller.UpdateEvent)}, - {Kind: &CertManagerCertificateKind}, - {Kind: &CertManagerIssuerKind}, - {Kind: &CertManagerClusterIssuerKind}, - }, - ReconcileFunc: t.Reconcile, - } -} - -func (t *ValidateTLSPolicyTask) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { - logger := controller.LoggerFromContext(ctx).WithName("ValidateTLSPolicyTask").WithName("Reconcile") - - // Get all TLS Policies - policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.TLSPolicy, bool) { - p, ok := item.(*kuadrantv1alpha1.TLSPolicy) - return p, ok - }) - - // Get all gateways - gws := lo.FilterMap(topology.Targetables().Items(), func(item machinery.Targetable, index int) (*machinery.Gateway, bool) { - gw, ok := item.(*machinery.Gateway) - return gw, ok - }) - - isCertManagerInstalled := false - installed, ok := s.Load(IsCertManagerInstalledKey) - if ok { - isCertManagerInstalled = installed.(bool) - } else { - logger.V(1).Error(errors.New("isCertManagerInstalled was not found in sync map, defaulting to false"), "sync map error") - } - - for _, policy := range policies { - if policy.DeletionTimestamp != nil { - logger.V(1).Info("tls policy is marked for deletion, skipping", "name", policy.Name, "namespace", policy.Namespace) - continue - } - - if !isCertManagerInstalled { - s.Store(TLSPolicyAcceptedKey(policy.GetUID()), kuadrant.NewErrDependencyNotInstalled("Cert Manager")) - continue - } - - // TODO: This should be only one target ref for now, but what should happen if multiple target refs is supported in the future? - targetRefs := policy.GetTargetRefs() - for _, targetRef := range targetRefs { - // Find gateway defined by target ref - _, ok := lo.Find(gws, func(item *machinery.Gateway) bool { - if item.GetName() == targetRef.GetName() && item.GetNamespace() == targetRef.GetNamespace() { - return true - } - return false - }) - - // Can't find gateway target ref - if !ok { - logger.V(1).Info("tls policy cannot find target ref", "name", policy.Name, "namespace", policy.Namespace) - s.Store(TLSPolicyAcceptedKey(policy.GetUID()), kuadrant.NewErrTargetNotFound(policy.Kind(), policy.GetTargetRef(), apierrors.NewNotFound(kuadrantv1alpha1.TLSPoliciesResource.GroupResource(), policy.GetName()))) - continue - } - - logger.V(1).Info("tls policy found target ref", "name", policy.Name, "namespace", policy.Namespace) - s.Store(TLSPolicyAcceptedKey(policy.GetUID()), nil) - } - } - - return nil -} - -type TLSPolicyStatusTask struct { +type TLSPolicyStatusUpdaterReconciler struct { Client *dynamic.DynamicClient } -func NewTLSPolicyStatusTask(client *dynamic.DynamicClient) *TLSPolicyStatusTask { - return &TLSPolicyStatusTask{Client: client} +func NewTLSPolicyStatusUpdaterReconciler(client *dynamic.DynamicClient) *TLSPolicyStatusUpdaterReconciler { + return &TLSPolicyStatusUpdaterReconciler{Client: client} } -func (t *TLSPolicyStatusTask) Subscription() *controller.Subscription { +func (t *TLSPolicyStatusUpdaterReconciler) Subscription() *controller.Subscription { return &controller.Subscription{ Events: []controller.ResourceEventMatcher{ {Kind: &machinery.GatewayGroupKind}, @@ -143,12 +42,12 @@ func (t *TLSPolicyStatusTask) Subscription() *controller.Subscription { {Kind: &CertManagerIssuerKind}, {Kind: &CertManagerClusterIssuerKind}, }, - ReconcileFunc: t.Reconcile, + ReconcileFunc: t.UpdateStatus, } } -func (t *TLSPolicyStatusTask) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { - logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusTask").WithName("Reconcile") +func (t *TLSPolicyStatusUpdaterReconciler) UpdateStatus(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdaterReconciler").WithName("Reconcile") policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.TLSPolicy, bool) { p, ok := item.(*kuadrantv1alpha1.TLSPolicy) @@ -207,7 +106,7 @@ func (t *TLSPolicyStatusTask) Reconcile(ctx context.Context, _ []controller.Reso return nil } -func (t *TLSPolicyStatusTask) enforcedCondition(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) *metav1.Condition { +func (t *TLSPolicyStatusUpdaterReconciler) enforcedCondition(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) *metav1.Condition { if err := t.isIssuerReady(ctx, tlsPolicy, topology); err != nil { return kuadrant.EnforcedCondition(tlsPolicy, kuadrant.NewErrUnknown(tlsPolicy.Kind(), err), false) } @@ -219,8 +118,8 @@ func (t *TLSPolicyStatusTask) enforcedCondition(ctx context.Context, tlsPolicy * return kuadrant.EnforcedCondition(tlsPolicy, nil, true) } -func (t *TLSPolicyStatusTask) isIssuerReady(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) error { - logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusTask").WithName("isIssuerReady") +func (t *TLSPolicyStatusUpdaterReconciler) isIssuerReady(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) error { + logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdaterReconciler").WithName("isIssuerReady") // Get all gateways gws := lo.FilterMap(topology.Targetables().Items(), func(item machinery.Targetable, index int) (*machinery.Gateway, bool) { @@ -285,7 +184,7 @@ func (t *TLSPolicyStatusTask) isIssuerReady(ctx context.Context, tlsPolicy *kuad return nil } -func (t *TLSPolicyStatusTask) isCertificatesReady(ctx context.Context, p machinery.Policy, topology *machinery.Topology) error { +func (t *TLSPolicyStatusUpdaterReconciler) isCertificatesReady(ctx context.Context, p machinery.Policy, topology *machinery.Topology) error { tlsPolicy, ok := p.(*kuadrantv1alpha1.TLSPolicy) if !ok { return errors.New("invalid policy") @@ -329,31 +228,6 @@ func (t *TLSPolicyStatusTask) isCertificatesReady(ctx context.Context, p machine return nil } -const IsCertManagerInstalledKey = "IsCertManagerInstalled" - -func NewIsCertManagerInstalledTask(restMapper meta.RESTMapper) IsCertManagerInstalledTask { - return IsCertManagerInstalledTask{ - restMapper: restMapper, - } -} - -type IsCertManagerInstalledTask struct { - restMapper meta.RESTMapper -} - -func (t IsCertManagerInstalledTask) Reconcile(ctx context.Context, _ []controller.ResourceEvent, _ *machinery.Topology, _ error, s *sync.Map) error { - logger := controller.LoggerFromContext(ctx).WithName("IsCertManagerInstalledTask").WithName("Reconcile") - isCertManagerInstalled, err := kuadrantgatewayapi.IsCertManagerInstalled(t.restMapper, logger) - - if err != nil { - logger.Error(err, "error checking IsCertManagerInstalled") - } - - s.Store(IsCertManagerInstalledKey, isCertManagerInstalled) - - return nil -} - func TLSPolicyAcceptedKey(uid types.UID) string { return fmt.Sprintf("TLSPolicyValid:%s", uid) } diff --git a/controllers/tlspolicy_tasks_test.go b/controllers/tlspolicy_status_updater_test.go similarity index 99% rename from controllers/tlspolicy_tasks_test.go rename to controllers/tlspolicy_status_updater_test.go index 0724a2d34..ea6d90a76 100644 --- a/controllers/tlspolicy_tasks_test.go +++ b/controllers/tlspolicy_status_updater_test.go @@ -486,7 +486,7 @@ func TestTLSPolicyStatusTask_enforcedCondition(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t1 *testing.T) { - t := &TLSPolicyStatusTask{} + t := &TLSPolicyStatusUpdaterReconciler{} if got := t.enforcedCondition(context.Background(), tt.args.tlsPolicy, tt.args.topology(tt.args.tlsPolicy)); !reflect.DeepEqual(got, tt.want) { t1.Errorf("enforcedCondition() = %v, want %v", got, tt.want) }