Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

Commit

Permalink
Merge pull request #635 from laurafitzgerald/gh-628
Browse files Browse the repository at this point in the history 
gh-628 fix deletion of dnsrecord and certificates on deletion of gateway target for dnspolicy and tlspolicy
  • Loading branch information
@openshift-ci
openshift-ci[bot] authored Nov 1, 2023
2 parents 59777f9 + 9598526 commit 76148fd
Show file tree
Hide file tree
Showing 9 changed files with 248 additions and 100 deletions.
44 changes: 30 additions & 14 deletions pkg/controllers/dnspolicy/dns_helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,27 @@ func findMatchingManagedZone(originalHost, host string, zones []v1alpha1.Managed
}

func commonDNSRecordLabels(gwKey, apKey client.ObjectKey) map[string]string {
common := map[string]string{}
for k, v := range policyDNSRecordLabels(apKey) {
common[k] = v
}
for k, v := range gatewayDNSRecordLabels(gwKey) {
common[k] = v
}
return common
}

func policyDNSRecordLabels(apKey client.ObjectKey) map[string]string {
return map[string]string{
DNSPolicyBackRefAnnotation: apKey.Name,
fmt.Sprintf("%s-namespace", DNSPolicyBackRefAnnotation): apKey.Namespace,
LabelGatewayNSRef: gwKey.Namespace,
LabelGatewayReference: gwKey.Name,
}
}

func gatewayDNSRecordLabels(gwKey client.ObjectKey) map[string]string {
return map[string]string{
LabelGatewayNSRef: gwKey.Namespace,
LabelGatewayReference: gwKey.Name,
}
}

Expand Down Expand Up @@ -282,13 +298,13 @@ func createOrUpdateEndpoint(dnsName string, targets v1alpha1.Targets, recordType
}

