From df493645c63a3422db61fbb3fa246d6f687792af Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 20 Sep 2023 15:16:33 +0100 Subject: [PATCH 1/2] Add TLSPolicyAffected condition to target gateway Adds the TLSPolicyAffected condition to the target gateway status following the same logic that is currently implemented for DNSPolicy. --- .../tlspolicy/tlspolicy_controller.go | 73 ++++++++++++++++--- test/integration/tlspolicy_controller_test.go | 21 ++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/pkg/controllers/tlspolicy/tlspolicy_controller.go b/pkg/controllers/tlspolicy/tlspolicy_controller.go index c7461e746..b166aaf8d 100644 --- a/pkg/controllers/tlspolicy/tlspolicy_controller.go +++ b/pkg/controllers/tlspolicy/tlspolicy_controller.go @@ -18,7 +18,9 @@ package tlspolicy import ( "context" + "errors" "fmt" + "reflect" "github.com/go-logr/logr" @@ -45,9 +47,10 @@ import ( ) const ( - TLSPolicyFinalizer = "kuadrant.io/tls-policy" - TLSPoliciesBackRefAnnotation = "kuadrant.io/tlspolicies" - TLSPolicyBackRefAnnotation = "kuadrant.io/tlspolicy" + TLSPolicyFinalizer = "kuadrant.io/tls-policy" + TLSPoliciesBackRefAnnotation = "kuadrant.io/tlspolicies" + TLSPolicyBackRefAnnotation = "kuadrant.io/tlspolicy" + TLSPolicyAffected conditions.ConditionType = "kuadrant.io/TLSPolicyAffected" ) type TLSPolicyRefsConfig struct{} @@ -145,6 +148,8 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } func (r *TLSPolicyReconciler) reconcileResources(ctx context.Context, tlsPolicy *v1alpha1.TLSPolicy, targetNetworkObject client.Object) error { + gatewayCondition := conditions.BuildPolicyAffectedCondition(TLSPolicyAffected, tlsPolicy, targetNetworkObject, conditions.PolicyReasonAccepted, nil) + // validate err := tlsPolicy.Validate() if err != nil { @@ -157,17 +162,33 @@ func (r *TLSPolicyReconciler) reconcileResources(ctx context.Context, tlsPolicy return err } - if err := r.reconcileCertificates(ctx, tlsPolicy, gatewayDiffObj); err != nil { - return err + if err = r.reconcileCertificates(ctx, tlsPolicy, gatewayDiffObj); err != nil { + gatewayCondition = conditions.BuildPolicyAffectedCondition(TLSPolicyAffected, tlsPolicy, targetNetworkObject, conditions.PolicyReasonInvalid, err) + updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) + return errors.Join(fmt.Errorf("reconcile Certificates error %w", err), updateErr) } // set direct back ref - i.e. claim the target network object as taken asap - if err := r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(tlsPolicy), targetNetworkObject, TLSPolicyBackRefAnnotation); err != nil { - return err + if err = r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(tlsPolicy), targetNetworkObject, TLSPolicyBackRefAnnotation); err != nil { + gatewayCondition = conditions.BuildPolicyAffectedCondition(TLSPolicyAffected, tlsPolicy, targetNetworkObject, conditions.PolicyReasonConflicted, err) + updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) + return errors.Join(fmt.Errorf("reconcile TargetBackReference error %w", err), updateErr) + } + + // set annotation of policies affecting the gateway + if err = r.ReconcileGatewayPolicyReferences(ctx, tlsPolicy, gatewayDiffObj); err != nil { + gatewayCondition = conditions.BuildPolicyAffectedCondition(TLSPolicyAffected, tlsPolicy, targetNetworkObject, conditions.PolicyReasonUnknown, err) + updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) + return errors.Join(fmt.Errorf("ReconcileGatewayPolicyReferences error %w", err), updateErr) } - // set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed - return r.ReconcileGatewayPolicyReferences(ctx, tlsPolicy, gatewayDiffObj) + // set gateway policy affected condition status - should be the last step, only when all the reconciliation steps succeed + updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) + if updateErr != nil { + return fmt.Errorf("failed to update gateway conditions %w ", updateErr) + } + + return nil } func (r *TLSPolicyReconciler) deleteResources(ctx context.Context, tlsPolicy *v1alpha1.TLSPolicy, targetNetworkObject client.Object) error { @@ -190,7 +211,12 @@ func (r *TLSPolicyReconciler) deleteResources(ctx context.Context, tlsPolicy *v1 } // update annotation of policies affecting the gateway - return r.ReconcileGatewayPolicyReferences(ctx, tlsPolicy, gatewayDiffObj) + if err := r.ReconcileGatewayPolicyReferences(ctx, tlsPolicy, gatewayDiffObj); err != nil { + return err + } + + // remove gateway policy affected condition status + return r.updateGatewayCondition(ctx, metav1.Condition{Type: string(TLSPolicyAffected)}, gatewayDiffObj) } func (r *TLSPolicyReconciler) calculateStatus(tlsPolicy *v1alpha1.TLSPolicy, specErr error) *v1alpha1.TLSPolicyStatus { @@ -220,6 +246,33 @@ func (r *TLSPolicyReconciler) readyCondition(targetNetworkObjectectKind string, return cond } +func (r *TLSPolicyReconciler) updateGatewayCondition(ctx context.Context, condition metav1.Condition, gatewayDiff *reconcilers.GatewayDiff) error { + + // update condition if needed + for _, gw := range append(gatewayDiff.GatewaysWithValidPolicyRef, gatewayDiff.GatewaysMissingPolicyRef...) { + previous := gw.DeepCopy() + meta.SetStatusCondition(&gw.Status.Conditions, condition) + if !reflect.DeepEqual(previous.Status.Conditions, gw.Status.Conditions) { + if err := r.Client().Status().Update(ctx, gw.Gateway); err != nil { + return err + } + } + } + + // remove condition from gateway that is no longer referenced + for _, gw := range gatewayDiff.GatewaysWithInvalidPolicyRef { + previous := gw.DeepCopy() + meta.RemoveStatusCondition(&gw.Status.Conditions, condition.Type) + if !reflect.DeepEqual(previous.Status.Conditions, gw.Status.Conditions) { + if err := r.Client().Status().Update(ctx, gw.Gateway); err != nil { + return err + } + } + } + + return nil +} + // SetupWithManager sets up the controller with the Manager. func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { gatewayEventMapper := events.NewGatewayEventMapper(r.Logger(), &TLSPolicyRefsConfig{}, "tlspolicy") diff --git a/test/integration/tlspolicy_controller_test.go b/test/integration/tlspolicy_controller_test.go index 32a5ddbb4..01f44d6be 100644 --- a/test/integration/tlspolicy_controller_test.go +++ b/test/integration/tlspolicy_controller_test.go @@ -142,6 +142,27 @@ var _ = Describe("TLSPolicy", Ordered, func() { }, time.Second*5, time.Second).Should(BeNil()) }) + It("should set policy affected condition in gateway status", func() { + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway); err != nil { + return err + } + + policyAffectedCond := meta.FindStatusCondition(gateway.Status.Conditions, string(TLSPolicyAffected)) + if policyAffectedCond == nil { + return fmt.Errorf("policy affected conditon expected but not found") + } + if policyAffectedCond.ObservedGeneration != gateway.Generation { + return fmt.Errorf("expected policy affected cond generation to be %d but got %d", gateway.Generation, policyAffectedCond.ObservedGeneration) + } + if !meta.IsStatusConditionTrue(gateway.Status.Conditions, string(TLSPolicyAffected)) { + return fmt.Errorf("expected gateway status condition %s to be True", TLSPolicyAffected) + } + + return nil + }, time.Second*15, time.Second).Should(BeNil()) + }) + }) Context("with http listener", func() { From a6833c148945b771b53adbe039bce7a7b21079a9 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 20 Sep 2023 15:23:27 +0100 Subject: [PATCH 2/2] Update DNSPolicy Updates the DNSPolicy reconciliation flow to update/remove (annotations) and then update status as the last step. --- pkg/controllers/dnspolicy/dnspolicy_controller.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/controllers/dnspolicy/dnspolicy_controller.go b/pkg/controllers/dnspolicy/dnspolicy_controller.go index 5dce06243..b79722525 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_controller.go +++ b/pkg/controllers/dnspolicy/dnspolicy_controller.go @@ -183,18 +183,19 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy return errors.Join(fmt.Errorf("reconcile TargetBackReference error %w", err), updateErr) } + // set annotation of policies affecting the gateway if err := r.ReconcileGatewayPolicyReferences(ctx, dnsPolicy, gatewayDiffObj); err != nil { gatewayCondition = conditions.BuildPolicyAffectedCondition(DNSPolicyAffected, dnsPolicy, targetNetworkObject, conditions.PolicyReasonUnknown, err) updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) return errors.Join(fmt.Errorf("ReconcileGatewayPolicyReferences error %w", err), updateErr) } + // set gateway policy affected condition status - should be the last step, only when all the reconciliation steps succeed updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj) if updateErr != nil { return fmt.Errorf("failed to update gateway conditions %w ", updateErr) } - // set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed return nil } @@ -227,13 +228,13 @@ func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1 return err } - // clean up condition on gateway - if err := r.updateGatewayCondition(ctx, metav1.Condition{Type: string(DNSPolicyAffected)}, gatewayDiffObj); err != nil { + // update annotation of policies affecting the gateway + if err := r.ReconcileGatewayPolicyReferences(ctx, dnsPolicy, gatewayDiffObj); err != nil { return err } - // update annotation of policies affecting the gateway - return r.ReconcileGatewayPolicyReferences(ctx, dnsPolicy, gatewayDiffObj) + // remove gateway policy affected condition status + return r.updateGatewayCondition(ctx, metav1.Condition{Type: string(DNSPolicyAffected)}, gatewayDiffObj) } func (r *DNSPolicyReconciler) calculateStatus(dnsPolicy *v1alpha1.DNSPolicy, specErr error) *v1alpha1.DNSPolicyStatus {