From af8c44ce95fab1e0ad5d10f0a2bae4d8fdeb667c Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 15 Oct 2024 17:10:40 +0100 Subject: [PATCH] fixup: conventions naming and move functions Signed-off-by: KevFan --- .../effective_tls_policies_reconciler.go | 108 ++++++++++++++++- ...effective_tls_policies_reconciler_test.go} | 0 controllers/tls_workflow.go | 4 +- controllers/tlspolicies_validator.go | 2 +- controllers/tlspolicy_certmanager.go | 112 ------------------ controllers/tlspolicy_status_updater.go | 20 ++-- controllers/tlspolicy_status_updater_test.go | 2 +- 7 files changed, 121 insertions(+), 127 deletions(-) rename controllers/{tlspolicy_certmanager_test.go => effective_tls_policies_reconciler_test.go} (100%) delete mode 100644 controllers/tlspolicy_certmanager.go diff --git a/controllers/effective_tls_policies_reconciler.go b/controllers/effective_tls_policies_reconciler.go index 7098a739a..e6cf2601f 100644 --- a/controllers/effective_tls_policies_reconciler.go +++ b/controllers/effective_tls_policies_reconciler.go @@ -208,6 +208,11 @@ func expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.G tlsHosts := make(map[corev1.ObjectReference][]string) for i, l := range gateway.Spec.Listeners { + hostname := "*" + if l.Hostname != nil { + hostname = string(*l.Hostname) + } + err := validateGatewayListenerBlock(field.NewPath("spec", "listeners").Index(i), l, gateway).ToAggregate() if err != nil { log.Info("Skipped a listener block: " + err.Error()) @@ -225,7 +230,7 @@ func expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.G } // Gateway API hostname explicitly disallows IP addresses, so this // should be OK. - tlsHosts[secretRef] = append(tlsHosts[secretRef], string(*l.Hostname)) + tlsHosts[secretRef] = append(tlsHosts[secretRef], hostname) } } @@ -285,3 +290,104 @@ func buildCertManagerCertificate(tlsPolicy *kuadrantv1alpha1.TLSPolicy, secretRe translatePolicy(crt, tlsPolicy.Spec) return crt } + +// https://cert-manager.io/docs/usage/gateway/#supported-annotations +// Helper functions largely based on cert manager https://github.com/cert-manager/cert-manager/blob/master/pkg/controller/certificate-shim/sync.go + +func validateGatewayListenerBlock(path *field.Path, l gatewayapiv1.Listener, ingLike metav1.Object) field.ErrorList { + var errs field.ErrorList + + if l.Hostname == nil || *l.Hostname == "" { + errs = append(errs, field.Required(path.Child("hostname"), "the hostname cannot be empty")) + } + + if l.TLS == nil { + errs = append(errs, field.Required(path.Child("tls"), "the TLS block cannot be empty")) + return errs + } + + if len(l.TLS.CertificateRefs) == 0 { + errs = append(errs, field.Required(path.Child("tls").Child("certificateRef"), + "listener has no certificateRefs")) + } else { + // check that each CertificateRef is valid + for i, secretRef := range l.TLS.CertificateRefs { + if *secretRef.Group != "core" && *secretRef.Group != "" { + errs = append(errs, field.NotSupported(path.Child("tls").Child("certificateRef").Index(i).Child("group"), + *secretRef.Group, []string{"core", ""})) + } + + if *secretRef.Kind != "Secret" && *secretRef.Kind != "" { + errs = append(errs, field.NotSupported(path.Child("tls").Child("certificateRef").Index(i).Child("kind"), + *secretRef.Kind, []string{"Secret", ""})) + } + + if secretRef.Namespace != nil && string(*secretRef.Namespace) != ingLike.GetNamespace() { + errs = append(errs, field.Invalid(path.Child("tls").Child("certificateRef").Index(i).Child("namespace"), + *secretRef.Namespace, "cross-namespace secret references are not allowed in listeners")) + } + } + } + + if l.TLS.Mode == nil { + errs = append(errs, field.Required(path.Child("tls").Child("mode"), + "the mode field is required")) + } else { + if *l.TLS.Mode != gatewayapiv1.TLSModeTerminate { + errs = append(errs, field.NotSupported(path.Child("tls").Child("mode"), + *l.TLS.Mode, []string{string(gatewayapiv1.TLSModeTerminate)})) + } + } + + return errs +} + +// translatePolicy updates the Certificate spec using the TLSPolicy spec +// converted from https://github.com/cert-manager/cert-manager/blob/master/pkg/controller/certificate-shim/helper.go#L63 +func translatePolicy(crt *certmanv1.Certificate, tlsPolicy kuadrantv1alpha1.TLSPolicySpec) { + if tlsPolicy.CommonName != "" { + crt.Spec.CommonName = tlsPolicy.CommonName + } + + if tlsPolicy.Duration != nil { + crt.Spec.Duration = tlsPolicy.Duration + } + + if tlsPolicy.RenewBefore != nil { + crt.Spec.RenewBefore = tlsPolicy.RenewBefore + } + + if tlsPolicy.RenewBefore != nil { + crt.Spec.RenewBefore = tlsPolicy.RenewBefore + } + + if tlsPolicy.Usages != nil { + crt.Spec.Usages = tlsPolicy.Usages + } + + if tlsPolicy.RevisionHistoryLimit != nil { + crt.Spec.RevisionHistoryLimit = tlsPolicy.RevisionHistoryLimit + } + + if tlsPolicy.PrivateKey != nil { + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &certmanv1.CertificatePrivateKey{} + } + + if tlsPolicy.PrivateKey.Algorithm != "" { + crt.Spec.PrivateKey.Algorithm = tlsPolicy.PrivateKey.Algorithm + } + + if tlsPolicy.PrivateKey.Encoding != "" { + crt.Spec.PrivateKey.Encoding = tlsPolicy.PrivateKey.Encoding + } + + if tlsPolicy.PrivateKey.Size != 0 { + crt.Spec.PrivateKey.Size = tlsPolicy.PrivateKey.Size + } + + if tlsPolicy.PrivateKey.RotationPolicy != "" { + crt.Spec.PrivateKey.RotationPolicy = tlsPolicy.PrivateKey.RotationPolicy + } + } +} diff --git a/controllers/tlspolicy_certmanager_test.go b/controllers/effective_tls_policies_reconciler_test.go similarity index 100% rename from controllers/tlspolicy_certmanager_test.go rename to controllers/effective_tls_policies_reconciler_test.go diff --git a/controllers/tls_workflow.go b/controllers/tls_workflow.go index 2b327af54..5e47ec1c9 100644 --- a/controllers/tls_workflow.go +++ b/controllers/tls_workflow.go @@ -35,11 +35,11 @@ var ( func NewTLSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme, isCertManagerInstalled bool) *controller.Workflow { return &controller.Workflow{ - Precondition: NewValidateTLSPoliciesValidatorReconciler(isCertManagerInstalled).Subscription().Reconcile, + Precondition: NewTLSPoliciesValidator(isCertManagerInstalled).Subscription().Reconcile, Tasks: []controller.ReconcileFunc{ NewEffectiveTLSPoliciesReconciler(client, scheme).Subscription().Reconcile, }, - Postcondition: NewTLSPolicyStatusUpdaterReconciler(client).Subscription().Reconcile, + Postcondition: NewTLSPolicyStatusUpdater(client).Subscription().Reconcile, } } diff --git a/controllers/tlspolicies_validator.go b/controllers/tlspolicies_validator.go index 6da50c026..68982e4c8 100644 --- a/controllers/tlspolicies_validator.go +++ b/controllers/tlspolicies_validator.go @@ -17,7 +17,7 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) -func NewValidateTLSPoliciesValidatorReconciler(isCertManagerInstalled bool) *ValidateTLSPoliciesValidatorReconciler { +func NewTLSPoliciesValidator(isCertManagerInstalled bool) *ValidateTLSPoliciesValidatorReconciler { return &ValidateTLSPoliciesValidatorReconciler{ isCertManagerInstalled: isCertManagerInstalled, } diff --git a/controllers/tlspolicy_certmanager.go b/controllers/tlspolicy_certmanager.go deleted file mode 100644 index 7cdee7de0..000000000 --- a/controllers/tlspolicy_certmanager.go +++ /dev/null @@ -1,112 +0,0 @@ -package controllers - -import ( - certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/validation/field" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/kuadrant/kuadrant-operator/api/v1alpha1" -) - -// https://cert-manager.io/docs/usage/gateway/#supported-annotations -// Helper functions largely based on cert manager https://github.com/cert-manager/cert-manager/blob/master/pkg/controller/certificate-shim/sync.go - -func validateGatewayListenerBlock(path *field.Path, l gatewayapiv1.Listener, ingLike metav1.Object) field.ErrorList { - var errs field.ErrorList - - if l.Hostname == nil || *l.Hostname == "" { - errs = append(errs, field.Required(path.Child("hostname"), "the hostname cannot be empty")) - } - - if l.TLS == nil { - errs = append(errs, field.Required(path.Child("tls"), "the TLS block cannot be empty")) - return errs - } - - if len(l.TLS.CertificateRefs) == 0 { - errs = append(errs, field.Required(path.Child("tls").Child("certificateRef"), - "listener has no certificateRefs")) - } else { - // check that each CertificateRef is valid - for i, secretRef := range l.TLS.CertificateRefs { - if *secretRef.Group != "core" && *secretRef.Group != "" { - errs = append(errs, field.NotSupported(path.Child("tls").Child("certificateRef").Index(i).Child("group"), - *secretRef.Group, []string{"core", ""})) - } - - if *secretRef.Kind != "Secret" && *secretRef.Kind != "" { - errs = append(errs, field.NotSupported(path.Child("tls").Child("certificateRef").Index(i).Child("kind"), - *secretRef.Kind, []string{"Secret", ""})) - } - - if secretRef.Namespace != nil && string(*secretRef.Namespace) != ingLike.GetNamespace() { - errs = append(errs, field.Invalid(path.Child("tls").Child("certificateRef").Index(i).Child("namespace"), - *secretRef.Namespace, "cross-namespace secret references are not allowed in listeners")) - } - } - } - - if l.TLS.Mode == nil { - errs = append(errs, field.Required(path.Child("tls").Child("mode"), - "the mode field is required")) - } else { - if *l.TLS.Mode != gatewayapiv1.TLSModeTerminate { - errs = append(errs, field.NotSupported(path.Child("tls").Child("mode"), - *l.TLS.Mode, []string{string(gatewayapiv1.TLSModeTerminate)})) - } - } - - return errs -} - -// translatePolicy updates the Certificate spec using the TLSPolicy spec -// converted from https://github.com/cert-manager/cert-manager/blob/master/pkg/controller/certificate-shim/helper.go#L63 -func translatePolicy(crt *certmanv1.Certificate, tlsPolicy v1alpha1.TLSPolicySpec) { - if tlsPolicy.CommonName != "" { - crt.Spec.CommonName = tlsPolicy.CommonName - } - - if tlsPolicy.Duration != nil { - crt.Spec.Duration = tlsPolicy.Duration - } - - if tlsPolicy.RenewBefore != nil { - crt.Spec.RenewBefore = tlsPolicy.RenewBefore - } - - if tlsPolicy.RenewBefore != nil { - crt.Spec.RenewBefore = tlsPolicy.RenewBefore - } - - if tlsPolicy.Usages != nil { - crt.Spec.Usages = tlsPolicy.Usages - } - - if tlsPolicy.RevisionHistoryLimit != nil { - crt.Spec.RevisionHistoryLimit = tlsPolicy.RevisionHistoryLimit - } - - if tlsPolicy.PrivateKey != nil { - if crt.Spec.PrivateKey == nil { - crt.Spec.PrivateKey = &certmanv1.CertificatePrivateKey{} - } - - if tlsPolicy.PrivateKey.Algorithm != "" { - crt.Spec.PrivateKey.Algorithm = tlsPolicy.PrivateKey.Algorithm - } - - if tlsPolicy.PrivateKey.Encoding != "" { - crt.Spec.PrivateKey.Encoding = tlsPolicy.PrivateKey.Encoding - } - - if tlsPolicy.PrivateKey.Size != 0 { - crt.Spec.PrivateKey.Size = tlsPolicy.PrivateKey.Size - } - - if tlsPolicy.PrivateKey.RotationPolicy != "" { - crt.Spec.PrivateKey.RotationPolicy = tlsPolicy.PrivateKey.RotationPolicy - } - } -} diff --git a/controllers/tlspolicy_status_updater.go b/controllers/tlspolicy_status_updater.go index 35c45df09..82f7fe15e 100644 --- a/controllers/tlspolicy_status_updater.go +++ b/controllers/tlspolicy_status_updater.go @@ -24,15 +24,15 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -type TLSPolicyStatusUpdaterReconciler struct { +type TLSPolicyStatusUpdater struct { Client *dynamic.DynamicClient } -func NewTLSPolicyStatusUpdaterReconciler(client *dynamic.DynamicClient) *TLSPolicyStatusUpdaterReconciler { - return &TLSPolicyStatusUpdaterReconciler{Client: client} +func NewTLSPolicyStatusUpdater(client *dynamic.DynamicClient) *TLSPolicyStatusUpdater { + return &TLSPolicyStatusUpdater{Client: client} } -func (t *TLSPolicyStatusUpdaterReconciler) Subscription() *controller.Subscription { +func (t *TLSPolicyStatusUpdater) Subscription() *controller.Subscription { return &controller.Subscription{ Events: []controller.ResourceEventMatcher{ {Kind: &machinery.GatewayGroupKind}, @@ -46,8 +46,8 @@ func (t *TLSPolicyStatusUpdaterReconciler) Subscription() *controller.Subscripti } } -func (t *TLSPolicyStatusUpdaterReconciler) UpdateStatus(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { - logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdaterReconciler").WithName("UpdateStatus") +func (t *TLSPolicyStatusUpdater) UpdateStatus(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, s *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdater").WithName("UpdateStatus") policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.TLSPolicy, bool) { p, ok := item.(*kuadrantv1alpha1.TLSPolicy) @@ -102,7 +102,7 @@ func (t *TLSPolicyStatusUpdaterReconciler) UpdateStatus(ctx context.Context, _ [ return nil } -func (t *TLSPolicyStatusUpdaterReconciler) enforcedCondition(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) *metav1.Condition { +func (t *TLSPolicyStatusUpdater) 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) } @@ -114,8 +114,8 @@ func (t *TLSPolicyStatusUpdaterReconciler) enforcedCondition(ctx context.Context return kuadrant.EnforcedCondition(tlsPolicy, nil, true) } -func (t *TLSPolicyStatusUpdaterReconciler) isIssuerReady(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) error { - logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdaterReconciler").WithName("isIssuerReady") +func (t *TLSPolicyStatusUpdater) isIssuerReady(ctx context.Context, tlsPolicy *kuadrantv1alpha1.TLSPolicy, topology *machinery.Topology) error { + logger := controller.LoggerFromContext(ctx).WithName("TLSPolicyStatusUpdater").WithName("isIssuerReady") // Get all gateways gws := lo.FilterMap(topology.Targetables().Items(), func(item machinery.Targetable, index int) (*machinery.Gateway, bool) { @@ -180,7 +180,7 @@ func (t *TLSPolicyStatusUpdaterReconciler) isIssuerReady(ctx context.Context, tl return nil } -func (t *TLSPolicyStatusUpdaterReconciler) isCertificatesReady(p machinery.Policy, topology *machinery.Topology) error { +func (t *TLSPolicyStatusUpdater) isCertificatesReady(p machinery.Policy, topology *machinery.Topology) error { tlsPolicy, ok := p.(*kuadrantv1alpha1.TLSPolicy) if !ok { return errors.New("invalid policy") diff --git a/controllers/tlspolicy_status_updater_test.go b/controllers/tlspolicy_status_updater_test.go index f43199e21..d482d82d4 100644 --- a/controllers/tlspolicy_status_updater_test.go +++ b/controllers/tlspolicy_status_updater_test.go @@ -461,7 +461,7 @@ func TestTLSPolicyStatusTask_enforcedCondition(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t1 *testing.T) { - t := &TLSPolicyStatusUpdaterReconciler{} + t := TLSPolicyStatusUpdater{} 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) }