From b12d0d06f798cb5a28bcc2b0a17e823b59b0827b Mon Sep 17 00:00:00 2001 From: Phil Brookes Date: Thu, 20 Nov 2025 14:12:08 +0100 Subject: [PATCH] DNS Groups Signed-off-by: Phil Brookes rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED --- .gitignore | 1 + Makefile | 2 +- api/v1alpha1/conditions.go | 5 + api/v1alpha1/dnsrecord_types.go | 11 + bundle/manifests/kuadrant.io_dnsrecords.yaml | 3 + charts/dns-operator/templates/manifests.yaml | 3 + cmd/main.go | 2 + cmd/plugin/get-zone-records.go | 2 +- cmd/plugin/secret_generation.go | 4 +- config/coredns/Corefile | 8 +- config/crd/bases/kuadrant.io_dnsrecords.yaml | 3 + .../dns-provider/coredns/kustomization.yaml | 1 + coredns/examples/Corefile | 31 +- .../controller/base_dnsrecord_reconciler.go | 27 +- internal/controller/dnsrecord_accessor.go | 33 +- internal/controller/dnsrecord_controller.go | 57 ++- .../dnsrecord_controller_delegation_test.go | 64 +-- internal/controller/dnsrecord_groups.go | 484 ++++++++++++++++++ internal/controller/dnsrecord_groups_test.go | 449 ++++++++++++++++ internal/controller/dnsrecord_healthchecks.go | 1 - internal/controller/helper_test.go | 82 ++- .../controller/remote_dnsrecord_controller.go | 28 +- internal/controller/suite_test.go | 59 ++- internal/external-dns/registry/group.go | 13 +- internal/external-dns/registry/txt.go | 187 +++++++ internal/external-dns/registry/txt_test.go | 282 +++++++++- internal/provider/inmemory/inmemory.go | 5 + test/e2e/multi_record_test.go | 31 +- test/e2e/single_record_test.go | 32 +- types/group.go | 15 + 30 files changed, 1811 insertions(+), 114 deletions(-) create mode 100644 internal/controller/dnsrecord_groups.go create mode 100644 internal/controller/dnsrecord_groups_test.go diff --git a/.gitignore b/.gitignore index 8cc99e4b..a8fb5b65 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,7 @@ Dockerfile.cross *.swp *.swo *~ +examples/* # Helm chart packages *operator*.tgz* diff --git a/Makefile b/Makefile index a68023a5..2bda5fbd 100644 --- a/Makefile +++ b/Makefile @@ -290,7 +290,7 @@ build: manifests generate fmt vet ## Build manager binary. RUN_METRICS_ADDR=":8080" RUN_HEALTH_ADDR=":8081" RUN_DELEGATION_ROLE="primary" -DEFAULT_RUN_FLAGS ?= --log-mode=development --provider inmemory,aws,google,azure,coredns,endpoint --delegation-role=${RUN_DELEGATION_ROLE} --metrics-bind-address=${RUN_METRICS_ADDR} --health-probe-bind-address=${RUN_HEALTH_ADDR} +DEFAULT_RUN_FLAGS ?= --log-mode=development --provider inmemory,aws,google,azure,coredns,endpoint --delegation-role=${RUN_DELEGATION_ROLE} --metrics-bind-address=${RUN_METRICS_ADDR} --health-probe-bind-address=${RUN_HEALTH_ADDR} --group=${GROUP} RUN_FLAGS ?= $(DEFAULT_RUN_FLAGS) .PHONY: run diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go index 05803e3e..72345ef0 100644 --- a/api/v1alpha1/conditions.go +++ b/api/v1alpha1/conditions.go @@ -18,3 +18,8 @@ const ConditionReasonUnhealthy ConditionReason = "HealthChecksFailed" const ConditionTypeReadyForDelegation ConditionType = "ReadyForDelegation" const ConditionReasonFinalizersSet ConditionReason = "FinalizersSet" + +const ConditionTypeActive ConditionType = "Active" +const ConditionReasonNotInActiveGroup ConditionReason = "NotMemberOfActiveGroup" +const ConditionReasonInActiveGroup ConditionReason = "MemberOfActiveGroup" +const ConditionReasonNoActiveGroups ConditionReason = "NoActiveGroupsSet" diff --git a/api/v1alpha1/dnsrecord_types.go b/api/v1alpha1/dnsrecord_types.go index 028204c2..ac27c659 100644 --- a/api/v1alpha1/dnsrecord_types.go +++ b/api/v1alpha1/dnsrecord_types.go @@ -180,6 +180,9 @@ type DNSRecordStatus struct { // Group displays the group which the dns-operator belongs to, if set. Group types.Group `json:"group,omitempty"` + + // ActiveGroups displays the last read list of active groups + ActiveGroups string `json:"activeGroups,omitempty"` } // GetRemoteRecordStatuses returns any remote record statuses in the current status. @@ -340,6 +343,14 @@ func (s *DNSRecord) IsDeleting() bool { return s.DeletionTimestamp != nil && !s.DeletionTimestamp.IsZero() } +// IsActive always returns true for base DNSRecord instances. +// This method is part of the DNSRecordAccessor interface and is overridden +// by GroupAdapter to provide group-aware behavior. The base implementation +// ensures that non-grouped records are always considered active. +func (s *DNSRecord) IsActive() bool { + return true +} + // ProviderAccessor impl var _ ProviderAccessor = &DNSRecord{} diff --git a/bundle/manifests/kuadrant.io_dnsrecords.yaml b/bundle/manifests/kuadrant.io_dnsrecords.yaml index a7252b8f..3bb4e836 100644 --- a/bundle/manifests/kuadrant.io_dnsrecords.yaml +++ b/bundle/manifests/kuadrant.io_dnsrecords.yaml @@ -228,6 +228,9 @@ spec: status: description: DNSRecordStatus defines the observed state of DNSRecord properties: + activeGroups: + description: ActiveGroups displays the last read list of active groups + type: string conditions: description: |- conditions are any conditions associated with the record in the dns provider. diff --git a/charts/dns-operator/templates/manifests.yaml b/charts/dns-operator/templates/manifests.yaml index c5f23dcd..5e5e1c3f 100644 --- a/charts/dns-operator/templates/manifests.yaml +++ b/charts/dns-operator/templates/manifests.yaml @@ -354,6 +354,9 @@ spec: status: description: DNSRecordStatus defines the observed state of DNSRecord properties: + activeGroups: + description: ActiveGroups displays the last read list of active groups + type: string conditions: description: |- conditions are any conditions associated with the record in the dns provider. diff --git a/cmd/main.go b/cmd/main.go index 303979e4..fa5b47eb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -264,6 +264,7 @@ func main() { ProviderFactory: providerFactory, DelegationRole: delegationRole, Group: group, + TXTResolver: &controller.DefaultTXTResolver{}, }, Client: mgr.GetClient(), } @@ -280,6 +281,7 @@ func main() { ProviderFactory: providerFactory, DelegationRole: delegationRole, Group: group, + TXTResolver: &controller.DefaultTXTResolver{}, }, RemoteClusterCollector: remoteClusterCollector, } diff --git a/cmd/plugin/get-zone-records.go b/cmd/plugin/get-zone-records.go index b8ea5f57..256614e7 100644 --- a/cmd/plugin/get-zone-records.go +++ b/cmd/plugin/get-zone-records.go @@ -83,7 +83,7 @@ func init() { } getZoneRecordsCMD.Flags().StringVar(&providerRef, "providerRef", noDefault, - fmt.Sprintf("A provider reference to the secert to use when querying. This can only be used with the type of %s. Format = '/'", host)) + fmt.Sprintf("A provider reference to the secret to use when querying. This can only be used with the type of %s. Format = '/'", host)) getZoneRecordsCMD.Flags().StringVarP(&namespace, "namespace", "n", "dns-operator-system", "namespace where resources exist") } diff --git a/cmd/plugin/secret_generation.go b/cmd/plugin/secret_generation.go index 39b20af1..7c562bd7 100644 --- a/cmd/plugin/secret_generation.go +++ b/cmd/plugin/secret_generation.go @@ -439,7 +439,7 @@ func saveSecret(log logr.Logger, secret Secret, dirPath string) (*os.File, error } func applySecretToCluser(log logr.Logger, secretFile string) error { - log.V(1).Info("Write secert to main cluster") + log.V(1).Info("Write secret to main cluster") args := []string{ "apply", "--filename", @@ -460,7 +460,7 @@ func applySecretToCluser(log logr.Logger, secretFile string) error { return errors.New("unable to write secret to cluster") } - log.Info(fmt.Sprintf("Secert %s created in namespace %s", generateSecretFlags.name, generateSecretFlags.namespace)) + log.Info(fmt.Sprintf("secret %s created in namespace %s", generateSecretFlags.name, generateSecretFlags.namespace)) return nil } diff --git a/config/coredns/Corefile b/config/coredns/Corefile index 3fbfe4f3..03e2d4bf 100644 --- a/config/coredns/Corefile +++ b/config/coredns/Corefile @@ -1,11 +1,15 @@ k.example.com { debug errors + log + + rewrite name regex kuadrant-active-groups\.(.*)k.example\.com kuadrant-active-groups-coredns.pb.hcpapps.net + forward kuadrant-active-groups-coredns.pb.hcpapps.net /etc/resolv.conf + health { lameduck 5s } ready - log geoip GeoLite2-City-demo.mmdb { edns-subnet } @@ -15,4 +19,4 @@ k.example.com { } kuadrant prometheus 0.0.0.0:9153 -} +} \ No newline at end of file diff --git a/config/crd/bases/kuadrant.io_dnsrecords.yaml b/config/crd/bases/kuadrant.io_dnsrecords.yaml index ff988e74..de3052f6 100644 --- a/config/crd/bases/kuadrant.io_dnsrecords.yaml +++ b/config/crd/bases/kuadrant.io_dnsrecords.yaml @@ -228,6 +228,9 @@ spec: status: description: DNSRecordStatus defines the observed state of DNSRecord properties: + activeGroups: + description: ActiveGroups displays the last read list of active groups + type: string conditions: description: |- conditions are any conditions associated with the record in the dns provider. diff --git a/config/local-setup/dns-provider/coredns/kustomization.yaml b/config/local-setup/dns-provider/coredns/kustomization.yaml index 8b3ec12f..c00f1f1e 100644 --- a/config/local-setup/dns-provider/coredns/kustomization.yaml +++ b/config/local-setup/dns-provider/coredns/kustomization.yaml @@ -5,6 +5,7 @@ generatorOptions: labels: app.kubernetes.io/part-of: dns-operator app.kubernetes.io/managed-by: kustomize + kuadrant.io/default-provider: "true" secretGenerator: - name: dns-provider-credentials diff --git a/coredns/examples/Corefile b/coredns/examples/Corefile index 17469cb5..03e2d4bf 100644 --- a/coredns/examples/Corefile +++ b/coredns/examples/Corefile @@ -1,11 +1,22 @@ k.example.com { - debug - errors - log - geoip GeoLite2-City-demo.mmdb - metadata - transfer { - to * - } - kuadrant -} + debug + errors + log + + rewrite name regex kuadrant-active-groups\.(.*)k.example\.com kuadrant-active-groups-coredns.pb.hcpapps.net + forward kuadrant-active-groups-coredns.pb.hcpapps.net /etc/resolv.conf + + health { + lameduck 5s + } + ready + geoip GeoLite2-City-demo.mmdb { + edns-subnet + } + metadata + transfer { + to * + } + kuadrant + prometheus 0.0.0.0:9153 +} \ No newline at end of file diff --git a/internal/controller/base_dnsrecord_reconciler.go b/internal/controller/base_dnsrecord_reconciler.go index e41f5dbd..6b5001cc 100644 --- a/internal/controller/base_dnsrecord_reconciler.go +++ b/internal/controller/base_dnsrecord_reconciler.go @@ -31,6 +31,7 @@ type BaseDNSRecordReconciler struct { ProviderFactory provider.Factory DelegationRole string Group types.Group + TXTResolver TXTResolver } func (r *BaseDNSRecordReconciler) IsPrimary() bool { @@ -106,7 +107,7 @@ func (r *BaseDNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord D if err != nil { return hadChanges, err } - logger.Info("Published DNSRecord to zone") + logger.Info("Published DNSRecord to zone", "hadChanges?", hadChanges) return hadChanges, nil } @@ -129,9 +130,11 @@ func (r *BaseDNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord DN return false, err } - recordRegistry = registry.GroupRegistry{ - Registry: recordRegistry, - Group: r.Group, + if !dnsRecord.GetDNSRecord().IsAuthoritativeRecord() { + recordRegistry = registry.GroupRegistry{ + Registry: recordRegistry, + Group: dnsRecord.GetGroup(), + } } policyID := "sync" @@ -192,7 +195,15 @@ func (r *BaseDNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord DN } func (r *BaseDNSRecordReconciler) updateStatus(ctx context.Context, client client.Client, previous, current DNSRecordAccessor, err error) (reconcile.Result, error) { - result, uErr := r.updateStatusAndRequeue(ctx, client, previous, current, 0) + _, requeueTime := recordReceivedPrematurely(current) + if !current.IsActive() { + requeueTime = InactiveGroupRequeueTime + } + if current.IsDeleting() { + requeueTime = defaultRequeueTime + } + result, uErr := r.updateStatusAndRequeue(ctx, client, previous, current, requeueTime) + if uErr != nil { err = uErr } @@ -201,12 +212,12 @@ func (r *BaseDNSRecordReconciler) updateStatus(ctx context.Context, client clien // updateStatusAndRequeue will update the status of the record if the current and previous status is different // and returns a reconcile.result that re-queues at the given time. -func (r *BaseDNSRecordReconciler) updateStatusAndRequeue(ctx context.Context, client client.Client, previous, current DNSRecordAccessor, requeueTime time.Duration) (reconcile.Result, error) { +func (r *BaseDNSRecordReconciler) updateStatusAndRequeue(ctx context.Context, c client.Client, previous, current DNSRecordAccessor, requeueTime time.Duration) (reconcile.Result, error) { logger := log.FromContext(ctx) + patch := client.MergeFrom(previous.GetDNSRecord()) // update the record after setting the status if !equality.Semantic.DeepEqual(previous.GetStatus(), current.GetStatus()) { - logger.V(1).Info("Updating status of DNSRecord") - if updateError := client.Status().Update(ctx, current.GetDNSRecord()); updateError != nil { + if updateError := c.Status().Patch(ctx, current.GetDNSRecord(), patch); updateError != nil { if apierrors.IsConflict(updateError) { return ctrl.Result{RequeueAfter: time.Second}, nil } diff --git a/internal/controller/dnsrecord_accessor.go b/internal/controller/dnsrecord_accessor.go index 0b8b2d0c..90668504 100644 --- a/internal/controller/dnsrecord_accessor.go +++ b/internal/controller/dnsrecord_accessor.go @@ -41,6 +41,8 @@ type DNSRecordAccessor interface { GetStatus() *v1alpha1.DNSRecordStatus SetStatusConditions(hadChanges bool) SetStatusCondition(conditionType string, status metav1.ConditionStatus, reason, message string) + ClearStatusCondition(conditionType string) + GetStatusCondition(conditionType string) *metav1.Condition SetStatusOwnerID(id string) SetStatusZoneID(id string) SetStatusZoneDomainName(domainName string) @@ -48,10 +50,12 @@ type DNSRecordAccessor interface { SetStatusEndpoints(endpoints []*externaldns.Endpoint) SetStatusObservedGeneration(observedGeneration int64) SetStatusGroup(types.Group) + SetStatusActiveGroups(types.Groups) HasOwnerIDAssigned() bool HasDNSZoneAssigned() bool HasProviderSecretAssigned() bool IsDeleting() bool + IsActive() bool } type DNSRecord struct { @@ -92,7 +96,14 @@ func (s *DNSRecord) GetStatus() *v1alpha1.DNSRecordStatus { func (s *DNSRecord) SetStatusConditions(_ bool) { //We do nothing here at the moment!! - return +} + +func (s *DNSRecord) GetStatusCondition(conditionType string) *metav1.Condition { + return meta.FindStatusCondition(s.GetStatus().Conditions, conditionType) +} + +func (s *DNSRecord) ClearStatusCondition(conditionType string) { + meta.RemoveStatusCondition(&s.GetStatus().Conditions, conditionType) } func (s *DNSRecord) SetStatusCondition(conditionType string, status metav1.ConditionStatus, reason, message string) { @@ -136,6 +147,10 @@ func (s *DNSRecord) SetStatusGroup(group types.Group) { s.GetStatus().Group = group } +func (s *DNSRecord) SetStatusActiveGroups(groups types.Groups) { + s.GetStatus().ActiveGroups = groups.String() +} + type RemoteDNSRecord struct { *v1alpha1.DNSRecord ClusterID string @@ -182,7 +197,16 @@ func (s *RemoteDNSRecord) GetStatus() *v1alpha1.DNSRecordStatus { func (s *RemoteDNSRecord) SetStatusConditions(_ bool) { //We do nothing here at the moment!! - return +} + +func (s *RemoteDNSRecord) GetStatusCondition(conditionType string) *metav1.Condition { + return meta.FindStatusCondition(s.Status.Conditions, conditionType) +} + +func (s *RemoteDNSRecord) ClearStatusCondition(conditionType string) { + conditions := s.GetStatus().Conditions + meta.RemoveStatusCondition(&conditions, conditionType) + s.GetStatus().Conditions = conditions } func (s *RemoteDNSRecord) SetStatusCondition(conditionType string, status metav1.ConditionStatus, reason, message string) { @@ -231,6 +255,11 @@ func (s *RemoteDNSRecord) SetStatusGroup(_ types.Group) { panic("cannot set Group on remote record") } +func (s *RemoteDNSRecord) SetStatusActiveGroups(groups types.Groups) { + s.GetStatus().ActiveGroups = groups.String() + s.setStatus() +} + func (s *RemoteDNSRecord) setStatus() { s.DNSRecord.Status.SetRemoteRecordStatus(s.ClusterID, *s.status) } diff --git a/internal/controller/dnsrecord_controller.go b/internal/controller/dnsrecord_controller.go index 36d69893..d61e8044 100644 --- a/internal/controller/dnsrecord_controller.go +++ b/internal/controller/dnsrecord_controller.go @@ -39,6 +39,7 @@ import ( "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/dns-operator/internal/common" "github.com/kuadrant/dns-operator/internal/provider" + "github.com/kuadrant/dns-operator/types" ) const ( @@ -215,8 +216,6 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return r.updateStatus(ctx, previous, dnsRecord, false, err) } - dnsRecord.SetStatusGroup(r.Group) - //Ensure an Owner ID has been assigned to the record (OwnerID set in the status) if !dnsRecord.HasOwnerIDAssigned() { if dnsRecord.GetSpec().OwnerID != "" { @@ -228,12 +227,17 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( ctx, logger = r.setLogger(ctx, baseLogger, dnsRecord) } + if !dnsRecord.GetDNSRecord().IsAuthoritativeRecord() { + dnsRecord.SetStatusGroup(r.Group) + } + if dnsRecord.IsDelegating() { // ReadyForDelegation can be set to true once: // - finalizer is added // - ownerID is set // - record is validated // - health probes created + // - group is added to status if !meta.IsStatusConditionPresentAndEqual(dnsRecord.GetStatus().Conditions, string(v1alpha1.ConditionTypeReadyForDelegation), metav1.ConditionTrue) { dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReadyForDelegation), metav1.ConditionTrue, string(v1alpha1.ConditionReasonFinalizersSet), "") return r.updateStatusAndRequeue(ctx, r.Client, previous, dnsRecord, randomizedValidationRequeue) @@ -314,6 +318,23 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( ctx, logger = r.setLogger(ctx, baseLogger, dnsRecord) } + dnsRecord = r.applyGroupAdapter(ctx, dnsRecord) + + // If this grouped record is not active, exit early (only active groups process unpublishing) + if !dnsRecord.IsActive() { + logger.V(1).Info("record is from an inactive group, exiting reconcile", + "currentGroup", dnsRecord.GetGroup(), + "activeGroups", dnsRecord.GetStatus().ActiveGroups, + "requeueIn", InactiveGroupRequeueTime) + + dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, "InInactiveGroup", "No further actions to take while in inactive group") + res, err := r.updateStatus(ctx, previous, dnsRecord, false, nil) + if err != nil { + return res, err + } + return r.updateStatusAndRequeue(ctx, r.Client, previous, dnsRecord, InactiveGroupRequeueTime) + } + // Create a dns provider for the current record, must have an owner and zone assigned or will throw an error dnsProvider, err := r.getDNSProvider(ctx, dnsRecord) if err != nil { @@ -341,9 +362,31 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return r.updateStatus(ctx, previous, dnsRecord, hadChanges, err) } + // process unpublish of inactive groups once active cluster has no changes to publish + if !hadChanges && dnsRecord.GetGroup() != "" { + err = r.unpublishInactiveGroups(ctx, r.Client, dnsRecord, dnsProvider) + if err != nil { + logger.Error(err, "Failed to unpublish inactive groups") + dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, + "ProviderError", fmt.Sprintf("The DNS provider failed to unpublish inactive groups: %v", provider.SanitizeError(err))) + } + } + return r.updateStatus(ctx, previous, dnsRecord, hadChanges, nil) } +func (r *DNSRecordReconciler) applyGroupAdapter(ctx context.Context, dnsRecord DNSRecordAccessor) DNSRecordAccessor { + if dnsRecord.GetDNSRecord().IsAuthoritativeRecord() || r.Group == "" { + return dnsRecord + } + var activeGroups types.Groups + activeGroups = r.getActiveGroups(ctx, r.Client, dnsRecord) + dnsRecord.SetStatusActiveGroups(activeGroups) + dnsRecord = newGroupAdapter(dnsRecord, activeGroups) + + return dnsRecord +} + func (r *DNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord DNSRecordAccessor, dnsProvider provider.Provider) (bool, error) { logger := log.FromContext(ctx) if prematurely, _ := recordReceivedPrematurely(dnsRecord); prematurely { @@ -535,6 +578,12 @@ func recordReceivedPrematurely(record DNSRecordAccessor) (bool, time.Duration) { if record.GetStatus().ValidFor != "" { requeueIn, _ = time.ParseDuration(record.GetStatus().ValidFor) } + + // do not consider active records premature, as they may have some unpublishing to do + if record.IsActive() && record.GetGroup() != "" { + return false, requeueIn + } + expiryTime := metav1.NewTime(record.GetStatus().QueuedAt.Add(requeueIn)) prematurely = !generationChanged(record.GetDNSRecord()) && reconcileStart.Before(&expiryTime) @@ -589,6 +638,7 @@ func exponentialRequeueTime(lastRequeueTime string) time.Duration { // setStatusConditions sets healthy and ready condition on given DNSRecord func setStatusConditions(record DNSRecordAccessor, hadChanges bool) { // we get here only when spec err is nil - can trust hadChanges bool + record.SetStatusConditions(hadChanges) readyCond := meta.FindStatusCondition(record.GetStatus().Conditions, string(v1alpha1.ConditionTypeReady)) if readyCond != nil && (readyCond.Reason == string(v1alpha1.ConditionReasonProviderEndpointsRemoved) || readyCond.Reason == string(v1alpha1.ConditionReasonProviderEndpointsDeletion)) { @@ -609,11 +659,8 @@ func setStatusConditions(record DNSRecordAccessor, hadChanges bool) { meta.RemoveStatusCondition(&record.GetStatus().Conditions, string(v1alpha1.ConditionTypeHealthy)) return } - // if we haven't published because of the health failure, we won't have changes but the spec endpoints will be empty if len(record.GetStatus().Endpoints) == 0 { record.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, string(v1alpha1.ConditionReasonUnhealthy), "Not publishing unhealthy records") } - - record.SetStatusConditions(hadChanges) } diff --git a/internal/controller/dnsrecord_controller_delegation_test.go b/internal/controller/dnsrecord_controller_delegation_test.go index 22bb4a9d..bdd1923a 100644 --- a/internal/controller/dnsrecord_controller_delegation_test.go +++ b/internal/controller/dnsrecord_controller_delegation_test.go @@ -199,7 +199,7 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -329,13 +329,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("wildcard." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("foo." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -555,7 +555,7 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -692,13 +692,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -854,13 +854,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -957,19 +957,19 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=127.0.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("cname." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=" + testHostname + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1008,13 +1008,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + secondaryDNSRecord.Status.OwnerID + ",external-dns/targets=127.0.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1053,7 +1053,7 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1324,13 +1324,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1371,13 +1371,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1505,19 +1505,19 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("cname." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=" + testHostname + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1542,19 +1542,19 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("cname." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=" + testHostname + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1597,13 +1597,13 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.1.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1631,19 +1631,19 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.1.2,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix("cname." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary2DNSRecord.Status.OwnerID + ",external-dns/targets=" + testHostname + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1687,7 +1687,7 @@ var _ = Describe("DNSRecordReconciler", func() { err := primary2K8sClient.Get(ctx, client.ObjectKeyFromObject(primary2DNSRecord), primary2DNSRecord) g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(ContainSubstring("not found"))) - }, TestTimeoutShort, time.Second).Should(Succeed()) + }, TestTimeoutMedium, time.Second).Should(Succeed()) By("verifying the primary-2 authoritative record endpoints are updated correctly") Eventually(func(g Gomega) { @@ -1704,7 +1704,7 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), @@ -1726,7 +1726,7 @@ var _ = Describe("DNSRecordReconciler", func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": HaveSuffix(testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + primary1DNSRecord.Status.OwnerID + ",external-dns/targets=127.0.0.1,external-dns/version=1\""), "RecordType": Equal("TXT"), "RecordTTL": Equal(externaldnsendpoint.TTL(0)), })), diff --git a/internal/controller/dnsrecord_groups.go b/internal/controller/dnsrecord_groups.go new file mode 100644 index 00000000..0f968f9b --- /dev/null +++ b/internal/controller/dnsrecord_groups.go @@ -0,0 +1,484 @@ +// Package controller implements DNS failover groups functionality for the DNS operator. +// +// # DNS Failover Groups Overview +// +// DNS Groups enable active-passive failover for DNS records across multiple clusters. +// Each cluster can be assigned to a named group (e.g., "primary", "secondary"), and +// only the records from the currently "active" groups are published to DNS. +// +// # How It Works +// +// 1. Group Assignment: Each DNSRecord controller is started with a group identifier +// (via --group flag or GROUP environment variable). Records managed by that +// controller inherit the group assignment. +// +// 2. Active Groups Declaration: The set of currently active groups is stored as a +// special TXT record in DNS at: kuadrant-active-groups. +// Format: "groups=group1&&group2;version=1" +// +// 3. Group Filtering: Before publishing, each controller: +// +// - Queries the active groups TXT record +// +// - Compares its own group against the active groups list +// +// - Only publishes if its group is active OR if it's ungrouped +// +// 4. Inactive Group Cleanup: When a group becomes active, it unpublishes DNS +// records from inactive groups to ensure clean failover without stale data. +// +// # Example Scenario +// +// Setup: +// - Cluster A (group="us-east") has DNSRecord pointing to 1.2.3.4 +// - Cluster B (group="us-west") has DNSRecord pointing to 5.6.7.8 +// - Cluster C (ungrouped) has DNSRecord pointing to 9.9.9.9 +// +// When active groups = ["us-east"]: +// - Published targets: 1.2.3.4, 9.9.9.9 +// - Cluster B sees it's inactive and skips publishing +// +// When active groups switch to ["us-west"]: +// - Cluster B becomes active and publishes 5.6.7.8 +// - Cluster A sees it's inactive and stops publishing +// - Cluster B also unpublishes stale 1.2.3.4 records +// - Published targets: 5.6.7.8, 9.9.9.9 +// +// # Ungrouped Records +// +// Records without a group assignment (group="") are always considered active +// and published alongside whichever groups are currently active. This allows +// for baseline traffic routing or shared infrastructure records. +// +// # Key Components +// +// - TXTResolver: Interface for looking up the active groups TXT record from DNS +// - GroupAdapter: Wraps DNSRecordAccessor to add group-aware IsActive() behavior +// - unpublishInactiveGroups(): Cleans up DNS records from inactive groups +// - getActiveGroups(): Queries DNS for the current active groups list +package controller + +import ( + "context" + "maps" + "net" + "slices" + "strings" + "time" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/external-dns/endpoint" + externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/plan" + + "github.com/kuadrant/dns-operator/api/v1alpha1" + externaldnsplan "github.com/kuadrant/dns-operator/internal/external-dns/plan" + "github.com/kuadrant/dns-operator/internal/external-dns/registry" + externaldnsregistry "github.com/kuadrant/dns-operator/internal/external-dns/registry" + "github.com/kuadrant/dns-operator/internal/provider" + "github.com/kuadrant/dns-operator/types" +) + +var ( + // InactiveGroupRequeueTime determines how frequently inactive group records + // are reconciled to check if they've become active + InactiveGroupRequeueTime = time.Second * 15 +) + +const ( + ActiveGroupsTXTRecordName = "kuadrant-active-groups" + TXTRecordKeysSeparator = ";" + TXTRecordGroupKey = "groups" +) + +// TXTResolver is an interface for resolving TXT DNS records. +// This abstraction allows for mocking in tests and custom resolution logic. +type TXTResolver interface { + // LookupTXT queries the specified host for TXT records. + // If nameservers are provided, queries are directed to those servers. + // If nameservers is empty or all fail, falls back to default DNS resolution. + LookupTXT(ctx context.Context, host string, nameservers []string) ([]string, error) +} + +// DefaultTXTResolver is the default implementation that uses net.LookupTXT. +// It supports custom nameserver configuration for querying specific DNS servers. +type DefaultTXTResolver struct{} + +// LookupTXT queries TXT records for the given host using custom nameservers if provided. +// This is used to query the active groups TXT record, which may be hosted on specific +// DNS servers (e.g., CoreDNS instances in local development). +// +// Nameserver Resolution Strategy: +// 1. Try each provided nameserver in sequence until one returns results +// 2. Automatically adds port 53 if not specified in the nameserver address +// 3. Falls back to system DNS resolver if all custom nameservers fail +// 4. Uses a 3-second timeout per nameserver to avoid hanging on unreachable servers +func (d *DefaultTXTResolver) LookupTXT(ctx context.Context, host string, nameservers []string) ([]string, error) { + logger := log.FromContext(ctx) + + logger.Info("looking up txt record", "host", host, "nameservers", nameservers) + // If nameservers are provided, try each one until we get an answer + for _, ns := range nameservers { + // Parse the nameserver to handle cases where port is already specified + nsAddr := ns + if _, _, err := net.SplitHostPort(ns); err != nil { + // No port specified, add default port 53 + nsAddr = net.JoinHostPort(ns, "53") + } + logger.V(1).Info("using nameserver", "nameserver", ns, "resolved", nsAddr) + + resolver := &net.Resolver{ + PreferGo: true, + Dial: func(ctx context.Context, network, address string) (net.Conn, error) { + dialer := net.Dialer{ + Timeout: time.Second * 3, + } + // Always use our specified nameserver, ignoring the address parameter + return dialer.DialContext(ctx, network, nsAddr) + }, + } + + records, err := resolver.LookupTXT(ctx, host) + + if err == nil && len(records) > 0 { + logger.V(1).Info("successfully resolved txt record", "nameserver", nsAddr, "records", records) + return records, nil + } + logger.V(1).Info("failed to resolve txt record", "nameserver", nsAddr, "error", err) + } + + // Fall back to default net.LookupTXT if no custom nameservers resolved. + return net.LookupTXT(host) +} + +// GroupAdapter wraps a DNSRecordAccessor to provide group-aware behavior. +// It determines whether a record should be published based on whether its +// group is in the list of currently active groups. +type GroupAdapter struct { + DNSRecordAccessor + activeGroups types.Groups +} + +// newGroupAdapter creates a new GroupAdapter wrapping the given DNSRecordAccessor. +// The activeGroups parameter should be the current list of active groups obtained +// from the active groups TXT record in DNS. +func newGroupAdapter(accessor DNSRecordAccessor, activeGroups types.Groups) *GroupAdapter { + ga := &GroupAdapter{ + DNSRecordAccessor: accessor, + activeGroups: activeGroups, + } + + return ga +} + +// IsActive determines if this record should be published based on group membership. +// Returns true if: +// - The record is ungrouped (group=""), OR +// - The record's group is in the active groups list +// +// Returns false if: +// - The record has a group assignment that is NOT in the active groups list +func (s *GroupAdapter) IsActive() bool { + //this controller is ungrouped - always active + if s.GetGroup() == "" { + return true + } + //this controllers group is active + return s.activeGroups.HasGroup(s.GetGroup()) +} + +// SetStatusConditions updates the DNSRecord status conditions including the Active condition. +// The Active condition indicates whether this record's group is currently active: +// - ConditionTrue: Record is in an active group (or ungrouped) and will be published +// - ConditionFalse: Record is in an inactive group and will not be published +// - Condition removed: Record is ungrouped (always active, no condition needed) +func (s *GroupAdapter) SetStatusConditions(hadChanges bool) { + s.DNSRecordAccessor.SetStatusConditions(hadChanges) + + if s.GetGroup() != "" { + if s.IsActive() { + s.SetStatusCondition(string(v1alpha1.ConditionTypeActive), metav1.ConditionTrue, string(v1alpha1.ConditionReasonInActiveGroup), "Group is included in active groups") + } else { + s.SetStatusCondition(string(v1alpha1.ConditionTypeActive), metav1.ConditionFalse, string(v1alpha1.ConditionReasonNotInActiveGroup), "Group is not included in active groups") + } + } else { + s.ClearStatusCondition(string(v1alpha1.ConditionTypeActive)) + } +} + +// getActiveGroups queries DNS for the current list of active groups. +// +// It looks up a special TXT record at: kuadrant-active-groups. +// The TXT record contains semicolon-separated key=value pairs, where the "groups" +// key contains double-ampersand separated group names. +// +// Example TXT record format: "groups=us-east&&us-west;version=1" +// +// This function: +// 1. Constructs the active groups hostname from the zone domain +// 2. Retrieves custom nameservers from the provider secret (if configured) +// 3. Queries those nameservers (or defaults) for the TXT record +// 4. Parses the "groups=..." entry and returns the list of active groups +// +// Returns an empty Groups list if: +// - The TXT record doesn't exist +// - The TXT record is malformed +// - DNS lookup fails +// - No "groups" key is found in the record +func (r *BaseDNSRecordReconciler) getActiveGroups(ctx context.Context, c client.Client, dnsRecord DNSRecordAccessor) types.Groups { + logger := log.FromContext(ctx).WithName("active-groups") + activeGroups := types.Groups{} + activeGroupsHost := ActiveGroupsTXTRecordName + "." + dnsRecord.GetZoneDomainName() + + nameservers, err := r.getNameserversFromProvider(ctx, c, dnsRecord) + if err != nil { + logger.Error(err, "error getting custom nameservers from provider") + return activeGroups + } + values, err := r.TXTResolver.LookupTXT(ctx, activeGroupsHost, nameservers) + if err != nil { + logger.Error(err, "error looking up active groups") + return activeGroups + } + // Parse the TXT record to extract active groups + // Expected format: "groups=group1&&group2&&group3;version=1;other=value" + // We're looking for the "groups" key and splitting on "&&" to get individual group names + if len(values) > 0 { + activeGroupsStr := strings.Join(values, "") + for _, pairStr := range strings.Split(activeGroupsStr, TXTRecordKeysSeparator) { + sections := strings.Split(pairStr, "=") + if len(sections) != 2 { + logger.V(1).Info("found badly formed data in the active groups TXT record, skipping", "pairStr", pairStr) + continue + } + if sections[0] == TXTRecordGroupKey { + for g := range strings.SplitSeq(sections[1], externaldnsplan.LabelDelimiter) { + group := types.Group(g) + if len(g) > 0 && !activeGroups.HasGroup(group) { + activeGroups = append(activeGroups, group) + } + } + } + + } + } + + logger.V(1).Info("got active groups", "groups", activeGroups) + // no answers, return empty + return activeGroups +} + +// getNameserversFromProvider extracts custom nameserver addresses from the provider secret. +// +// Provider secrets can optionally include a NAMESERVERS field containing a comma-separated +// list of DNS server addresses to use for active groups lookups. This is useful when: +// - Using CoreDNS or other custom DNS servers in development +// - DNS providers host the active groups record on specific nameservers +// - You need to bypass public DNS for the active groups query +// +// The function looks for the NAMESERVERS data key in either: +// 1. The provider secret referenced by dnsRecord.GetProviderRef(), OR +// 2. The default provider secret (labeled kuadrant.io/default-provider=true) +// +// Returns a list of nameserver addresses (e.g., ["10.96.0.10:53", "10.96.0.11"]) +// Returns an empty list if no NAMESERVERS field is found or the secret doesn't exist. +func (r *BaseDNSRecordReconciler) getNameserversFromProvider(ctx context.Context, c client.Client, dnsRecord DNSRecordAccessor) ([]string, error) { + var nameservers []string + var providerSecret v1.Secret + providerRef := dnsRecord.GetProviderRef() + + if providerRef.Name != "" { + secretKey := client.ObjectKey{ + Name: providerRef.Name, + Namespace: dnsRecord.GetNamespace(), + } + + if err := c.Get(ctx, secretKey, &providerSecret); err == nil { + nameservers = r.extractNameserversFromSecret(&providerSecret) + } else { + return nameservers, err + } + } else { + secretList := &v1.SecretList{} + err := c.List(ctx, secretList, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + v1alpha1.DefaultProviderSecretLabel: "true", + }), + Namespace: dnsRecord.GetNamespace(), + }) + + if err == nil && len(secretList.Items) == 1 { + nameservers = r.extractNameserversFromSecret(&secretList.Items[0]) + } + } + return nameservers, nil +} + +// extractNameserversFromSecret extracts and parses the NAMESERVERS field from a secret. +// The NAMESERVERS field should contain a comma-separated list of DNS server addresses. +// Each address is trimmed of whitespace and empty entries are ignored. +// +// Example secret data: +// +// NAMESERVERS: "10.96.0.10:53, 10.96.0.11:53" +// NAMESERVERS: "coredns.kube-system.svc.cluster.local" +func (r *BaseDNSRecordReconciler) extractNameserversFromSecret(secret *v1.Secret) []string { + var nameservers []string + if secret == nil { + return nameservers + } + if nameserversData, ok := secret.Data["NAMESERVERS"]; ok && len(nameserversData) > 0 { + nameserversStr := string(nameserversData) + for _, ns := range strings.Split(nameserversStr, ",") { + ns = strings.TrimSpace(ns) + if ns != "" { + nameservers = append(nameservers, ns) + } + } + } + + return nameservers +} + +// unpublishInactiveGroups removes DNS records from inactive groups in the DNS provider zone. +// +// This function is called by active group controllers after they successfully publish +// their own records. It ensures that when a group becomes active, any leftover DNS +// records from previously active groups are cleaned up to prevent stale routing. +// +// How it works: +// +// 1. Retrieves all DNS records currently in the provider zone +// 2. Reads the TXT registry entries to determine which group owns each target +// 3. For each DNS record: +// - If ALL owners are from inactive groups: DELETE the entire record +// - If SOME owners are from inactive groups: UPDATE to remove only inactive targets +// - If the record has active group or ungrouped owners: KEEP unchanged +// 4. After DNS record cleanup, deletes the TXT registry records for inactive groups +// +// Example scenario: +// +// Zone has: foo.example.com -> [1.1.1.1 (group1), 2.2.2.2 (group2), 3.3.3.3 (ungrouped)] +// Active groups: [group2] +// Result: foo.example.com -> [2.2.2.2 (group2), 3.3.3.3 (ungrouped)] +// The 1.1.1.1 target and its TXT registry entry are removed. +// +// Important notes: +// - Only runs when the current controller's group is active +// - Does not run for ungrouped or authoritative records +// - Applies changes directly to the provider (bypassing the registry to avoid conflicts) +// - TXT record cleanup happens AFTER DNS record cleanup (required for Azure) +func (r *BaseDNSRecordReconciler) unpublishInactiveGroups(ctx context.Context, c client.Client, dnsRecord DNSRecordAccessor, dnsProvider provider.Provider) error { + logger := log.FromContext(ctx).WithName("active-groups").WithName("unpublish") + managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME} + + // if this record does not have a group, do not process unpublish + if dnsRecord.GetGroup() == "" { + return nil + } + + activeGroups := r.getActiveGroups(ctx, c, dnsRecord) + + // only process unpublish when we are reconciling a record from an active group + if !dnsRecord.IsActive() || dnsRecord.GetGroup() == "" { + return nil + } + + //allZoneEndpoints = Records in the current dns provider zone + allZoneEndpoints, err := dnsProvider.Records(ctx) + if err != nil { + return err + } + registryMap := externaldnsregistry.TxtRecordsToRegistryMap(allZoneEndpoints, txtRegistryPrefix, txtRegistrySuffix, txtRegistryWildcardReplacement, []byte(txtRegistryEncryptAESKey)) + changes := &plan.Changes{} + + // work out required changes to clean out inactive groups managed DNS Records (not including TXT records) + for _, endpoint := range allZoneEndpoints { + //not a managed record type, skip it + if !slices.Contains(managedDNSRecordTypes, endpoint.RecordType) { + continue + } + + //no registry entries for this host at all, skip it + if _, ok := registryMap.Hosts[endpoint.DNSName]; !ok { + continue + } + + registryHost := registryMap.Hosts[endpoint.DNSName] + if !registryHost.HasAnyGroup(activeGroups) && len(registryHost.UngroupedOwners) == 0 { + // This host doesn't have owners in active groups nor any ungrouped owners, delete it + changes.Delete = append(changes.Delete, endpoint) + continue + } + + // active targets is the combination of all active group owners targets, plus all ungrouped owners targets + activeTargets := append(registryHost.GetGroupsTargets(activeGroups), registryHost.GetUngroupedTargets()...) + + // remove all inactive targets from endpoint + newTargets := []string{} + for _, t := range endpoint.Targets { + if slices.Contains(activeTargets, t) { + newTargets = append(newTargets, t) + } + } + + if len(newTargets) > 0 { + if !slices.Equal(newTargets, endpoint.Targets) { + // some targets were only owned from inactive groups, modify the endpoint + changes.UpdateOld = append(changes.UpdateOld, endpoint.DeepCopy()) + endpoint.Targets = newTargets + changes.UpdateNew = append(changes.UpdateNew, endpoint) + } + } else { + // no targets left for this host, delete it + changes.Delete = append(changes.Delete, endpoint) + } + } + + if changes.HasChanges() { + // changes against provider directly, as using the registry here + // would interfere with this controllers registry entries incorrectly. + err = dnsProvider.ApplyChanges(ctx, changes) + if err != nil { + logger.Error(err, "error unpublishing inactive group records") + return err + } + } + + // Clean out all TXT records that are registry entries for inactive groups, + // this is done after the previous cleanup to ensure records are deleted before + // the relevant registry entries (i.e. Azure cannot batch changes) + changes = &plan.Changes{} + for _, e := range allZoneEndpoints { + //not a TXT record type, skip it + if e.RecordType != endpoint.RecordTypeTXT { + continue + } + labels := make(map[string]string) + for _, target := range e.Targets { + var labelsFromTarget endpoint.Labels + _, _, labelsFromTarget, err = registry.NewLabelsFromString(target, []byte(txtRegistryEncryptAESKey)) + if err != nil { + logger.V(1).Info("failed to parse labels from TXT target", "target", target, "error", err) + continue + } + maps.Copy(labels, labelsFromTarget) + } + // no group, or active group, do not delete + if v, ok := labels[types.GroupLabelKey]; !ok || activeGroups.HasGroup(types.Group(v)) { + continue + } + changes.Delete = append(changes.Delete, e) + } + + if changes.HasChanges() { + err = dnsProvider.ApplyChanges(ctx, changes) + return err + } + + return nil +} diff --git a/internal/controller/dnsrecord_groups_test.go b/internal/controller/dnsrecord_groups_test.go new file mode 100644 index 00000000..3b2626e5 --- /dev/null +++ b/internal/controller/dnsrecord_groups_test.go @@ -0,0 +1,449 @@ +//go:build integration + +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/gbytes" + . "github.com/onsi/gomega/gstruct" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/types" +) + +var _ = Describe("DNSRecordReconciler with Groups", func() { + var ( + groupsCtx context.Context + groupsCancel context.CancelFunc + + // Shared mock TXT resolver for all environments + mockTXTResolver *MockTXTResolver + + // cluster 1 Group 1 + cluster1Group1Env *envtest.Environment + cluster1Group1Manager ctrl.Manager + cluster1Group1K8sClient client.Client + + // cluster 1 Group 2 + cluster1Group2Env *envtest.Environment + cluster1Group2Manager ctrl.Manager + cluster1Group2K8sClient client.Client + + // Ungrouped Primary (only primary cluster in the test setup) + ungroupedPrimaryEnv *envtest.Environment + ungroupedPrimaryManager ctrl.Manager + ungroupedPrimaryK8sClient client.Client + ) + + BeforeEach(func() { + groupsCtx, groupsCancel = context.WithCancel(ctx) + + // Create a shared mock TXT resolver for all environments + By("creating shared mock TXT resolver") + mockTXTResolver = NewMockTXTResolver() + + By("setting up cluster1 group1 environment") + cluster1Group1Env, cluster1Group1Manager = setupEnv(DelegationRoleSecondary, 1, "group1", mockTXTResolver) + cluster1Group1K8sClient = cluster1Group1Manager.GetClient() + + By("setting up cluster1 group2 environment") + cluster1Group2Env, cluster1Group2Manager = setupEnv(DelegationRoleSecondary, 3, "group2", mockTXTResolver) + cluster1Group2K8sClient = cluster1Group2Manager.GetClient() + + By("setting up ungrouped primary environment") + ungroupedPrimaryEnv, ungroupedPrimaryManager = setupEnv(DelegationRolePrimary, 5, "", mockTXTResolver) + ungroupedPrimaryK8sClient = ungroupedPrimaryManager.GetClient() + + // Start all managers + go func() { + defer GinkgoRecover() + err := cluster1Group1Manager.Start(groupsCtx) + Expect(err).ToNot(HaveOccurred()) + }() + + go func() { + defer GinkgoRecover() + err := cluster1Group2Manager.Start(groupsCtx) + Expect(err).ToNot(HaveOccurred()) + }() + + go func() { + defer GinkgoRecover() + err := ungroupedPrimaryManager.Start(groupsCtx) + Expect(err).ToNot(HaveOccurred()) + }() + + By(fmt.Sprintf("creating namespace '%s' on ungrouped primary", testDefaultClusterSecretNamespace)) + CreateNamespace(testDefaultClusterSecretNamespace, ungroupedPrimaryK8sClient) + + k8sConfigs := map[string]string{} + // Create kubeconfig users for each cluster + By("creating user 'kuadrant' in cluster1 group1") + k8sConfigs["cluster1-group1"] = string(createKuadrantUser(cluster1Group1Env)) + Expect(k8sConfigs["cluster1-group1"]).ToNot(BeEmpty()) + + By("creating user 'kuadrant' in cluster1 group2") + k8sConfigs["cluster1-group2"] = string(createKuadrantUser(cluster1Group2Env)) + Expect(k8sConfigs["cluster1-group2"]).ToNot(BeEmpty()) + + // Create cluster connection secrets on primary cluster + By("creating cluster connection secrets on ungrouped primary") + for name, secret := range k8sConfigs { + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testDefaultClusterSecretNamespace, + Labels: map[string]string{ + testDefaultClusterSecretLabel: "true", + }, + }, + StringData: map[string]string{ + "kubeconfig": secret, + }, + } + Expect(ungroupedPrimaryK8sClient.Create(ctx, secret)).To(Succeed()) + } + + }) + + AfterEach(func() { + By("tearing down the group test environments") + groupsCancel() + + if cluster1Group1Env != nil { + err := cluster1Group1Env.Stop() + Expect(err).NotTo(HaveOccurred()) + } + + if cluster1Group2Env != nil { + err := cluster1Group2Env.Stop() + Expect(err).NotTo(HaveOccurred()) + } + + if ungroupedPrimaryEnv != nil { + err := ungroupedPrimaryEnv.Stop() + Expect(err).NotTo(HaveOccurred()) + } + }) + + Describe("Groups", Labels{"groups"}, func() { + var ( + logBuffer *gbytes.Buffer + + testNamespace string + testZoneDomainName string + testHostname string + ) + + BeforeEach(func() { + logBuffer = gbytes.NewBuffer() + GinkgoWriter.TeeTo(logBuffer) + + testNamespace = generateTestNamespaceName() + testZoneDomainName = strings.Join([]string{GenerateName(), "example.com"}, ".") + testHostname = strings.Join([]string{"foo", testZoneDomainName}, ".") + + // Create test namespaces on all clusters + By(fmt.Sprintf("creating '%s' test namespace on cluster1 group1", testNamespace)) + CreateNamespace(testNamespace, cluster1Group1K8sClient) + + By(fmt.Sprintf("creating '%s' test namespace on cluster1 group2", testNamespace)) + CreateNamespace(testNamespace, cluster1Group2K8sClient) + + By(fmt.Sprintf("creating '%s' test namespace on ungrouped primary", testNamespace)) + CreateNamespace(testNamespace, ungroupedPrimaryK8sClient) + + // Create DNS provider secret on the primary cluster only + // All secondary clusters use delegation and don't need provider secrets + By(fmt.Sprintf("creating inmemory dns provider in the '%s' test namespace on ungrouped primary", testNamespace)) + createDefaultDNSProviderSecret(groupsCtx, testNamespace, testZoneDomainName, ungroupedPrimaryK8sClient) + }) + + AfterEach(func(ctx SpecContext) { + GinkgoWriter.ClearTeeWriters() + }) + + It("should publish ungrouped records when no active groups are set", Labels{"groups"}, func(ctx SpecContext) { + // Create DNS records for each cluster + + cluster1Group1DNSRecord := createDNSRecord(testHostname+"-cluster1-group1", testNamespace, testHostname, "cluster1-group1.example.com") + cluster1Group2DNSRecord := createDNSRecord(testHostname+"-cluster1-group2", testNamespace, testHostname, "cluster1-group2.example.com") + ungroupedDNSRecord := createDNSRecord(testHostname+"-ungrouped", testNamespace, testHostname, "cluster-ungrouped.example.com") + + By("creating DNSRecords on all clusters") + Expect(cluster1Group1K8sClient.Create(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Create(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Create(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("waiting for ungrouped DNSRecord to be ready (grouped records should not reach ready)") + Eventually(func(g Gomega) { + g.Expect(ungroupedPrimaryK8sClient.Get(ctx, client.ObjectKeyFromObject(ungroupedDNSRecord), ungroupedDNSRecord)).To(Succeed()) + g.Expect(ungroupedDNSRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })), + ) + }, TestTimeoutLong, time.Second).Should(Succeed()) + + By("verifying only ungrouped records are published via authoritative records on primary cluster") + Eventually(func(g Gomega) { + var targets []string + + authRecordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, authRecordList, client.InNamespace(testNamespace), client.MatchingLabels{ + v1alpha1.AuthoritativeRecordLabel: "true", + })).To(Succeed()) + + var foundCount int + for _, authRecord := range authRecordList.Items { + if authRecord.Spec.RootHost != testHostname { + continue + } + foundCount++ + for _, endpoint := range authRecord.Spec.Endpoints { + if endpoint.RecordType == "CNAME" && endpoint.DNSName == testHostname { + targets = append(targets, endpoint.Targets...) + } + } + } + g.Expect(foundCount).To(Equal(1), "Expected exactly 1 authoritative record on primary cluster") + g.Expect(targets).To(ConsistOf("cluster-ungrouped.example.com"), "Expected only ungrouped target in authoritative record") + }, TestTimeoutLong, time.Second*5).Should(Succeed()) + + By("deleting all DNSRecords") + Expect(cluster1Group1K8sClient.Delete(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Delete(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Delete(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("confirming all DNSRecords are removed from authoritative records") + Eventually(func(g Gomega) { + // Check that no authoritative records exist on primary cluster + authRecordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, authRecordList, client.InNamespace(testNamespace), client.MatchingLabels{ + v1alpha1.AuthoritativeRecordLabel: "true", + })).To(Succeed()) + + g.Expect(len(authRecordList.Items)).To(Equal(0)) + + }, TestTimeoutLong, time.Second).Should(Succeed()) + }) + + It("should publish group1 and ungrouped records when group1 is active before creation", Labels{"groups"}, func(ctx SpecContext) { + By("setting group1 as the active group before creating DNSRecords for hostnames: " + testZoneDomainName + " and " + testHostname) + // Set for both the base zone and the hostname zone since the DNSRecord might use either + setActiveGroupsInDNS(testZoneDomainName, types.Groups{types.Group("group1")}, mockTXTResolver) + setActiveGroupsInDNS(testHostname, types.Groups{types.Group("group1")}, mockTXTResolver) + + // Create DNS records for each cluster + cluster1Group1DNSRecord := createDNSRecord(testHostname+"-cluster1-group1", testNamespace, testHostname, "cluster1-group1.example.com") + cluster1Group2DNSRecord := createDNSRecord(testHostname+"-cluster1-group2", testNamespace, testHostname, "cluster1-group2.example.com") + ungroupedDNSRecord := createDNSRecord(testHostname+"-ungrouped", testNamespace, testHostname, "cluster-ungrouped.example.com") + + By("creating DNSRecords on all clusters") + Expect(cluster1Group1K8sClient.Create(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Create(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Create(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("waiting for group1 and ungrouped DNSRecords to be ready (group2 records should not reach ready)") + Eventually(func(g Gomega) { + g.Expect(cluster1Group1K8sClient.Get(ctx, client.ObjectKeyFromObject(cluster1Group1DNSRecord), cluster1Group1DNSRecord)).To(Succeed()) + g.Expect(cluster1Group1DNSRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })), + ) + + g.Expect(ungroupedPrimaryK8sClient.Get(ctx, client.ObjectKeyFromObject(ungroupedDNSRecord), ungroupedDNSRecord)).To(Succeed()) + g.Expect(ungroupedDNSRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })), + ) + + g.Expect(cluster1Group1DNSRecord.Status.Group).To(Equal(types.Group("group1"))) + g.Expect(ungroupedDNSRecord.Status.Group).To(Equal(types.Group(""))) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + By("verifying group1 and ungrouped records are published via authoritative records") + Eventually(func(g Gomega) { + var targets []string + + authRecordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, authRecordList, client.InNamespace(testNamespace), client.MatchingLabels{ + v1alpha1.AuthoritativeRecordLabel: "true", + })).To(Succeed()) + + var foundCount int + for _, authRecord := range authRecordList.Items { + if authRecord.Spec.RootHost != testHostname { + continue + } + foundCount++ + for _, endpoint := range authRecord.Spec.Endpoints { + if endpoint.RecordType == "CNAME" && endpoint.DNSName == testHostname { + targets = append(targets, endpoint.Targets...) + } + } + } + g.Expect(foundCount).To(Equal(1), "Expected exactly 1 authoritative record on primary cluster") + g.Expect(targets).To(ConsistOf("cluster1-group1.example.com", "cluster-ungrouped.example.com"), "Expected group1 and ungrouped targets in authoritative record") + }, TestTimeoutMedium, time.Second*5).Should(Succeed()) + + By("deleting all DNSRecords") + Expect(cluster1Group1K8sClient.Delete(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Delete(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Delete(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("confirming all DNSRecords are removed from authoritative records") + Eventually(func(g Gomega) { + recordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, recordList, client.InNamespace(testNamespace))).To(Succeed()) + + g.Expect(len(recordList.Items)).To(Equal(0)) + }, TestTimeoutLong*2, time.Second).Should(Succeed()) + + By("clearing active groups") + setActiveGroupsInDNS(testZoneDomainName, types.Groups{}, mockTXTResolver) + setActiveGroupsInDNS(testHostname, types.Groups{}, mockTXTResolver) + }) + + It("should publish group2 then switch to group1 with ungrouped always published", Labels{"groups"}, func(ctx SpecContext) { + By("setting group2 as the active group before creating DNSRecords") + setActiveGroupsInDNS(testZoneDomainName, types.Groups{types.Group("group2")}, mockTXTResolver) + setActiveGroupsInDNS(testHostname, types.Groups{types.Group("group2")}, mockTXTResolver) + + // Create DNS records for each cluster + cluster1Group1DNSRecord := createDNSRecord(testHostname+"-cluster1-group1", testNamespace, testHostname, "cluster1-group1.example.com") + cluster1Group2DNSRecord := createDNSRecord(testHostname+"-cluster1-group2", testNamespace, testHostname, "cluster1-group2.example.com") + ungroupedDNSRecord := createDNSRecord(testHostname+"-ungrouped", testNamespace, testHostname, "cluster-ungrouped.example.com") + + By("creating DNSRecords on all clusters") + Expect(cluster1Group1K8sClient.Create(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Create(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Create(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("waiting for group2 and ungrouped DNSRecords to be ready (group1 records should not reach ready)") + Eventually(func(g Gomega) { + g.Expect(cluster1Group2K8sClient.Get(ctx, client.ObjectKeyFromObject(cluster1Group2DNSRecord), cluster1Group2DNSRecord)).To(Succeed()) + g.Expect(cluster1Group2DNSRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })), + ) + + g.Expect(ungroupedPrimaryK8sClient.Get(ctx, client.ObjectKeyFromObject(ungroupedDNSRecord), ungroupedDNSRecord)).To(Succeed()) + g.Expect(ungroupedDNSRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })), + ) + }, TestTimeoutLong, time.Second*5).Should(Succeed()) + + By("verifying group2 and ungrouped records are published via authoritative records") + Eventually(func(g Gomega) { + var targets []string + + authRecordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, authRecordList, client.InNamespace(testNamespace), client.MatchingLabels{ + v1alpha1.AuthoritativeRecordLabel: "true", + })).To(Succeed()) + + var foundCount int + for _, authRecord := range authRecordList.Items { + if authRecord.Spec.RootHost != testHostname { + continue + } + foundCount++ + for _, endpoint := range authRecord.Spec.Endpoints { + if endpoint.RecordType == "CNAME" && endpoint.DNSName == testHostname { + targets = append(targets, endpoint.Targets...) + } + } + } + g.Expect(foundCount).To(Equal(1), "Expected exactly 1 authoritative record on primary cluster") + g.Expect(targets).To(ConsistOf("cluster1-group2.example.com", "cluster-ungrouped.example.com"), "Expected group2 and ungrouped targets in authoritative record") + }, TestTimeoutLong, time.Second*5).Should(Succeed()) + + By("switching to group1 as the active group") + setActiveGroupsInDNS(testZoneDomainName, types.Groups{types.Group("group1")}, mockTXTResolver) + setActiveGroupsInDNS(testHostname, types.Groups{types.Group("group1")}, mockTXTResolver) + + By("verifying group1 and ungrouped records are now published via authoritative records") + Eventually(func(g Gomega) { + // Collect targets from primary cluster + var targets []string + + // Check primary cluster authoritative record + authRecordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, authRecordList, client.InNamespace(testNamespace), client.MatchingLabels{ + v1alpha1.AuthoritativeRecordLabel: "true", + })).To(Succeed()) + + var foundCount int + for _, authRecord := range authRecordList.Items { + if authRecord.Spec.RootHost != testHostname { + continue + } + foundCount++ + for _, endpoint := range authRecord.Spec.Endpoints { + if endpoint.RecordType == "CNAME" && endpoint.DNSName == testHostname { + targets = append(targets, endpoint.Targets...) + } + } + } + g.Expect(foundCount).To(Equal(1), "Expected exactly 1 authoritative record on primary cluster") + g.Expect(targets).To(ConsistOf("cluster1-group1.example.com", "cluster-ungrouped.example.com"), "Expected group1 and ungrouped targets in authoritative record after switching") + }, TestTimeoutLong, time.Second*5).Should(Succeed()) + + By("deleting all DNSRecords") + Expect(cluster1Group1K8sClient.Delete(ctx, cluster1Group1DNSRecord)).To(Succeed()) + Expect(cluster1Group2K8sClient.Delete(ctx, cluster1Group2DNSRecord)).To(Succeed()) + Expect(ungroupedPrimaryK8sClient.Delete(ctx, ungroupedDNSRecord)).To(Succeed()) + + By("confirming all DNSRecords are removed from authoritative records") + Eventually(func(g Gomega) { + recordList := &v1alpha1.DNSRecordList{} + g.Expect(ungroupedPrimaryK8sClient.List(ctx, recordList, client.InNamespace(testNamespace))).To(Succeed()) + g.Expect(len(recordList.Items)).To(Equal(0)) + + }, TestTimeoutLong, time.Second).Should(Succeed()) + + By("clearing active groups") + setActiveGroupsInDNS(testZoneDomainName, types.Groups{}, mockTXTResolver) + setActiveGroupsInDNS(testHostname, types.Groups{}, mockTXTResolver) + }) + }) +}) diff --git a/internal/controller/dnsrecord_healthchecks.go b/internal/controller/dnsrecord_healthchecks.go index d45b9b49..b36b95e9 100644 --- a/internal/controller/dnsrecord_healthchecks.go +++ b/internal/controller/dnsrecord_healthchecks.go @@ -259,5 +259,4 @@ func (s *healthCheckAdapter) SetStatusConditions(hadChanges bool) { } // none of the probes is healthy s.SetStatusCondition(string(v1alpha1.ConditionTypeHealthy), metav1.ConditionFalse, string(v1alpha1.ConditionReasonUnhealthy), fmt.Sprintf("Not healthy addresses: %s", s.notHealthyProbes)) - return } diff --git a/internal/controller/helper_test.go b/internal/controller/helper_test.go index 949741a9..8ed77273 100644 --- a/internal/controller/helper_test.go +++ b/internal/controller/helper_test.go @@ -3,16 +3,25 @@ package controller import ( + "context" "crypto/rand" + "fmt" "math/big" + "strings" "time" "github.com/goombaio/namegenerator" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/pkg/builder" + "github.com/kuadrant/dns-operator/types" ) const ( @@ -20,9 +29,9 @@ const ( TestTimeoutMedium = time.Second * 15 TestTimeoutLong = time.Second * 30 TestRetryIntervalMedium = time.Millisecond * 250 - RequeueDuration = time.Second * 6 - ValidityDuration = time.Second * 3 - DefaultValidationDuration = time.Millisecond * 500 + RequeueDuration = time.Second * 2 + ValidityDuration = time.Second * 2 + DefaultValidationDuration = time.Second * 1 ) func GenerateName() string { @@ -58,6 +67,11 @@ func NewTestEndpoints(dnsName string) *TestEndpoints { } } +func (te *TestEndpoints) WithSetIdentifier(id string) *TestEndpoints { + te.endpoints[0].SetIdentifier = id + return te +} + func (te *TestEndpoints) WithTargets(targets []string) *TestEndpoints { te.endpoints[0].Targets = targets return te @@ -68,6 +82,11 @@ func (te *TestEndpoints) WithTTL(ttl externaldnsendpoint.TTL) *TestEndpoints { return te } +func (te *TestEndpoints) WithRecordType(recordType string) *TestEndpoints { + te.endpoints[0].RecordType = recordType + return te +} + func (te *TestEndpoints) Endpoints() []*externaldnsendpoint.Endpoint { return te.endpoints } @@ -84,3 +103,60 @@ func getTestHealthCheckSpec() *v1alpha1.HealthCheckSpec { }, } } + +// setActiveGroupsInDNS sets active groups via the mock TXT resolver +func setActiveGroupsInDNS(zoneDomain string, groups types.Groups, resolver *MockTXTResolver) { + activeGroupsHost := ActiveGroupsTXTRecordName + "." + zoneDomain + + // Create the active groups value + groupsStr := "" + if len(groups) > 0 { + groupNames := []string{} + for _, g := range groups { + groupNames = append(groupNames, string(g)) + } + groupsStr = "groups=" + strings.Join(groupNames, "&&") + } + + if len(groups) == 0 { + // Delete the active groups record + resolver.DeleteTXTRecord(activeGroupsHost) + fmt.Fprintf(GinkgoWriter, "DEBUG: Deleted active groups TXT record for host: %s\n", activeGroupsHost) + } else { + // Set the active groups record + resolver.SetTXTRecord(activeGroupsHost, []string{groupsStr}) + fmt.Fprintf(GinkgoWriter, "DEBUG: Set active groups TXT record - host: %s, value: %s\n", activeGroupsHost, groupsStr) + } +} + +// createDefaultDNSProviderSecret creates an inmemory DNS provider secret with default provider label +func createDefaultDNSProviderSecret(ctx context.Context, namespace, zoneDomainName string, k8sClient client.Client) *v1.Secret { + secret := builder.NewProviderBuilder("inmemory-credentials", namespace). + For(v1alpha1.SecretTypeKuadrantInmemory). + WithZonesInitialisedFor(zoneDomainName). + Build() + labels := secret.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[v1alpha1.DefaultProviderSecretLabel] = "true" + secret.SetLabels(labels) + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + return secret +} + +// createDNSRecord creates a delegated DNSRecord with weighted CNAME routing +func createDNSRecord(name, namespace, hostname, clusterTarget string) *v1alpha1.DNSRecord { + eps := NewTestEndpoints(hostname) + return &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + RootHost: hostname, + Delegate: true, + Endpoints: eps.WithRecordType("CNAME").WithTargets([]string{clusterTarget}).WithSetIdentifier("").Endpoints(), + }, + } +} diff --git a/internal/controller/remote_dnsrecord_controller.go b/internal/controller/remote_dnsrecord_controller.go index 7144390b..56c763be 100644 --- a/internal/controller/remote_dnsrecord_controller.go +++ b/internal/controller/remote_dnsrecord_controller.go @@ -143,7 +143,7 @@ func (r *RemoteDNSRecordReconciler) Reconcile(ctx context.Context, req mcreconci logger.Info("Deleting DNSRecord") if dnsRecord.GetStatus().ProviderEndpointsRemoved() { - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: time.Second}, nil } if !dnsRecord.GetStatus().ProviderEndpointsDeletion() { @@ -163,7 +163,7 @@ func (r *RemoteDNSRecordReconciler) Reconcile(ctx context.Context, req mcreconci _, err = r.deleteRecord(ctx, dnsRecord, dnsProvider) if err != nil { logger.Error(err, "Failed to delete DNSRecord") - return ctrl.Result{}, err + return ctrl.Result{RequeueAfter: time.Second}, err } } else { logger.Info("dns zone was never assigned, skipping zone cleanup") @@ -218,8 +218,20 @@ func (r *RemoteDNSRecordReconciler) Reconcile(ctx context.Context, req mcreconci return r.updateStatus(ctx, cl.GetClient(), previous, dnsRecord, err) } + activeGroups := r.getActiveGroups(ctx, r.mgr.GetLocalManager().GetClient(), dnsRecord) + if dnsRecord.GetGroup() != "" { + dnsRecord.SetStatusActiveGroups(activeGroups) + dnsRecord = newGroupAdapter(dnsRecord, activeGroups) + } + + if !dnsRecord.IsActive() { + logger.V(1).Info("remote record is from an inactive group, exiting reconcile") + _, err := r.updateStatus(ctx, r.mgr.GetLocalManager().GetClient(), previous, dnsRecord, nil) + return reconcile.Result{RequeueAfter: InactiveGroupRequeueTime}, err + } + // Publish the record - _, err = r.publishRecord(ctx, dnsRecord, dnsProvider) + hadChanges, err := r.publishRecord(ctx, dnsRecord, dnsProvider) if err != nil { logger.Error(err, "Failed to publish record") dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, @@ -229,6 +241,16 @@ func (r *RemoteDNSRecordReconciler) Reconcile(ctx context.Context, req mcreconci dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionTrue, string(v1alpha1.ConditionReasonProviderSuccess), "Provider ensured the dns record") dnsRecord.SetStatusObservedGeneration(dnsRecord.GetDNSRecord().GetGeneration()) + + // process unpublish of inactive groups once active cluster has no changes to publish + if !hadChanges && dnsRecord.IsActive() && dnsRecord.GetGroup() != "" { + err = r.unpublishInactiveGroups(ctx, r.mgr.GetLocalManager().GetClient(), dnsRecord, dnsProvider) + if err != nil { + dnsRecord.SetStatusCondition(string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, + "ProviderError", fmt.Sprintf("The DNS provider failed to unpublish inactive groups: %v", provider.SanitizeError(err))) + } + } + return r.updateStatus(ctx, cl.GetClient(), previous, dnsRecord, nil) } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index dc31e000..4b30cfc1 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -55,6 +55,7 @@ import ( ep "github.com/kuadrant/dns-operator/internal/provider/endpoint" _ "github.com/kuadrant/dns-operator/internal/provider/google" _ "github.com/kuadrant/dns-operator/internal/provider/inmemory" + kuadrantTypes "github.com/kuadrant/dns-operator/types" //+kubebuilder:scaffold:imports ) @@ -99,6 +100,50 @@ var ( cancel context.CancelFunc ) +type MockTXTResolver struct { + response []string + records map[string][]string +} + +func NewMockTXTResolver() *MockTXTResolver { + return &MockTXTResolver{ + records: make(map[string][]string), + } +} + +func (m *MockTXTResolver) SetTXTRecord(host string, values []string) { + if m.records == nil { + m.records = make(map[string][]string) + } + m.records[host] = values +} + +func (m *MockTXTResolver) DeleteTXTRecord(host string) { + if m.records != nil { + delete(m.records, host) + } +} + +func (m *MockTXTResolver) LookupTXT(ctx context.Context, host string, nameservers []string) ([]string, error) { + logger := ctrl.LoggerFrom(ctx) + + // If records map is set, use it for host-specific lookups + if m.records != nil { + if values, ok := m.records[host]; ok { + logger.V(1).Info("MockTXTResolver.LookupTXT found record", "host", host, "values", values) + return values, nil + } + availableHosts := []string{} + for h := range m.records { + availableHosts = append(availableHosts, h) + } + logger.V(1).Info("MockTXTResolver.LookupTXT no record found", "host", host, "available_hosts", availableHosts) + return []string{}, nil + } + // Fall back to legacy single response behavior for backwards compatibility + return m.response, nil +} + func TestControllers(t *testing.T) { RegisterFailHandler(Fail) @@ -108,12 +153,15 @@ func TestControllers(t *testing.T) { var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.Level(zapcore.DebugLevel))) + // Speed up inactive group requeuing for tests + InactiveGroupRequeueTime = time.Millisecond * 500 + ctx, cancel = context.WithCancel(ctrl.SetupSignalHandler()) By("bootstrapping test environment") - primaryTestEnv, primaryManager = setupEnv(DelegationRolePrimary, 1) - primary2TestEnv, primary2Manager = setupEnv(DelegationRolePrimary, 2) - secondaryTestEnv, secondaryManager = setupEnv(DelegationRoleSecondary, 1) + primaryTestEnv, primaryManager = setupEnv(DelegationRolePrimary, 1, "", &MockTXTResolver{}) + primary2TestEnv, primary2Manager = setupEnv(DelegationRolePrimary, 2, "", &MockTXTResolver{}) + secondaryTestEnv, secondaryManager = setupEnv(DelegationRoleSecondary, 1, "", &MockTXTResolver{}) primaryK8sClient = primaryManager.GetClient() primary2K8sClient = primary2Manager.GetClient() @@ -229,7 +277,7 @@ func CreateNamespace(name string, client client.Client) { // Secondary: // - create controller-runtime manager // - setup DNSRecordReconciler -func setupEnv(delegationRole string, count int) (*envtest.Environment, ctrl.Manager) { +func setupEnv(delegationRole string, count int, group string, txtResolver TXTResolver) (*envtest.Environment, ctrl.Manager) { testEnv := &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, @@ -300,6 +348,8 @@ func setupEnv(delegationRole string, count int) (*envtest.Environment, ctrl.Mana Scheme: mgr.GetScheme(), ProviderFactory: providerFactory, DelegationRole: delegationRole, + Group: kuadrantTypes.Group(group), + TXTResolver: txtResolver, }, } @@ -314,6 +364,7 @@ func setupEnv(delegationRole string, count int) (*envtest.Environment, ctrl.Mana Scheme: mgr.GetScheme(), ProviderFactory: providerFactory, DelegationRole: delegationRole, + TXTResolver: txtResolver, }, } diff --git a/internal/external-dns/registry/group.go b/internal/external-dns/registry/group.go index 5a21be21..e3a1d76d 100644 --- a/internal/external-dns/registry/group.go +++ b/internal/external-dns/registry/group.go @@ -33,15 +33,18 @@ func (g GroupRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpo } for _, ep := range eps { + if ep.Labels == nil { + ep.Labels = endpoint.NewLabels() + } + + // The targets labels is present even when there are no groups + // to ensure ungrouped targets persist through unpublishes + ep.Labels[types.TargetsLabelKey] = ep.Targets.String() + if g.Group.IsSet() { - if ep.Labels == nil { - ep.Labels = endpoint.NewLabels() - } ep.Labels[types.GroupLabelKey] = g.Group.String() - ep.Labels[types.TargetsLabelKey] = ep.Targets.String() } else { delete(ep.Labels, types.GroupLabelKey) - delete(ep.Labels, types.TargetsLabelKey) } } diff --git a/internal/external-dns/registry/txt.go b/internal/external-dns/registry/txt.go index 2093304d..6cf7b2de 100644 --- a/internal/external-dns/registry/txt.go +++ b/internal/external-dns/registry/txt.go @@ -19,6 +19,8 @@ package registry import ( "context" "errors" + "maps" + "slices" "strings" "time" @@ -29,6 +31,7 @@ import ( "sigs.k8s.io/external-dns/provider" kuadrantPlan "github.com/kuadrant/dns-operator/internal/external-dns/plan" + "github.com/kuadrant/dns-operator/types" ) const ( @@ -502,3 +505,187 @@ func NewLabelsFromString(labelText string, aesKey []byte) (owner, version string return owner, version, labels, err } + +type RegistryOwner struct { + OwnerID string + Labels map[string]string +} + +type RegistryGroup struct { + GroupID types.Group + Owners map[string]*RegistryOwner +} + +// RegistryHost represents a compilation of all metadata stored in TXT records in a DNS zone +// for a particular rootHost. When the TXT registry stores ownership and metadata information, +// it creates TXT records alongside the actual DNS records (A, CNAME, etc.). This structure +// aggregates all such TXT record metadata for a single host. +// +// The metadata is organized hierarchically: +// - Host: The root domain name (e.g., "api.example.com") +// - Groups: TXT records that contain a group label are organized into RegistryGroup objects, +// where each group contains multiple owners and their associated metadata +// - UngroupedOwners: TXT records without a group label are stored here, indexed by owner ID +// +// The various IDs here are used as keys and also available as properties, this allows more economical +// searching for particular entries, but allows the structure to continue to contain this useful data +// once extracted from the structure. +// +// This compilation is useful for: +// - Understanding which owners (DNSRecord instances) have claimed a particular host +// - Retrieving all targets and metadata associated with a host across multiple owners +// - Supporting multi-cluster delegation where multiple clusters may publish to the same host +// - Determining group membership for advanced routing strategies (Geo, Weighted) +type RegistryHost struct { + Host string + Groups map[types.Group]*RegistryGroup + UngroupedOwners map[string]*RegistryOwner +} + +type RegistryMap struct { + Hosts map[string]*RegistryHost +} + +func (m *RegistryMap) GetHosts() []string { + return slices.Collect(maps.Keys(m.Hosts)) +} + +func (h *RegistryHost) GetGroupIDs() types.Groups { + return slices.Collect(maps.Keys(h.Groups)) +} + +func (h *RegistryHost) HasAnyGroup(groups types.Groups) bool { + return slices.ContainsFunc(groups, h.HasGroup) +} + +func (h *RegistryHost) HasGroup(group types.Group) bool { + return h.GetGroupIDs().HasGroup(group) +} + +func (h *RegistryHost) GetUngroupedTargets() []string { + targets := []string{} + for _, o := range h.UngroupedOwners { + for _, t := range strings.Split(o.Labels["targets"], ",") { + if !slices.Contains(targets, t) { + targets = append(targets, t) + } + } + } + return targets +} + +func (h *RegistryHost) GetGroupsTargets(groups types.Groups) []string { + targets := []string{} + for _, g := range groups { + if group, ok := h.Groups[g]; !ok { + continue + } else { + for _, t := range group.GetTargets() { + if !slices.Contains(targets, t) { + targets = append(targets, t) + } + } + } + } + return targets +} + +func (g *RegistryGroup) GetOwnerIDs() []string { + return slices.Collect(maps.Keys(g.Owners)) +} + +func (g *RegistryGroup) GetTargets() []string { + targets := []string{} + for _, o := range g.Owners { + for _, t := range strings.Split(o.Labels["targets"], ";") { + if !slices.Contains(targets, t) { + targets = append(targets, t) + } + } + } + return targets +} + +func TxtRecordsToRegistryMap(endpoints []*endpoint.Endpoint, prefix, suffix, wildcardReplacement string, txtEncryptAESKey []byte) *RegistryMap { + registryMap := &RegistryMap{ + Hosts: make(map[string]*RegistryHost), + } + + nameMapper := newKuadrantAffixMapper(legacyMapperTemplate{ + "": { + prefix: prefix, + suffix: suffix, + wildcardReplacement: wildcardReplacement, + }, + }, prefix, wildcardReplacement) + + for _, ep := range endpoints { + if ep.RecordType != endpoint.RecordTypeTXT { + continue + } + labels := make(map[string]string) + var ownerID string + var version string + var err error + var hasValidHeritage bool + for _, target := range ep.Targets { + var labelsFromTarget endpoint.Labels + ownerID, version, labelsFromTarget, err = NewLabelsFromString(target, txtEncryptAESKey) + if err != nil { + continue + } + hasValidHeritage = true + maps.Copy(labels, labelsFromTarget) + } + + // Skip if no valid heritage was found in any target + if !hasValidHeritage { + continue + } + + // If ownerID wasn't extracted (no delimiter), get it from labels + if _, ok := labels[endpoint.OwnerLabelKey]; ok && ownerID == "" { + ownerID = labels[endpoint.OwnerLabelKey] + } else if ownerID == "" { + // couldn't find an owner ID for this record, skip it + continue + } + + // Convert TXT record name to actual endpoint name + endpointName, _ := nameMapper.ToEndpointName(ep.DNSName, version) + + // Use endpoint name as the host key (without record type prefix) + hostKey := endpointName + + if _, ok := registryMap.Hosts[hostKey]; !ok { + registryMap.Hosts[hostKey] = &RegistryHost{ + Host: endpointName, + Groups: make(map[types.Group]*RegistryGroup), + UngroupedOwners: make(map[string]*RegistryOwner), + } + } + if gID, ok := labels[types.GroupLabelKey]; ok { + groupID := types.Group(gID) + if _, ok := registryMap.Hosts[hostKey].Groups[groupID]; !ok { + registryMap.Hosts[hostKey].Groups[groupID] = &RegistryGroup{ + GroupID: groupID, + Owners: make(map[string]*RegistryOwner), + } + } + if _, ok := registryMap.Hosts[hostKey].Groups[groupID].Owners[ownerID]; !ok { + registryMap.Hosts[hostKey].Groups[groupID].Owners[ownerID] = &RegistryOwner{ + OwnerID: ownerID, + } + } + registryMap.Hosts[hostKey].Groups[groupID].Owners[ownerID].Labels = labels + } else { + if _, ok := registryMap.Hosts[hostKey].UngroupedOwners[ownerID]; !ok { + registryMap.Hosts[hostKey].UngroupedOwners[ownerID] = &RegistryOwner{ + OwnerID: ownerID, + } + } + registryMap.Hosts[hostKey].UngroupedOwners[ownerID].Labels = labels + } + } + return registryMap +} diff --git a/internal/external-dns/registry/txt_test.go b/internal/external-dns/registry/txt_test.go index 7b4939f3..a898b1e3 100644 --- a/internal/external-dns/registry/txt_test.go +++ b/internal/external-dns/registry/txt_test.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/external-dns/provider/inmemory" "github.com/kuadrant/dns-operator/internal/external-dns/testutils" + "github.com/kuadrant/dns-operator/types" ) const ( @@ -63,8 +64,6 @@ func testTXTRegistryNew(t *testing.T) { _, err = NewTXTRegistry(context.Background(), p, "txt", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil) require.Error(t, err) - _, ok := r.mapper.(NameMapper) - require.True(t, ok) assert.Equal(t, "owner", r.ownerID) assert.Equal(t, p, r.provider) @@ -80,9 +79,6 @@ func testTXTRegistryNew(t *testing.T) { r, err = NewTXTRegistry(context.Background(), p, "", "", "owner", time.Hour, "", []string{}, []string{}, true, aesKey) require.NoError(t, err) - - _, ok = r.mapper.(NameMapper) - assert.True(t, ok) } func testTXTRegistryRecords(t *testing.T) { @@ -1736,3 +1732,279 @@ func newEndpointWithResource(dnsName, recordType, resource string, targets ...st e.Labels[endpoint.ResourceLabelKey] = resource return e } + +func TestTxtRecordsToRegistryMap(t *testing.T) { + tests := []struct { + name string + endpoints []*endpoint.Endpoint + validate func(t *testing.T, result *RegistryMap) + }{ + { + name: "Empty endpoints", + endpoints: []*endpoint.Endpoint{}, + validate: func(t *testing.T, result *RegistryMap) { + assert.Empty(t, result.Hosts) + }, + }, + { + name: "No TXT records", + endpoints: []*endpoint.Endpoint{ + endpoint.NewEndpoint("foo.example.org", endpoint.RecordTypeCNAME, "target.example.org"), + endpoint.NewEndpoint("bar.example.org", endpoint.RecordTypeA, "1.2.3.4"), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.Empty(t, result.Hosts) + }, + }, + { + name: "TXT records without groups", + endpoints: []*endpoint.Endpoint{ + endpoint.NewEndpoint("txt.2tqs20a7-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner1,external-dns/version=1\""), + endpoint.NewEndpoint("txt.b1e3677c-cname-bar.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner2,external-dns/version=1\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.NotNil(t, result) + assert.Len(t, result.Hosts, 2) + + host1 := result.Hosts["foo.example.org"] + assert.Contains(t, host1.UngroupedOwners, "owner1") + assert.Empty(t, host1.Groups) + + host2 := result.Hosts["bar.example.org"] + assert.Contains(t, host2.UngroupedOwners, "owner2") + assert.Empty(t, host2.Groups) + }, + }, + { + name: "TXT records with groups", + endpoints: []*endpoint.Endpoint{ + endpoint.NewEndpoint("txt.2tqs20a7-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner1,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1\""), + endpoint.NewEndpoint("txt.b1e3677c-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner2,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1\""), + endpoint.NewEndpoint("txt.c2f4788d-cname-bar.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner3,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group2\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.NotNil(t, result) + // TXT records for same endpoint with same groupID are consolidated + assert.Len(t, result.Hosts, 2) + + // Check first host with group1 (should have 2 owners) + host1 := result.Hosts["foo.example.org"] + assert.Len(t, host1.Groups, 1) + assert.Empty(t, host1.UngroupedOwners) + + group1 := host1.Groups["group1"] + assert.Len(t, group1.Owners, 2) + assert.Contains(t, group1.Owners, "owner1") + assert.Contains(t, group1.Owners, "owner2") + + // Check second host with different group + host2 := result.Hosts["bar.example.org"] + assert.Len(t, host2.Groups, 1) + + group2 := host2.Groups["group2"] + assert.Len(t, group2.Owners, 1) + assert.Contains(t, group2.Owners, "owner3") + }, + }, + { + name: "TXT records with mixed grouped and ungrouped", + endpoints: []*endpoint.Endpoint{ + // Grouped records + endpoint.NewEndpoint("txt.2tqs20a7-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner1,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1\""), + endpoint.NewEndpoint("txt.b1e3677c-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner2,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1\""), + // Ungrouped record + endpoint.NewEndpoint("txt.c2f4788d-cname-bar.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner3,external-dns/version=1\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.NotNil(t, result) + assert.Len(t, result.Hosts, 2) + + // Check grouped hosts (consolidated into one) + host1 := result.Hosts["foo.example.org"] + assert.Len(t, host1.Groups, 1) + assert.Empty(t, host1.UngroupedOwners) + + group1 := host1.Groups["group1"] + assert.Len(t, group1.Owners, 2) + assert.Contains(t, group1.Owners, "owner1") + assert.Contains(t, group1.Owners, "owner2") + + // Check ungrouped host + host2 := result.Hosts["bar.example.org"] + assert.Empty(t, host2.Groups) + assert.Contains(t, host2.UngroupedOwners, "owner3") + }, + }, + { + name: "TXT records with additional labels", + endpoints: []*endpoint.Endpoint{ + endpoint.NewEndpoint("txt.2tqs20a7-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner1,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1,external-dns/target=us-east-1,external-dns/weight=100\""), + endpoint.NewEndpoint("txt.b1e3677c-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner2,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1,external-dns/target=us-west-2,external-dns/weight=200\""), + endpoint.NewEndpoint("txt.c2f4788d-cname-bar.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner3,external-dns/version=1,external-dns/target=eu-west-1,external-dns/priority=high\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.NotNil(t, result) + assert.Len(t, result.Hosts, 2) + + // Check grouped record with additional labels (both owners in same group) + host1 := result.Hosts["foo.example.org"] + group1 := host1.Groups["group1"] + assert.Len(t, group1.Owners, 2) + + owner1 := group1.Owners["owner1"] + assert.Equal(t, "us-east-1", owner1.Labels["target"]) + assert.Equal(t, "100", owner1.Labels["weight"]) + + owner2 := group1.Owners["owner2"] + assert.Equal(t, "us-west-2", owner2.Labels["target"]) + assert.Equal(t, "200", owner2.Labels["weight"]) + + // Check ungrouped record with labels + host2 := result.Hosts["bar.example.org"] + owner3 := host2.UngroupedOwners["owner3"] + assert.Equal(t, "eu-west-1", owner3.Labels["target"]) + assert.Equal(t, "high", owner3.Labels["priority"]) + }, + }, + { + name: "TXT records with invalid heritage", + endpoints: []*endpoint.Endpoint{ + // Valid record + endpoint.NewEndpoint("txt.2tqs20a7-cname-foo.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner1,external-dns/version=1\""), + // Invalid heritage - should be skipped + endpoint.NewEndpoint("txt.invalid.example.org", endpoint.RecordTypeTXT, + "\"some-random-text\""), + // Another valid record + endpoint.NewEndpoint("txt.b1e3677c-cname-bar.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=owner2,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=group1\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + assert.NotNil(t, result) + // Only 2 hosts should be in the map (invalid heritage is skipped) + assert.Len(t, result.Hosts, 2) + assert.Contains(t, result.Hosts, "foo.example.org") + assert.Contains(t, result.Hosts, "bar.example.org") + }, + }, + { + name: "Multiple groups and owners against a shared hostname", + endpoints: []*endpoint.Endpoint{ + // Group 1 (geo-us) with two owners (cluster1 and cluster2) for shared-host.example.org + endpoint.NewEndpoint("txt.2tqs20a7-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster1,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-us,external-dns/target=us-east-1,external-dns/geo-code=NA\""), + endpoint.NewEndpoint("txt.b1e3677c-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster2,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-us,external-dns/target=us-west-2,external-dns/geo-code=NA\""), + + // Group 2 (geo-eu) with two different owners (cluster3 and cluster4) for the same shared-host.example.org + endpoint.NewEndpoint("txt.c2f4788d-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster3,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-eu,external-dns/target=eu-west-1,external-dns/geo-code=EU\""), + endpoint.NewEndpoint("txt.d3g5899e-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster4,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-eu,external-dns/target=eu-central-1,external-dns/geo-code=EU\""), + + // Group 3 (geo-asia) with a single owner (cluster5) for the same shared-host.example.org + endpoint.NewEndpoint("txt.e4h6900f-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster5,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-asia,external-dns/target=ap-southeast-1,external-dns/geo-code=AS\""), + + // An ungrouped owner (cluster6) for the same shared-host.example.org + endpoint.NewEndpoint("txt.f5i7011g-cname-shared-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster6,external-dns/version=1,external-dns/target=us-south-1\""), + + // Different host to ensure proper separation + endpoint.NewEndpoint("txt.g6j8122h-cname-other-host.example.org", endpoint.RecordTypeTXT, + "\"heritage=external-dns,external-dns/owner=cluster7,external-dns/version=1,external-dns/"+types.GroupLabelKey+"=geo-us\""), + }, + validate: func(t *testing.T, result *RegistryMap) { + // Verify we have exactly 2 hosts + assert.Len(t, result.Hosts, 2, "Expected 2 distinct hosts") + assert.Contains(t, result.Hosts, "shared-host.example.org") + assert.Contains(t, result.Hosts, "other-host.example.org") + + // Verify shared-host.example.org structure + sharedHost := result.Hosts["shared-host.example.org"] + + // Verify it has 3 groups and 1 ungrouped owner + assert.Len(t, sharedHost.Groups, 3, "Expected 3 groups for shared-host") + assert.Len(t, sharedHost.UngroupedOwners, 1, "Expected 1 ungrouped owner for shared-host") + + // Verify Group 1 (geo-us) has 2 owners: cluster1 and cluster2 + geoUSGroup := sharedHost.Groups["geo-us"] + assert.Len(t, geoUSGroup.Owners, 2, "geo-us group should have 2 owners") + assert.Contains(t, geoUSGroup.Owners, "cluster1") + assert.Contains(t, geoUSGroup.Owners, "cluster2") + + // Verify cluster1 labels in geo-us group + cluster1 := geoUSGroup.Owners["cluster1"] + assert.Equal(t, "us-east-1", cluster1.Labels["target"]) + assert.Equal(t, "NA", cluster1.Labels["geo-code"]) + + // Verify cluster2 labels in geo-us group + cluster2 := geoUSGroup.Owners["cluster2"] + assert.Equal(t, "us-west-2", cluster2.Labels["target"]) + assert.Equal(t, "NA", cluster2.Labels["geo-code"]) + + // Verify Group 2 (geo-eu) has 2 owners: cluster3 and cluster4 + geoEUGroup := sharedHost.Groups["geo-eu"] + assert.Len(t, geoEUGroup.Owners, 2, "geo-eu group should have 2 owners") + assert.Contains(t, geoEUGroup.Owners, "cluster3") + assert.Contains(t, geoEUGroup.Owners, "cluster4") + + // Verify cluster3 labels in geo-eu group + cluster3 := geoEUGroup.Owners["cluster3"] + assert.Equal(t, "eu-west-1", cluster3.Labels["target"]) + assert.Equal(t, "EU", cluster3.Labels["geo-code"]) + + // Verify cluster4 labels in geo-eu group + cluster4 := geoEUGroup.Owners["cluster4"] + assert.Equal(t, "eu-central-1", cluster4.Labels["target"]) + assert.Equal(t, "EU", cluster4.Labels["geo-code"]) + + // Verify Group 3 (geo-asia) has 1 owner: cluster5 + geoAsiaGroup := sharedHost.Groups["geo-asia"] + assert.Len(t, geoAsiaGroup.Owners, 1, "geo-asia group should have 1 owner") + assert.Contains(t, geoAsiaGroup.Owners, "cluster5") + + // Verify cluster5 labels in geo-asia group + cluster5 := geoAsiaGroup.Owners["cluster5"] + assert.Equal(t, "ap-southeast-1", cluster5.Labels["target"]) + assert.Equal(t, "AS", cluster5.Labels["geo-code"]) + + // Verify ungrouped owner (cluster6) + assert.Contains(t, sharedHost.UngroupedOwners, "cluster6") + cluster6 := sharedHost.UngroupedOwners["cluster6"] + assert.Equal(t, "us-south-1", cluster6.Labels["target"]) + + // Verify other-host.example.org structure + otherHost := result.Hosts["other-host.example.org"] + assert.Len(t, otherHost.Groups, 1, "Expected 1 group for other-host") + assert.Len(t, otherHost.UngroupedOwners, 0, "Expected 0 ungrouped owners for other-host") + + // Verify other-host has geo-us group with cluster7 + otherGeoUSGroup := otherHost.Groups["geo-us"] + assert.Len(t, otherGeoUSGroup.Owners, 1, "other-host geo-us group should have 1 owner") + assert.Contains(t, otherGeoUSGroup.Owners, "cluster7") + + cluster7 := otherGeoUSGroup.Owners["cluster7"] + assert.Equal(t, "cluster7", cluster7.OwnerID) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := TxtRecordsToRegistryMap(tt.endpoints, "txt.", "", "", nil) + tt.validate(t, result) + }) + } +} diff --git a/internal/provider/inmemory/inmemory.go b/internal/provider/inmemory/inmemory.go index 3a6bfd10..a6c39814 100644 --- a/internal/provider/inmemory/inmemory.go +++ b/internal/provider/inmemory/inmemory.go @@ -104,6 +104,11 @@ func (i *InMemoryDNSProvider) ProviderSpecific() provider.ProviderSpecificLabels return provider.ProviderSpecificLabels{} } +// GetInMemoryClient returns the global inmemory client for testing purposes +func GetInMemoryClient() *inmemory.InMemoryClient { + return client +} + // Register this Provider with the provider factory func init() { client = inmemory.NewInMemoryClient() diff --git a/test/e2e/multi_record_test.go b/test/e2e/multi_record_test.go index 5a4356a6..ae06da39 100644 --- a/test/e2e/multi_record_test.go +++ b/test/e2e/multi_record_test.go @@ -258,11 +258,11 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { } if txtRegistryEnabled { - for _, owner := range allOwners { + for i, owner := range allOwners { expectedElementMatchers = append(expectedElementMatchers, PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(owner, 8) + "-a-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/targets=" + allTargetIps[i] + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), @@ -331,7 +331,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { PointTo(MatchFields(IgnoreExtras, Fields{ // if we are deleting record we should not have txt record for it "DNSName": Not(Equal("kuadrant-" + recordToDelete.record.Status.OwnerID + "-a-" + testHostname)), - "Targets": Not(ConsistOf("\"heritage=external-dns,external-dns/owner=" + recordToDelete.record.Status.OwnerID + ",external-dns/version=1\"")), + "Targets": Not(ConsistOf("\"heritage=external-dns,external-dns/owner=" + recordToDelete.record.Status.OwnerID + ",external-dns/targets=" + recordToDelete.config.testTargetIP + ",external-dns/version=1\"")), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), }))) @@ -689,7 +689,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Common] checking " + testHostname + " TXT endpoint for owner " + owner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(owner, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/targets=" + testRecords[0].config.hostnames.klb + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -729,7 +729,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Common] checking " + testHostname + " TXT endpoint for owner " + owner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(owner, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/targets=" + testRecords[0].config.hostnames.klb + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -763,7 +763,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Common] checking " + testHostname + " TXT endpoint for owner " + owner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(owner, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + owner + ",external-dns/targets=" + testRecords[0].config.hostnames.klb + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -773,6 +773,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { if testDNSProvider == provider.DNSProviderAWS.String() { // A CNAME record for klbHostName should exist for each geo and be owned by all endpoints in that geo klbHostName := testRecords[0].config.hostnames.klb + defaultGeoKlbHostName := testRecords[0].config.hostnames.defaultGeoKlb for geoCode, geoRecords := range testGeoRecords { geoKlbHostName := geoRecords[0].config.hostnames.geoKlb @@ -801,7 +802,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Geo] checking " + klbHostName + " -> " + geoCode + " - TXT endpoint for owner " + geoOwner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(geoOwner, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/targets=" + geoKlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(geoCode), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ @@ -814,7 +815,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Geo] checking " + klbHostName + " -> default - TXT endpoint for owner " + geoOwner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(geoOwner, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/targets=" + defaultGeoKlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal("default"), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ @@ -825,8 +826,6 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { } } - defaultGeoKlbHostName := testRecords[0].config.hostnames.defaultGeoKlb - By("[Geo] checking endpoint " + klbHostName + " -> default") Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal(klbHostName), @@ -849,11 +848,13 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { geoKlbHostName := geoRecords[0].config.hostnames.geoKlb allGeoClusterHostnames := []string{} + ownerToClusterKlb := make(map[string]string) gcpWeightProps := []externaldnsendpoint.ProviderSpecificProperty{ {Name: "routingpolicy", Value: weighted}, } for i := range geoRecords { geoClusterHostname := geoRecords[i].config.hostnames.clusterKlb + ownerToClusterKlb[geoRecords[i].record.Status.OwnerID] = geoClusterHostname allGeoClusterHostnames = append(allGeoClusterHostnames, geoClusterHostname) gcpWeightProps = append(gcpWeightProps, externaldnsendpoint.ProviderSpecificProperty{Name: geoClusterHostname, Value: "200"}) } @@ -873,7 +874,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Weight] checking " + geoKlbHostName + " TXT endpoint for owner " + geoOwner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(geoOwner, 8) + "-cname-" + geoKlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/targets=" + ownerToClusterKlb[geoOwner] + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -890,10 +891,12 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { gcpWeightProps := []externaldnsendpoint.ProviderSpecificProperty{ {Name: "routingpolicy", Value: weighted}, } + ownerToClusterKlb := make(map[string]string) for i := range geoRecords { geoClusterHostname := geoRecords[i].config.hostnames.clusterKlb allGeoClusterHostnames = append(allGeoClusterHostnames, geoClusterHostname) gcpWeightProps = append(gcpWeightProps, externaldnsendpoint.ProviderSpecificProperty{Name: geoClusterHostname, Value: "200"}) + ownerToClusterKlb[geoRecords[i].record.Status.OwnerID] = geoClusterHostname } By("[Weight] checking " + geoKlbHostName + " endpoint") @@ -911,7 +914,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Weight] checking " + geoKlbHostName + " TXT endpoint for owner " + geoOwner) Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(geoOwner, 8) + "-cname-" + geoKlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + geoOwner + ",external-dns/targets=" + ownerToClusterKlb[geoOwner] + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -942,7 +945,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Weight] checking " + geoKlbHostName + " -> " + clusterKlbHostName + " -> " + ownerID + " TXT owner endpoint") Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(ownerID, 8) + "-cname-" + geoKlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + ownerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + ownerID + ",external-dns/targets=" + clusterKlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(clusterKlbHostName), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ @@ -973,7 +976,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { By("[Cluster] checking " + clusterKlbHostName + " TXT owner endpoint") Expect(zoneEndpoints).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(ownerID, 8) + "-a-" + clusterKlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + ownerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + ownerID + ",external-dns/targets=" + clusterTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) diff --git a/test/e2e/single_record_test.go b/test/e2e/single_record_test.go index b439e918..9cbe955a 100644 --- a/test/e2e/single_record_test.go +++ b/test/e2e/single_record_test.go @@ -246,13 +246,13 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { expectedElementMatchers = append(expectedElementMatchers, PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-wildcard." + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-" + testHostname2), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP2 + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), @@ -340,7 +340,7 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { expectedElementMatchers = append(expectedElementMatchers, PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), @@ -563,25 +563,25 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-" + cluster1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + klbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + geo1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + cluster1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + geo1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), @@ -632,28 +632,28 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { Expect(zoneEndpoints).To(ContainElement( PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-" + cluster1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) Expect(zoneEndpoints).To(ContainElement( PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + klbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) Expect(zoneEndpoints).To(ContainElement( PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + geo1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + cluster1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) Expect(zoneEndpoints).To(ContainElement( PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + geo1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })))) @@ -710,19 +710,19 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-a-" + cluster1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + testTargetIP + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + testHostname), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + klbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(""), })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + geo1KlbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + cluster1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(cluster1KlbHostName), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ @@ -731,7 +731,7 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + geo1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal(geoCode), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ @@ -740,7 +740,7 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal("kuadrant-" + hash.ToBase36HashLen(dnsRecord.Status.OwnerID, 8) + "-cname-" + klbHostName), - "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/version=1\""), + "Targets": ConsistOf("\"heritage=external-dns,external-dns/owner=" + dnsRecord.Status.OwnerID + ",external-dns/targets=" + geo1KlbHostName + ",external-dns/version=1\""), "RecordType": Equal("TXT"), "SetIdentifier": Equal("default"), "ProviderSpecific": Equal(externaldnsendpoint.ProviderSpecific{ diff --git a/types/group.go b/types/group.go index 26d7ae79..9516d2fa 100644 --- a/types/group.go +++ b/types/group.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "slices" "strings" ) @@ -47,3 +48,17 @@ func (g *Group) IsSet() bool { func (g *Group) Labels() map[string]string { return map[string]string{GroupLabelKey: g.String()} } + +type Groups []Group + +func (g Groups) HasGroup(group Group) bool { + return slices.Contains(g, group) +} + +func (g Groups) String() string { + activeGroupsStr := []string{} + for _, group := range g { + activeGroupsStr = append(activeGroupsStr, string(group)) + } + return strings.Join(activeGroupsStr, ",") +}