// removeDNSForDeletedListeners remove any DNSRecords that are associated with listeners that no longer exist in this gateway
func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGateway *gatewayv1beta1.Gateway) error {
func (dh *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGateway *gatewayv1beta1.Gateway) error {
dnsList := &v1alpha1.DNSRecordList{}
//List all dns records that belong to this gateway
labelSelector := &client.MatchingLabels{
LabelGatewayReference: upstreamGateway.Name,
}
if err := r.List(ctx, dnsList, labelSelector, &client.ListOptions{Namespace: upstreamGateway.Namespace}); err != nil {
if err := dh.List(ctx, dnsList, labelSelector, &client.ListOptions{Namespace: upstreamGateway.Namespace}); err != nil {
return err
}

Expand All @@ -301,7 +317,7 @@ func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGa
}
}
if !listenerExists {
if err := r.Delete(ctx, &dns, &client.DeleteOptions{}); client.IgnoreNotFound(err) != nil {
if err := dh.Delete(ctx, &dns, &client.DeleteOptions{}); client.IgnoreNotFound(err) != nil {
return err
}
}
Expand All @@ -310,9 +326,9 @@ func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGa

}

func (r *dnsHelper) getManagedZoneForListener(ctx context.Context, ns string, listener gatewayv1beta1.Listener) (*v1alpha1.ManagedZone, error) {
func (dh *dnsHelper) getManagedZoneForListener(ctx context.Context, ns string, listener gatewayv1beta1.Listener) (*v1alpha1.ManagedZone, error) {
var managedZones v1alpha1.ManagedZoneList
if err := r.List(ctx, &managedZones, client.InNamespace(ns)); err != nil {
if err := dh.List(ctx, &managedZones, client.InNamespace(ns)); err != nil {
log.FromContext(ctx).Error(err, "unable to list managed zones for gateway ", "in ns", ns)
return nil, err
}
Expand All @@ -325,37 +341,37 @@ func dnsRecordName(gatewayName, listenerName string) string {
return fmt.Sprintf("%s-%s", gatewayName, listenerName)
}

func (r *dnsHelper) createDNSRecordForListener(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy, mz *v1alpha1.ManagedZone, listener gatewayv1beta1.Listener) (*v1alpha1.DNSRecord, error) {
func (dh *dnsHelper) createDNSRecordForListener(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy, mz *v1alpha1.ManagedZone, listener gatewayv1beta1.Listener) (*v1alpha1.DNSRecord, error) {

log := log.FromContext(ctx)
log.Info("creating dns for gateway listener", "listener", listener.Name)
dnsRecord := r.buildDNSRecordForListener(gateway, dnsPolicy, listener, mz)
if err := controllerutil.SetControllerReference(mz, dnsRecord, r.Scheme()); err != nil {
dnsRecord := dh.buildDNSRecordForListener(gateway, dnsPolicy, listener, mz)
if err := controllerutil.SetControllerReference(mz, dnsRecord, dh.Scheme()); err != nil {
return dnsRecord, err
}

err := r.Create(ctx, dnsRecord, &client.CreateOptions{})
err := dh.Create(ctx, dnsRecord, &client.CreateOptions{})
if err != nil && !k8serrors.IsAlreadyExists(err) {
return dnsRecord, err
}
if err != nil && k8serrors.IsAlreadyExists(err) {
err = r.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
err = dh.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
if err != nil {
return dnsRecord, err
}
}
return dnsRecord, nil
}

func (r *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav1.Object, listener gatewayv1beta1.Listener) error {
func (dh *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav1.Object, listener gatewayv1beta1.Listener) error {
recordName := dnsRecordName(owner.GetName(), string(listener.Name))
dnsRecord := v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
Name: recordName,
Namespace: owner.GetNamespace(),
},
}
return r.Delete(ctx, &dnsRecord, &client.DeleteOptions{})
return dh.Delete(ctx, &dnsRecord, &client.DeleteOptions{})
}

func isWildCardListener(l gatewayv1beta1.Listener) bool {
Expand Down
7 changes: 3 additions & 4 deletions pkg/controllers/dnspolicy/dnspolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy
return errors.Join(fmt.Errorf("reconcile DNSRecords error %w", err), updateErr)
}

if err = r.reconcileHealthChecks(ctx, dnsPolicy, gatewayDiffObj); err != nil {
if err = r.reconcileHealthCheckProbes(ctx, dnsPolicy, gatewayDiffObj); err != nil {
gatewayCondition = conditions.BuildPolicyAffectedCondition(DNSPolicyAffected, dnsPolicy, targetNetworkObject, conditions.PolicyReasonInvalid, err)
updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj)
return errors.Join(fmt.Errorf("reconcile HealthChecks error %w", err), updateErr)
Expand Down Expand Up @@ -195,13 +195,12 @@ func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1
if err != nil {
return err
}

if err := r.reconcileDNSRecords(ctx, dnsPolicy, gatewayDiffObj); err != nil {
if err = r.deleteDNSRecords(ctx, dnsPolicy); err != nil {
log.V(3).Info("error reconciling DNS records from delete, returning", "error", err)
return err
}

if err := r.reconcileHealthChecks(ctx, dnsPolicy, gatewayDiffObj); err != nil {
if err := r.deleteHealthCheckProbes(ctx, dnsPolicy); err != nil {
return err
}

Expand Down
24 changes: 15 additions & 9 deletions pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,21 @@ import (
func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
log := crlog.FromContext(ctx)

log.V(3).Info("reconciling dns records")
for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
log.V(1).Info("reconcileDNSRecords: gateway with invalid policy ref", "key", gw.Key())
err := r.deleteGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy)
if err != nil {
return err
if err := r.deleteGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil {
return fmt.Errorf("error deleting dns records for gw %v: %w", gw.Gateway.Name, err)
}
}

// Reconcile DNSRecords for each gateway directly referred by the policy (existing and new)
for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) {
log.V(1).Info("reconcileDNSRecords: gateway with valid and missing policy ref", "key", gw.Key())
err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy)
if err != nil {
return err
log.V(1).Info("reconcileDNSRecords: gateway with valid or missing policy ref", "key", gw.Key())
if err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil {
return fmt.Errorf("error reconciling dns records for gateway %v: %w", gw.Gateway.Name, err)
}
}

return nil
}

Expand Down Expand Up @@ -124,9 +122,17 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
}

func (r *DNSPolicyReconciler) deleteGatewayDNSRecords(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
return r.deleteDNSRecordsWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
}

func (r *DNSPolicyReconciler) deleteDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error {
return r.deleteDNSRecordsWithLabels(ctx, policyDNSRecordLabels(client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
}

func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lbls map[string]string, namespace string) error {
log := crlog.FromContext(ctx)

listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)))}
listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace}
recordsList := &v1alpha1.DNSRecordList{}
if err := r.Client().List(ctx, recordsList, listOptions); err != nil {
return err
Expand Down
75 changes: 48 additions & 27 deletions pkg/controllers/dnspolicy/dnspolicy_healthchecks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/controller-runtime/pkg/client"
crlog "sigs.k8s.io/controller-runtime/pkg/log"
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kuadrant/kuadrant-operator/pkg/common"
"github.com/kuadrant/kuadrant-operator/pkg/reconcilers"
Expand All @@ -19,33 +20,33 @@ import (
"github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1"
)

func (r *DNSPolicyReconciler) reconcileHealthChecks(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
func (r *DNSPolicyReconciler) reconcileHealthCheckProbes(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
log := crlog.FromContext(ctx)

log.V(3).Info("reconciling health checks")
for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
log.V(1).Info("reconcileHealthCheckProbes: gateway with invalid policy ref", "key", gw.Key())
if err := r.deleteGatewayHealthCheckProbes(ctx, gw.Gateway, dnsPolicy); err != nil {
return fmt.Errorf("error deleting probes for gw %v: %w", gw.Gateway.Name, err)
}
}

// Reconcile DNSHealthCheckProbes for each gateway directly referred by the policy (existing and new)
for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) {
log.V(3).Info("reconciling probes", "gateway", gw.Name)
expectedProbes := r.expectedProbesForGateway(ctx, gw, dnsPolicy)
if err := r.createOrUpdateProbes(ctx, expectedProbes); err != nil {
return fmt.Errorf("error creating and updating expected proves for gateway %v: %w", gw.Gateway.Name, err)
expectedProbes := r.expectedHealthCheckProbesForGateway(ctx, gw, dnsPolicy)
if err := r.createOrUpdateHealthCheckProbes(ctx, expectedProbes); err != nil {
return fmt.Errorf("error creating or updating expected probes for gateway %v: %w", gw.Gateway.Name, err)
}
if err := r.deleteUnexpectedGatewayProbes(ctx, expectedProbes, gw, dnsPolicy); err != nil {
if err := r.deleteUnexpectedGatewayHealthCheckProbes(ctx, expectedProbes, gw.Gateway, dnsPolicy); err != nil {
return fmt.Errorf("error removing unexpected probes for gateway %v: %w", gw.Gateway.Name, err)
}

}

for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
log.V(3).Info("deleting probes", "gateway", gw.Gateway.Name)
if err := r.deleteUnexpectedGatewayProbes(ctx, []*v1alpha1.DNSHealthCheckProbe{}, gw, dnsPolicy); err != nil {
return fmt.Errorf("error deleting probes for gw %v: %w", gw.Gateway.Name, err)
}
}

return nil
}

func (r *DNSPolicyReconciler) createOrUpdateProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe) error {
func (r *DNSPolicyReconciler) createOrUpdateHealthCheckProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe) error {
//create or update all expected probes
for _, hcProbe := range expectedProbes {
p := &v1alpha1.DNSHealthCheckProbe{}
Expand All @@ -66,29 +67,49 @@ func (r *DNSPolicyReconciler) createOrUpdateProbes(ctx context.Context, expected
return nil
}

func (r *DNSPolicyReconciler) deleteUnexpectedGatewayProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) error {
func (r *DNSPolicyReconciler) deleteGatewayHealthCheckProbes(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
return r.deleteHealthCheckProbesWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
}

func (r *DNSPolicyReconciler) deleteHealthCheckProbes(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error {
return r.deleteHealthCheckProbesWithLabels(ctx, policyDNSRecordLabels(client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
}

func (r *DNSPolicyReconciler) deleteHealthCheckProbesWithLabels(ctx context.Context, lbls map[string]string, namespace string) error {
probes := &v1alpha1.DNSHealthCheckProbeList{}
listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace}
if err := r.Client().List(ctx, probes, listOptions); client.IgnoreNotFound(err) != nil {
return err
}
for _, p := range probes.Items {
if err := r.Client().Delete(ctx, &p); err != nil {
return err
}
}
return nil
}

func (r *DNSPolicyReconciler) deleteUnexpectedGatewayHealthCheckProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
// remove any probes for this gateway and DNS Policy that are no longer expected
existingProbes := &v1alpha1.DNSHealthCheckProbeList{}
dnsLabels := commonDNSRecordLabels(client.ObjectKeyFromObject(gw), client.ObjectKeyFromObject(dnsPolicy))
dnsLabels := commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy))
listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(dnsLabels)}
if err := r.Client().List(ctx, existingProbes, listOptions); client.IgnoreNotFound(err) != nil {
return err
} else {
for _, p := range existingProbes.Items {
if !slice.Contains(expectedProbes, func(expectedProbe *v1alpha1.DNSHealthCheckProbe) bool {
return expectedProbe.Name == p.Name && expectedProbe.Namespace == p.Namespace
}) {
if err := r.Client().Delete(ctx, &p); err != nil {
return err
}
}
for _, p := range existingProbes.Items {
if !slice.Contains(expectedProbes, func(expectedProbe *v1alpha1.DNSHealthCheckProbe) bool {
return expectedProbe.Name == p.Name && expectedProbe.Namespace == p.Namespace
}) {
if err := r.Client().Delete(ctx, &p); err != nil {
return err
}
}
}

return nil
}

func (r *DNSPolicyReconciler) expectedProbesForGateway(ctx context.Context, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) []*v1alpha1.DNSHealthCheckProbe {
func (r *DNSPolicyReconciler) expectedHealthCheckProbesForGateway(ctx context.Context, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) []*v1alpha1.DNSHealthCheckProbe {
log := crlog.FromContext(ctx)
var healthChecks []*v1alpha1.DNSHealthCheckProbe
if dnsPolicy.Spec.HealthCheck == nil {
Expand Down Expand Up @@ -126,7 +147,7 @@ func (r *DNSPolicyReconciler) expectedProbesForGateway(ctx context.Context, gw c
} else {
protocol = string(*dnsPolicy.Spec.HealthCheck.Protocol)
}
log.V(1).Info("reconcileHealthChecks: adding health check for target", "target", address.Value)
log.V(1).Info("reconcileHealthCheckProbes: adding health check for target", "target", address.Value)
healthCheck := &v1alpha1.DNSHealthCheckProbe{
ObjectMeta: metav1.ObjectMeta{
Name: dnsHealthCheckProbeName(matches[1], gw.Name, string(listener.Name)),
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/dnspolicy/dnspolicy_healthchecks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,9 @@ func TestDNSPolicyReconciler_expectedProbesForGateway(t *testing.T) {
DNSProvider: tt.fields.DNSProvider,
dnsHelper: tt.fields.dnsHelper,
}
got := r.expectedProbesForGateway(tt.args.ctx, tt.args.gw, tt.args.dnsPolicy)
got := r.expectedHealthCheckProbesForGateway(tt.args.ctx, tt.args.gw, tt.args.dnsPolicy)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("expectedProbesForGateway() got = %v, want %v", got, tt.want)
t.Errorf("expectedHealthCheckProbesForGateway() got = %v, want %v", got, tt.want)
}
})
}
Expand Down
Loading

0 comments on commit 76148fd

Please sign in to comment.