From ebfc38a695d3083f2803e5ece4eef0bf3a70a4af Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Tue, 3 Mar 2026 17:51:41 -0600 Subject: [PATCH] feat: add activity timeline integration for DNS resources Add activity event emission for DNSZone and DNSRecordSet resources to enable activity timeline tracking in the Milo platform. This provides users with visibility into DNS resource lifecycle events. Changes: - Emit activity events for DNSRecordSet programming success/failure - Emit activity events when DNSZone is claimed by another resource - Add display-name and display-value annotations for human-friendly activity summaries (e.g., "www.example.com" instead of resource name) - Use conversational tone in activity messages - Add ActivityPolicy configurations for both resource types - Add e2e tests for display annotation functionality Co-Authored-By: Claude Opus 4.5 --- .claude/settings.json | 14 + .github/workflows/e2e.yml | 59 + .gitignore | 3 + Makefile | 6 + config/milo/activity/kustomization.yaml | 7 + .../policies/dnsrecordset-policy.yaml | 142 ++ .../activity/policies/dnszone-policy.yaml | 65 + .../milo/activity/policies/kustomization.yaml | 8 + config/milo/kustomization.yaml | 12 + docs/enhancements/activity-integration.md | 454 ++++++ .../dnsrecordset_replicator_controller.go | 94 +- ...dnsrecordset_replicator_controller_test.go | 299 ++++ .../dnszone_replicator_controller.go | 58 + internal/controller/events.go | 405 +++++ internal/controller/events_test.go | 1303 +++++++++++++++++ .../display-annotations/chainsaw-test.yaml | 239 +++ 16 files changed, 3165 insertions(+), 3 deletions(-) create mode 100644 .claude/settings.json create mode 100644 .github/workflows/e2e.yml create mode 100644 config/milo/activity/kustomization.yaml create mode 100644 config/milo/activity/policies/dnsrecordset-policy.yaml create mode 100644 config/milo/activity/policies/dnszone-policy.yaml create mode 100644 config/milo/activity/policies/kustomization.yaml create mode 100644 config/milo/kustomization.yaml create mode 100644 docs/enhancements/activity-integration.md create mode 100644 internal/controller/events.go create mode 100644 internal/controller/events_test.go create mode 100644 test/e2e/display-annotations/chainsaw-test.yaml diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..e330836 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,14 @@ +{ + "enabledPlugins": { + "datum-platform@datum-claude-code-plugins": true, + "datum-gtm@datum-claude-code-plugins": true + }, + "extraKnownMarketplaces": { + "datum-claude-code-plugins": { + "source": { + "source": "github", + "repo": "datum-cloud/claude-code-plugins" + } + } + } +} diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml new file mode 100644 index 0000000..2d3937b --- /dev/null +++ b/.github/workflows/e2e.yml @@ -0,0 +1,59 @@ +name: E2E Tests + +on: + push: + branches: + - main + pull_request: + branches: + - main + +jobs: + e2e: + name: Run Chainsaw E2E Tests + runs-on: ubuntu-latest + steps: + - name: Clone the code + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Setup Kind + uses: helm/kind-action@v1 + with: + install_only: true + + - name: Bootstrap e2e environment + run: make bootstrap-e2e + + - name: Wait for controllers to be ready + run: | + kubectl --context kind-dns-upstream -n dns-replicator-system rollout status deployment/dns-operator-controller-manager --timeout=120s + kubectl --context kind-dns-downstream -n dns-agent-system rollout status statefulset/pdns-auth --timeout=120s + + - name: Prepare kubeconfigs for Chainsaw + run: make chainsaw-prepare-kubeconfigs + + - name: Run Chainsaw tests + run: make chainsaw-test + + - name: Collect diagnostic logs + if: failure() + run: | + echo "=== Upstream controller logs ===" + kubectl --context kind-dns-upstream -n dns-replicator-system logs deployment/dns-operator-controller-manager --tail=200 || true + echo "" + echo "=== Upstream controller describe ===" + kubectl --context kind-dns-upstream -n dns-replicator-system describe deployment/dns-operator-controller-manager || true + echo "" + echo "=== Downstream agent logs ===" + kubectl --context kind-dns-downstream -n dns-agent-system logs statefulset/pdns-auth --tail=100 || true + echo "" + echo "=== DNSZones in upstream ===" + kubectl --context kind-dns-upstream get dnszones -A || true + echo "" + echo "=== DNSZones in downstream ===" + kubectl --context kind-dns-downstream get dnszones -A || true diff --git a/.gitignore b/.gitignore index 401eae5..de533aa 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,6 @@ go.work.sum # Editor/IDE # .idea/ # .vscode/ + +# Local claude settings +.claude/settings.local.json diff --git a/Makefile b/Makefile index ce6e312..68d7bf5 100644 --- a/Makefile +++ b/Makefile @@ -257,6 +257,7 @@ secret-from-file: ## Create or update a secret from a file in NAMESPACE on CONTE bootstrap-downstream: ## Create kind downstream and deploy agent with embedded PowerDNS CLUSTER=$(DOWNSTREAM_CLUSTER_NAME) $(MAKE) kind-create CLUSTER=$(DOWNSTREAM_CLUSTER_NAME) $(MAKE) kind-load-image + CONTEXT=kind-$(DOWNSTREAM_CLUSTER_NAME) $(MAKE) install-dns-operator-crds CONTEXT=kind-$(DOWNSTREAM_CLUSTER_NAME) KUSTOMIZE_DIR=config/overlays/agent-powerdns $(MAKE) kustomize-apply # Export external kubeconfig for downstream cluster (reachable from host/other containers) CLUSTER=$(DOWNSTREAM_CLUSTER_NAME) OUT=dev/kind.downstream.kubeconfig $(MAKE) export-kind-kubeconfig-raw @@ -459,6 +460,11 @@ install-networking-crds: controller-gen ## Generate and install networking servi output:crd:dir=dev/crds/network-services $(KUBECTL) --context $(CONTEXT) apply -f dev/crds/network-services +.PHONY: install-dns-operator-crds +install-dns-operator-crds: kustomize ## Install DNS operator CRDs into CONTEXT + @test -n "$(CONTEXT)" || { echo "CONTEXT is required (e.g., kind-$(DOWNSTREAM_CLUSTER_NAME))"; exit 1; } + $(KUSTOMIZE) build config/crd | $(KUBECTL) --context $(CONTEXT) apply -f - + .PHONY: install-gateway-api-crds install-gateway-api-crds: ## Install Kubernetes Gateway API CRDs into CONTEXT @test -n "$(CONTEXT)" || { echo "CONTEXT is required (e.g., kind-$(UPSTREAM_CLUSTER_NAME))"; exit 1; } diff --git a/config/milo/activity/kustomization.yaml b/config/milo/activity/kustomization.yaml new file mode 100644 index 0000000..7e24b6d --- /dev/null +++ b/config/milo/activity/kustomization.yaml @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: AGPL-3.0-only + +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: + - policies diff --git a/config/milo/activity/policies/dnsrecordset-policy.yaml b/config/milo/activity/policies/dnsrecordset-policy.yaml new file mode 100644 index 0000000..7f2ad52 --- /dev/null +++ b/config/milo/activity/policies/dnsrecordset-policy.yaml @@ -0,0 +1,142 @@ +# SPDX-License-Identifier: AGPL-3.0-only + +# ActivityPolicy for DNSRecordSet resources. +# Defines how DNSRecordSet API operations and controller events appear in activity timelines. +# +# Audit rules handle CRUD operations captured by the Kubernetes API server audit log. +# Event rules handle async controller events for programming outcomes. +# +# Design principles: +# - Use FQDNs (www.example.com) as display text, not resource names +# - Show record values (IPs, CNAME targets, MX hosts) where available +# - Action-oriented language ("added", "removed", "is now resolving to") +# - Type-specific phrasing for common record types +# - Graceful fallback to resource name when display annotations are missing +# +# Controller-provided annotations on resource (for audit rules): +# dns.networking.miloapis.com/display-name: www.example.com +# dns.networking.miloapis.com/display-value: 192.0.2.10 +# +# Controller-provided annotations on events: +# dns.networking.miloapis.com/display-name: www.example.com +# dns.networking.miloapis.com/cname-target: api.internal.example.com +# dns.networking.miloapis.com/mx-hosts: 10 mail.example.com, 20 mail2.example.com +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns.networking.miloapis.com-dnsrecordset +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSRecordSet + + # Audit log rules for CRUD operations. + # These use display-name and display-value annotations set by the controller. + # Rules are ordered from most specific to fallback. + # All rules include fallbacks for resources without display annotations. + auditRules: + # ----- CREATE RULES (type-specific with display annotations) ----- + # A/AAAA records with display annotations: "added www.example.com pointing to 192.0.2.10" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType in ['A', 'AAAA'] && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} pointing to {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # CNAME records with display annotations: "added api.example.com as an alias for api.internal.example.com" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType == 'CNAME' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} as an alias for {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # ALIAS records with display annotations: "added example.com as an alias for lb.example.com" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType == 'ALIAS' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} as an alias for {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # MX records with display annotations: "configured mail for example.com using 10 mail.example.com" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType == 'MX' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} configured mail for {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} using {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # TXT records with display annotations: "added TXT record for example.com" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType == 'TXT' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added TXT record for {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }}" + + # NS records with display annotations: "added nameservers for sub.example.com" + - match: "audit.verb == 'create' && audit.responseObject.spec.recordType == 'NS' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added nameservers for {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }}" + + # Generic create with display annotations: "added SRV record for _sip._tcp.example.com" + - match: "audit.verb == 'create' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} added {{ audit.responseObject.spec.recordType }} record for {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }}" + + # Create fallback (no display annotations): "added A record my-recordset" + - match: "audit.verb == 'create'" + summary: "{{ actor }} added {{ audit.responseObject.spec.recordType }} record {{ link(audit.objectRef.name, audit.responseObject) }}" + + # ----- DELETE RULES ----- + # Delete with display annotations available + - match: "audit.verb == 'delete' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} removed {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} ({{ audit.responseObject.spec.recordType }})" + + # Delete fallback (no display annotations): "removed A record my-recordset" + - match: "audit.verb == 'delete'" + summary: "{{ actor }} removed {{ audit.responseObject.spec.recordType }} record {{ link(audit.objectRef.name, audit.objectRef) }}" + + # ----- UPDATE RULES (type-specific with display annotations) ----- + # A/AAAA records with display annotations: "updated www.example.com to point to 192.0.2.20" + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && audit.responseObject.spec.recordType in ['A', 'AAAA'] && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} updated {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} to point to {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # CNAME records with display annotations: "updated api.example.com to alias api.v2.example.com" + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && audit.responseObject.spec.recordType == 'CNAME' && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} updated {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }} to alias {{ audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-value'] }}" + + # Generic update with display annotations + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.responseObject.metadata.annotations) && 'dns.networking.miloapis.com/display-name' in audit.responseObject.metadata.annotations" + summary: "{{ actor }} updated {{ audit.responseObject.spec.recordType }} record {{ link(audit.responseObject.metadata.annotations['dns.networking.miloapis.com/display-name'], audit.responseObject) }}" + + # Update fallback (no display annotations): "updated A record my-recordset" + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource)" + summary: "{{ actor }} updated {{ audit.responseObject.spec.recordType }} record {{ link(audit.objectRef.name, audit.responseObject) }}" + + # Status subresource updates are system-initiated and typically not shown to users + # - match: "audit.verb in ['update', 'patch'] && audit.objectRef.subresource == 'status'" + # summary: "System updated status of {{ link(audit.objectRef.name, audit.responseObject) }}" + + # Event rules for controller-emitted Kubernetes events. + # These capture async outcomes that audit logs cannot represent. + # Events always have annotations set by the controller, so no fallback needed. + # + # Available annotations on all events: + # dns.networking.miloapis.com/event-type: dns.recordset.programmed + # dns.networking.miloapis.com/domain-name: example.com + # dns.networking.miloapis.com/record-type: A + # dns.networking.miloapis.com/record-names: www + # dns.networking.miloapis.com/record-count: 1 + # dns.networking.miloapis.com/display-name: www.example.com + # + # Type-specific annotations: + # dns.networking.miloapis.com/ip-addresses: 192.0.2.10 (A/AAAA only) + # dns.networking.miloapis.com/cname-target: target.example.com (CNAME only) + # dns.networking.miloapis.com/mx-hosts: 10 mail.example.com (MX only) + eventRules: + # ----- PROGRAMMED EVENTS (type-specific, conversational tone) ----- + # A/AAAA records: "Your record is live! www.example.com resolves to 192.0.2.10" + - match: "event.reason == 'RecordSetProgrammed' && event.annotations['dns.networking.miloapis.com/record-type'] in ['A', 'AAAA']" + summary: "Your record is live! {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }} resolves to {{ event.annotations['dns.networking.miloapis.com/ip-addresses'] }}" + + # CNAME records: "Your record is live! api.example.com resolves to api.internal.example.com" + - match: "event.reason == 'RecordSetProgrammed' && event.annotations['dns.networking.miloapis.com/record-type'] == 'CNAME'" + summary: "Your record is live! {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }} resolves to {{ event.annotations['dns.networking.miloapis.com/cname-target'] }}" + + # MX records: "Mail is ready! example.com routes to 10 mail.example.com" + - match: "event.reason == 'RecordSetProgrammed' && event.annotations['dns.networking.miloapis.com/record-type'] == 'MX'" + summary: "Mail is ready! {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }} routes to {{ event.annotations['dns.networking.miloapis.com/mx-hosts'] }}" + + # TXT records: "Your TXT record is live! example.com" + - match: "event.reason == 'RecordSetProgrammed' && event.annotations['dns.networking.miloapis.com/record-type'] == 'TXT'" + summary: "Your TXT record is live! {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }}" + + # Generic programmed fallback: "Your NS record is live! sub.example.com" + - match: "event.reason == 'RecordSetProgrammed'" + summary: "Your {{ event.annotations['dns.networking.miloapis.com/record-type'] }} record is live! {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }}" + + # ----- PROGRAMMING FAILED EVENTS ----- + # "We hit a snag configuring www.example.com: Provider API error" + - match: "event.reason == 'RecordSetProgrammingFailed'" + summary: "We hit a snag configuring {{ link(event.annotations['dns.networking.miloapis.com/display-name'], event.regarding) }}: {{ event.annotations['dns.networking.miloapis.com/failure-reason'] }}" diff --git a/config/milo/activity/policies/dnszone-policy.yaml b/config/milo/activity/policies/dnszone-policy.yaml new file mode 100644 index 0000000..2860ac5 --- /dev/null +++ b/config/milo/activity/policies/dnszone-policy.yaml @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: AGPL-3.0-only + +# ActivityPolicy for DNSZone resources. +# Defines how DNSZone API operations and controller events appear in activity timelines. +# +# Audit rules handle CRUD operations captured by the Kubernetes API server audit log. +# Event rules handle async controller events for programming outcomes. +# +# Design principles: +# - Use domain names (example.com) as display text, not resource names (example-com) +# - Action-oriented language ("created zone", not "created DNS zone") +# - Show relevant DNS data (nameservers) where available +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns.networking.miloapis.com-dnszone +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSZone + + # Audit log rules for CRUD operations. + # These are automatically captured by the API server and don't require controller code. + auditRules: + # Zone creation - use domain name as display text in link + - match: "audit.verb == 'create'" + summary: "{{ actor }} created zone {{ link(audit.responseObject.spec.domainName, audit.responseObject) }}" + + # Zone deletion - use domain name from request object (response may be empty) + - match: "audit.verb == 'delete'" + summary: "{{ actor }} deleted zone {{ audit.requestObject.spec.domainName }}" + + # Zone update - use domain name as display text + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource)" + summary: "{{ actor }} updated zone {{ link(audit.responseObject.spec.domainName, audit.responseObject) }}" + + # Status subresource updates are system-initiated and typically not shown to users, + # but can be enabled if needed for debugging. + # - match: "audit.verb in ['update', 'patch'] && audit.objectRef.subresource == 'status'" + # summary: "System updated status of zone {{ link(audit.responseObject.spec.domainName, audit.responseObject) }}" + + # Event rules for controller-emitted Kubernetes events. + # These capture async outcomes that audit logs cannot represent. + # Uses conversational tone for system-initiated events. + eventRules: + # ZoneProgrammed: DNS zone successfully programmed to the DNS provider. + # Annotations available: + # dns.networking.miloapis.com/event-type: dns.zone.programmed + # dns.networking.miloapis.com/domain-name: example.com + # dns.networking.miloapis.com/zone-class: production + # dns.networking.miloapis.com/nameservers: ns1.example.com,ns2.example.com + # dns.networking.miloapis.com/resource-name: my-zone + # dns.networking.miloapis.com/resource-namespace: my-project + - match: "event.reason == 'ZoneProgrammed'" + summary: "Your zone is ready! {{ link(event.annotations['dns.networking.miloapis.com/domain-name'], event.regarding) }} is live with nameservers {{ event.annotations['dns.networking.miloapis.com/nameservers'] }}" + + # ZoneProgrammingFailed: DNS zone programming failed after previous success. + # Annotations available: + # dns.networking.miloapis.com/event-type: dns.zone.programming_failed + # dns.networking.miloapis.com/domain-name: example.com + # dns.networking.miloapis.com/failure-reason: Provider API error + # dns.networking.miloapis.com/resource-name: my-zone + # dns.networking.miloapis.com/resource-namespace: my-project + - match: "event.reason == 'ZoneProgrammingFailed'" + summary: "We hit a snag configuring {{ link(event.annotations['dns.networking.miloapis.com/domain-name'], event.regarding) }}: {{ event.annotations['dns.networking.miloapis.com/failure-reason'] }}" diff --git a/config/milo/activity/policies/kustomization.yaml b/config/milo/activity/policies/kustomization.yaml new file mode 100644 index 0000000..ddadab9 --- /dev/null +++ b/config/milo/activity/policies/kustomization.yaml @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: AGPL-3.0-only + +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: + - dnszone-policy.yaml + - dnsrecordset-policy.yaml diff --git a/config/milo/kustomization.yaml b/config/milo/kustomization.yaml new file mode 100644 index 0000000..4d05c0e --- /dev/null +++ b/config/milo/kustomization.yaml @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: AGPL-3.0-only + +# This kustomization program is used to create all of the Milo platform resources +# including ActivityPolicy configurations for activity timelines. +# +# This is created as a component so it can be included with other +# kustomizations in project control planes. +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +resources: + - activity/ diff --git a/docs/enhancements/activity-integration.md b/docs/enhancements/activity-integration.md new file mode 100644 index 0000000..fae883e --- /dev/null +++ b/docs/enhancements/activity-integration.md @@ -0,0 +1,454 @@ +# Enhancement: Activity Service Integration + +**Status**: Proposed +**Author**: Engineering +**Created**: 2026-02-12 + +## Summary + +Integrate the Activity Service with the DNS Operator to provide human-readable activity timelines for consumers and service providers. + +## Motivation + +Currently, users and support staff must inspect Kubernetes resources directly to understand what's happening with their DNS infrastructure. This requires technical knowledge and doesn't provide a clear timeline of events. + +By integrating with the Activity Service, we provide a transparent, unified view of all DNS activity that both consumers and service providers can use to understand system behavior. + +## Goals + +- **Transparency**: Consumers and service providers see the same activity timeline +- **Clarity**: Non-technical users can understand what's happening to their DNS resources +- **Visibility**: All meaningful state transitions are captured and surfaced +- **No internal details exposed**: Backend technology and cluster topology remain hidden + +## Non-Goals + +- Real-time alerting (handled by separate monitoring systems) +- Replacing Kubernetes Events (activities are derived from events, not replacing them) + +--- + +## Proposed Activity Timeline + +Both consumers and service providers see the same activity timeline, providing full transparency into what's happening with DNS infrastructure. + +Activities should use human-friendly display names (e.g., "example.com" not "example-com") and include relevant DNS information like IP addresses and record values. + +### Example Timeline + +| Timestamp | Activity | +|-----------|----------| +| 10:00:00 | user@example.com created zone example.com | +| 10:00:01 | Zone example.com is waiting for dependencies | +| 10:00:02 | Zone example.com is now active | +| 10:00:05 | Zone example.com is ready with default SOA and NS records | +| 10:01:00 | user@example.com added www.example.com pointing to 192.0.2.10 | +| 10:01:02 | www.example.com is now resolving to 192.0.2.10 | +| 10:02:00 | user@example.com added api.example.com as an alias for api.internal.example.com | +| 10:02:02 | api.example.com is now resolving to api.internal.example.com | +| 10:03:00 | user@example.com configured mail for example.com to use mail.example.com and mail2.example.com | +| 10:04:00 | user@example.com added www.example.com pointing to 192.0.2.20 | +| 10:04:01 | www.example.com pointing to 192.0.2.20 won't take effect because another record already controls this name | +| 10:05:00 | user@example.com removed www.example.com pointing to 192.0.2.10 | +| 10:05:01 | www.example.com is now resolving to 192.0.2.20 | + +### Error Scenario + +| Timestamp | Activity | +|-----------|----------| +| 10:00:00 | user@example.com added www.example.com pointing to 192.0.2.10 | +| 10:00:01 | www.example.com is waiting for zone to be ready | +| 10:00:05 | Failed to apply www.example.com | +| 10:00:10 | www.example.com is now resolving to 192.0.2.10 | + +### Activity Categories + +**Zone Lifecycle:** +- Zone created / updated / deleted +- Zone is now active +- Zone is ready with default records +- Zone conflicts with existing zone +- Zone is waiting for dependencies + +**Record Lifecycle:** +- Record added / updated / removed +- Record is now resolving +- Record won't take effect (another record controls this name) +- Failed to apply record +- Record is waiting for zone + +**Discovery:** +- Discovery started +- Discovery completed +- Discovery is waiting for zone + +--- + +## Design Details + +### Overview + +The Activity Service translates audit logs and Kubernetes events into human-readable activities using CEL-based `ActivityPolicy` resources. We will: + +1. Define ActivityPolicy resources for each DNS resource type +2. Update controllers to emit Kubernetes Events for status transitions +3. Deploy policies alongside the DNS operator + +### Data Sources + +Activities are generated from two sources: + +| Source | Use Case | Available Data | +|--------|----------|----------------| +| **Audit Logs** | User actions (create, update, delete) | Full resource spec, user info, response status | +| **Kubernetes Events** | System state changes (Accepted, Programmed, errors) | Event reason, message, object reference (name only) | + +### ActivityPolicy Resources + +#### DNSZone Policy + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns-dnszone +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSZone + + # Audit rules have access to full resource - use spec.domainName for display + auditRules: + - match: "audit.verb == 'create' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} created zone {{ link(audit.responseObject.spec.domainName, audit.responseObject) }}" + + - match: "audit.verb == 'delete' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} deleted zone {{ audit.requestObject.spec.domainName }}" + + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource)" + summary: "{{ actor }} updated zone {{ link(audit.responseObject.spec.domainName, audit.responseObject) }}" + + # Event rules - use annotations for structured data + # Annotation: dns.datumapis.com/domain-name + eventRules: + - match: "event.reason == 'Accepted'" + summary: "Zone {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }} is now active" + + - match: "event.reason == 'Programmed'" + summary: "Zone {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }} is ready with default SOA and NS records" + + - match: "event.reason == 'DNSZoneInUse'" + summary: "Zone {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }} conflicts with an existing zone" + + - match: "event.reason == 'Pending'" + summary: "Zone {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }} is waiting for dependencies" +``` + +#### DNSRecordSet Policy + +Since audit logs contain the full resource, we can access record details directly via CEL. However, constructing human-friendly strings for multiple records is complex in CEL. + +**Recommended approach:** Add a `dns.datumapis.com/display-summary` annotation to DNSRecordSet resources that controllers populate with a human-friendly description (e.g., "www.example.com pointing to 192.0.2.10"). This keeps formatting logic in Go where it's easier to handle. + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns-dnsrecordset +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSRecordSet + + auditRules: + # Use the display-summary annotation for human-friendly output + - match: "audit.verb == 'create' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} added {{ audit.responseObject.metadata.annotations['dns.datumapis.com/display-summary'] }}" + + - match: "audit.verb == 'delete' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} removed {{ audit.requestObject.metadata.annotations['dns.datumapis.com/display-summary'] }}" + + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource)" + summary: "{{ actor }} updated {{ audit.responseObject.metadata.annotations['dns.datumapis.com/display-summary'] }}" + + # Event rules - use annotations for structured data + # Annotations: dns.datumapis.com/fqdn, dns.datumapis.com/value, dns.datumapis.com/record-type + eventRules: + - match: "event.reason == 'Programmed'" + summary: "{{ event.metadata.annotations['dns.datumapis.com/fqdn'] }} is now resolving to {{ event.metadata.annotations['dns.datumapis.com/value'] }}" + + - match: "event.reason == 'NotOwner'" + summary: "{{ event.metadata.annotations['dns.datumapis.com/fqdn'] }} pointing to {{ event.metadata.annotations['dns.datumapis.com/value'] }} won't take effect because another record already controls this name" + + - match: "event.reason == 'BackendError'" + summary: "Failed to apply {{ event.metadata.annotations['dns.datumapis.com/fqdn'] }}" + + - match: "event.reason == 'Pending'" + summary: "{{ event.metadata.annotations['dns.datumapis.com/fqdn'] }} is waiting for zone to be ready" +``` + +**Controller responsibility:** The DNS operator must set the `dns.datumapis.com/display-summary` annotation on DNSRecordSet resources with a human-readable description: + +```go +// Example annotation values by record type: +// A: "www.example.com pointing to 192.0.2.10" +// AAAA: "www.example.com pointing to 2001:db8::1" +// CNAME: "api.example.com as an alias for api.internal.example.com" +// MX: "mail for example.com using mail.example.com, mail2.example.com" +// TXT: "TXT record for example.com" +``` + +#### DNSZoneClass Policy + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns-dnszoneclass +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSZoneClass + + auditRules: + - match: "audit.verb == 'create' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} created DNSZoneClass {{ link(audit.responseObject.metadata.name, audit.responseObject) }}" + + - match: "audit.verb == 'delete' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} deleted DNSZoneClass {{ audit.objectRef.name }}" + + - match: "audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource)" + summary: "{{ actor }} updated DNSZoneClass {{ link(audit.responseObject.metadata.name, audit.responseObject) }}" +``` + +#### DNSZoneDiscovery Policy + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityPolicy +metadata: + name: dns-dnszonediscovery +spec: + resource: + apiGroup: dns.networking.miloapis.com + kind: DNSZoneDiscovery + + auditRules: + # dnsZoneRef.name is the resource name, but event.message will have the domain name + - match: "audit.verb == 'create' && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + summary: "{{ actor }} started discovering existing records" + + # Annotation: dns.datumapis.com/domain-name + eventRules: + - match: "event.reason == 'Discovered'" + summary: "Finished discovering existing records for {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }}" + + - match: "event.reason == 'Pending'" + summary: "Discovery for {{ event.metadata.annotations['dns.datumapis.com/domain-name'] }} is waiting for zone to be ready" +``` + +### Controller Changes + +Controllers must: +1. Set a `dns.datumapis.com/display-summary` annotation on DNSRecordSet with a human-friendly description +2. Emit Kubernetes Events with human-friendly messages for status transitions + +**Why annotations?** The Activity Service's CEL templates can access annotations from audit logs. This lets us format complex record information in Go code rather than CEL. + +**Why event messages?** The `event.regarding` field only contains an ObjectReference (name, namespace, kind) - not the full resource. The `event.message` field carries the human-friendly context. + +**Files to modify:** +- `internal/controller/dnszone_replicator_controller.go` +- `internal/controller/dnsrecordset_replicator_controller.go` +- `internal/controller/dnsrecordset_powerdns_controller.go` +- `internal/controller/dnszonediscovery_controller.go` + +**1. Display summary annotation for DNSRecordSet:** + +```go +// Set during reconciliation, before any status updates +func buildDisplaySummary(rs *v1alpha1.DNSRecordSet, zoneDomainName string) string { + record := rs.Spec.Records[0] // Primary record + fqdn := buildFQDN(record.Name, zoneDomainName) + + switch rs.Spec.RecordType { + case "A", "AAAA": + return fmt.Sprintf("%s pointing to %s", fqdn, record.A.Content) + case "CNAME": + return fmt.Sprintf("%s as an alias for %s", fqdn, record.CNAME.Content) + case "MX": + return fmt.Sprintf("mail for %s", fqdn) + default: + return fmt.Sprintf("%s record for %s", rs.Spec.RecordType, fqdn) + } +} + +// Apply to resource +rs.Annotations["dns.datumapis.com/display-summary"] = buildDisplaySummary(rs, zone.Spec.DomainName) +``` + +**2. Events with annotations:** + +Kubernetes Events have ObjectMeta, so we can set annotations on the Event itself. This allows templates to access structured data via `event.metadata.annotations['...']`. + +```go +// Helper to emit events with annotations +func (r *Reconciler) emitEvent(obj runtime.Object, eventType, reason, message string, annotations map[string]string) { + event := &eventsv1.Event{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: obj.GetName() + "-", + Namespace: obj.GetNamespace(), + Annotations: annotations, + }, + Regarding: corev1.ObjectReference{...}, + Reason: reason, + Note: message, + Type: eventType, + EventTime: metav1.NowMicro(), + ReportingController: "dns-operator", + ReportingInstance: r.podName, + Action: reason, + } + r.EventClient.Create(ctx, event) +} + +// Zone events +r.emitEvent(zone, corev1.EventTypeNormal, "Accepted", "Zone is now active", map[string]string{ + "dns.datumapis.com/domain-name": zone.Spec.DomainName, +}) +r.emitEvent(zone, corev1.EventTypeNormal, "Programmed", "Default records created", map[string]string{ + "dns.datumapis.com/domain-name": zone.Spec.DomainName, +}) +r.emitEvent(zone, corev1.EventTypeWarning, "DNSZoneInUse", "Zone conflicts", map[string]string{ + "dns.datumapis.com/domain-name": zone.Spec.DomainName, +}) + +// RecordSet events - include structured data for template flexibility +r.emitEvent(rs, corev1.EventTypeNormal, "Programmed", "Record is now resolving", map[string]string{ + "dns.datumapis.com/fqdn": fqdn, // "www.example.com" + "dns.datumapis.com/record-type": string(rs.Spec.RecordType), // "A" + "dns.datumapis.com/value": recordValue, // "192.0.2.10" + "dns.datumapis.com/zone": zone.Spec.DomainName, // "example.com" +}) + +// Discovery events +r.emitEvent(disc, corev1.EventTypeNormal, "Discovered", "Discovery complete", map[string]string{ + "dns.datumapis.com/domain-name": zone.Spec.DomainName, +}) +``` + +This allows templates to construct summaries flexibly: + +```yaml +eventRules: + - match: "event.reason == 'Programmed'" + summary: "{{ event.metadata.annotations['dns.datumapis.com/fqdn'] }} is now resolving to {{ event.metadata.annotations['dns.datumapis.com/value'] }}" +``` + +### Directory Structure + +``` +config/milo/ + kustomization.yaml # Component for control plane installation + activity/ + kustomization.yaml + policies/ + kustomization.yaml + dnszone-policy.yaml + dnsrecordset-policy.yaml +``` + +### Kustomize Integration + +The `config/milo` directory is a Kustomize component that can be installed into project control planes. Include it in your control plane kustomization: + +```yaml +components: + - ../milo +``` + +--- + +## Query Examples + +### Recent zone activities + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityQuery +metadata: + name: my-zone-activities +spec: + startTime: "now-24h" + resourceKind: DNSZone + filter: "spec.resource.name == 'example-com'" + limit: 50 +``` + +### All DNS record errors + +```yaml +apiVersion: activity.miloapis.com/v1alpha1 +kind: ActivityQuery +metadata: + name: dns-errors +spec: + startTime: "now-1h" + resourceKind: DNSRecordSet + filter: "spec.summary.contains('Failed')" + limit: 50 +``` + +--- + +## Implementation Plan + +### Phase 1: ActivityPolicy Resources +1. Create `config/milo/activity/` directory structure +2. Implement ActivityPolicy YAML files for each resource type +3. Add Kustomize integration +4. Test policies using PolicyPreview API + +### Phase 2: Controller Event Recording +1. Add event recording to zone replicator controller +2. Add event recording to recordset replicator controller +3. Add event recording to recordset backend controller +4. Add event recording to discovery controller + +### Phase 3: Validation +1. Deploy to test cluster +2. Create DNS resources and verify activities appear +3. Test error scenarios (conflicts, backend errors) +4. Verify activity summaries are clear and actionable + +--- + +## Alternatives Considered + +### Option A: Custom Activity Generation in Controllers + +Rather than using audit logs and events, controllers could directly create Activity resources. + +**Pros:** More control over activity content +**Cons:** Duplicates Activity Service functionality, harder to maintain, misses API-level changes + +### Option B: Webhooks + +Use admission webhooks to intercept changes and create activities. + +**Pros:** Real-time, guaranteed delivery +**Cons:** Adds latency to API calls, single point of failure + +The proposed approach (ActivityPolicy + Events) was chosen because: +- Decouples activity generation from DNS controller logic +- Leverages existing Activity Service infrastructure +- Policy-based rules are easier to update without code changes +- Works with audit logs to capture all API changes + +--- + +## Open Questions + +1. How should we handle bulk operations (e.g., many records updated at once)? +2. Should activities include namespace information for multi-tenant visibility? +3. How should we format multiple record values (e.g., multiple A records for the same name)? diff --git a/internal/controller/dnsrecordset_replicator_controller.go b/internal/controller/dnsrecordset_replicator_controller.go index d62182d..9fd73d4 100644 --- a/internal/controller/dnsrecordset_replicator_controller.go +++ b/internal/controller/dnsrecordset_replicator_controller.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -52,6 +53,10 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. return ctrl.Result{}, client.IgnoreNotFound(err) } + // Obtain a per-cluster event recorder so events are written to the upstream + // (user-facing) cluster API server, not just the deployment cluster. + recorder := upstreamCluster.GetEventRecorderFor("dns-operator") + strategy := downstreamclient.NewMappedNamespaceResourceStrategy(req.ClusterName, upstreamCluster.GetClient(), r.DownstreamClient) // Ensure upstream finalizer (non-deletion path; replaces webhook defaulter) @@ -82,7 +87,9 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. return ctrl.Result{}, nil } - // Gate on referenced DNSZone early and update status when missing + // Gate on referenced DNSZone early and update status when missing. + // We fetch the zone before emitting the spec-update event so we can include + // the domain name annotation. var zone dnsv1alpha1.DNSZone if err := upstreamCluster.GetClient().Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: upstream.Spec.DNSZoneRef.Name}, &zone); err != nil { if apierrors.IsNotFound(err) { @@ -112,7 +119,9 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. return ctrl.Result{}, nil } - // Ensure OwnerReference to upstream DNSZone (same ns) + // Ensure OwnerReference to upstream DNSZone (same ns). + // We return early after setting OwnerReference to ensure we reconcile with + // the fresh object that has the updated owner metadata. if !metav1.IsControlledBy(&upstream, &zone) { base := upstream.DeepCopy() if err := controllerutil.SetControllerReference(&zone, &upstream, upstreamCluster.GetScheme()); err != nil { @@ -121,9 +130,21 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. if err := upstreamCluster.GetClient().Patch(ctx, &upstream, client.MergeFrom(base)); err != nil { return ctrl.Result{}, err } - // ensure we continue with the updated object + // Return to reconcile with updated owner metadata return ctrl.Result{}, nil } + + // Ensure display annotations are set for ActivityPolicy templates. + // Unlike OwnerReference, annotation changes don't require a fresh reconcile, + // so we patch and continue with downstream processing. + // Take base copy BEFORE modifying, so the patch captures the changes. + base := upstream.DeepCopy() + if ensureDisplayAnnotations(&upstream, zone.Spec.DomainName) { + if err := upstreamCluster.GetClient().Patch(ctx, &upstream, client.MergeFrom(base)); err != nil { + return ctrl.Result{}, err + } + // Continue with downstream processing - don't return early + } // Ensure the downstream recordset object mirrors the upstream spec if _, err = r.ensureDownstreamRecordSet(ctx, strategy, &upstream); err != nil { return ctrl.Result{}, err @@ -139,6 +160,15 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. return ctrl.Result{}, err } + // Capture the Programmed condition before the status update so we can detect + // a transition after the update completes. + prevProgrammed := apimeta.FindStatusCondition(upstream.Status.Conditions, CondProgrammed) + var prevProgrammedCopy *metav1.Condition + if prevProgrammed != nil { + c := *prevProgrammed + prevProgrammedCopy = &c + } + // Update upstream status by mirroring downstream when present if err := r.updateStatus(ctx, upstreamCluster.GetClient(), &upstream, shadow.Status.DeepCopy()); err != nil { if !apierrors.IsNotFound(err) { // tolerate races @@ -146,6 +176,40 @@ func (r *DNSRecordSetReplicator) Reconcile(ctx context.Context, req mcreconcile. } } + // Emit Programmed/ProgrammingFailed events on condition transitions. + // updateStatus mutates upstream.Status in-memory, so the in-memory state + // already reflects the new condition after the call. + currProgrammed := apimeta.FindStatusCondition(upstream.Status.Conditions, CondProgrammed) + if programmedConditionTransitioned(prevProgrammedCopy, currProgrammed) { + if currProgrammed.Status == metav1.ConditionTrue { + recordRecordSetActivityEventWithData(recorder, + RecordSetEventData{ + RecordSet: &upstream, + DomainName: zone.Spec.DomainName, + }, + corev1.EventTypeNormal, + EventReasonRecordSetProgrammed, + ActivityTypeRecordSetProgrammed, + "DNSRecordSet %q successfully programmed to DNS provider", + upstream.Name, + ) + } else if currProgrammed.Status == metav1.ConditionFalse && + prevProgrammedCopy != nil && prevProgrammedCopy.Status == metav1.ConditionTrue { + recordRecordSetActivityEventWithData(recorder, + RecordSetEventData{ + RecordSet: &upstream, + DomainName: zone.Spec.DomainName, + FailReason: currProgrammed.Message, + }, + corev1.EventTypeWarning, + EventReasonRecordSetProgrammingFailed, + ActivityTypeRecordSetProgrammingFailed, + "DNSRecordSet %q programming failed: %s", + upstream.Name, currProgrammed.Message, + ) + } + } + return ctrl.Result{}, nil } @@ -255,6 +319,30 @@ func (r *DNSRecordSetReplicator) updateStatus( return c.Status().Patch(ctx, upstream, client.MergeFrom(base)) } +// ensureDisplayAnnotations sets the display-name and display-value annotations +// on the DNSRecordSet if they are missing or outdated. These annotations provide +// human-friendly values for ActivityPolicy audit rule templates. +// Returns true if annotations were modified (caller should patch the object). +func ensureDisplayAnnotations(rs *dnsv1alpha1.DNSRecordSet, zoneDomainName string) bool { + expectedDisplayName := computeDisplayName(rs, zoneDomainName) + expectedDisplayValue := computeDisplayValue(rs) + + if rs.Annotations == nil { + rs.Annotations = make(map[string]string) + } + + currentDisplayName := rs.Annotations[AnnotationDisplayName] + currentDisplayValue := rs.Annotations[AnnotationDisplayValue] + + if currentDisplayName == expectedDisplayName && currentDisplayValue == expectedDisplayValue { + return false + } + + rs.Annotations[AnnotationDisplayName] = expectedDisplayName + rs.Annotations[AnnotationDisplayValue] = expectedDisplayValue + return true +} + // ---- Watches / mapping helpers ------------------------- func (r *DNSRecordSetReplicator) SetupWithManager(mgr mcmanager.Manager, downstreamCl cluster.Cluster) error { r.mgr = mgr diff --git a/internal/controller/dnsrecordset_replicator_controller_test.go b/internal/controller/dnsrecordset_replicator_controller_test.go index 9caf4ab..628d0c0 100644 --- a/internal/controller/dnsrecordset_replicator_controller_test.go +++ b/internal/controller/dnsrecordset_replicator_controller_test.go @@ -12,6 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -188,3 +189,301 @@ func TestEnsureDownstreamRecordSet_ExistingLabelsPreserved(t *testing.T) { shadow.Annotations[downstreamclient.UpstreamOwnerNameAnnotation]) } } + +// TestEnsureDisplayAnnotations_SetsAnnotationsCorrectly verifies that +// ensureDisplayAnnotations correctly computes and sets display-name and +// display-value annotations on DNSRecordSet resources. +func TestEnsureDisplayAnnotations_SetsAnnotationsCorrectly(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + zoneDomainName string + wantDisplayName string + wantDisplayValue string + wantModified bool + }{ + { + name: "A record without annotations sets display name and value", + rs: &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "www-record", Namespace: "default"}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, + }, + }, + }, + zoneDomainName: "example.com", + wantDisplayName: "www.example.com", + wantDisplayValue: "192.0.2.10", + wantModified: true, + }, + { + name: "CNAME record sets target as display value", + rs: &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "api-record", Namespace: "default"}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "api", CNAME: &dnsv1alpha1.CNAMERecordSpec{Content: "api.internal.example.com"}}, + }, + }, + }, + zoneDomainName: "example.com", + wantDisplayName: "api.example.com", + wantDisplayValue: "api.internal.example.com", + wantModified: true, + }, + { + name: "apex record uses zone domain as display name", + rs: &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "apex-record", Namespace: "default"}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.1"}}, + }, + }, + }, + zoneDomainName: "example.com", + wantDisplayName: "example.com", + wantDisplayValue: "192.0.2.1", + wantModified: true, + }, + { + name: "already correct annotations returns false", + rs: &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "www-record", + Namespace: "default", + Annotations: map[string]string{ + AnnotationDisplayName: "www.example.com", + AnnotationDisplayValue: "192.0.2.10", + }, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, + }, + }, + }, + zoneDomainName: "example.com", + wantDisplayName: "www.example.com", + wantDisplayValue: "192.0.2.10", + wantModified: false, + }, + { + name: "outdated annotations are updated", + rs: &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "www-record", + Namespace: "default", + Annotations: map[string]string{ + AnnotationDisplayName: "www.example.com", + AnnotationDisplayValue: "192.0.2.99", // old IP + }, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, // new IP + }, + }, + }, + zoneDomainName: "example.com", + wantDisplayName: "www.example.com", + wantDisplayValue: "192.0.2.10", + wantModified: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rs := tt.rs.DeepCopy() + modified := ensureDisplayAnnotations(rs, tt.zoneDomainName) + + if modified != tt.wantModified { + t.Errorf("ensureDisplayAnnotations() modified = %v, want %v", modified, tt.wantModified) + } + if got := rs.Annotations[AnnotationDisplayName]; got != tt.wantDisplayName { + t.Errorf("display-name = %q, want %q", got, tt.wantDisplayName) + } + if got := rs.Annotations[AnnotationDisplayValue]; got != tt.wantDisplayValue { + t.Errorf("display-value = %q, want %q", got, tt.wantDisplayValue) + } + }) + } +} + +// TestEnsureDisplayAnnotations_IdempotentAfterFirstCall verifies that +// calling ensureDisplayAnnotations twice in a row returns false on the +// second call (no modifications needed), which is critical for avoiding +// infinite reconciliation loops. +func TestEnsureDisplayAnnotations_IdempotentAfterFirstCall(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "www-record", Namespace: "default"}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, + }, + }, + } + + // First call should modify + firstCall := ensureDisplayAnnotations(rs, "example.com") + if !firstCall { + t.Fatal("first call should return true (annotations were set)") + } + + // Second call with same data should NOT modify + secondCall := ensureDisplayAnnotations(rs, "example.com") + if secondCall { + t.Fatal("second call should return false (no changes needed)") + } +} + +// trackingClient wraps a client.Client to track Create/Patch operations. +// Used to verify that downstream operations occur even when annotations are updated. +type trackingClient struct { + client.Client + createCount int + patchCount int +} + +func (c *trackingClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + c.createCount++ + return c.Client.Create(ctx, obj, opts...) +} + +func (c *trackingClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + c.patchCount++ + return c.Client.Patch(ctx, obj, patch, opts...) +} + +// TestDNSRecordSetReplicator_EnsureDownstreamRecordSet_WorksWithAnnotationUpdates +// verifies that ensureDownstreamRecordSet works correctly when called after +// display annotations are updated. This test validates the components work +// together but does NOT test the Reconcile control flow directly. +// +// NOTE: A full integration test that calls Reconcile() would be needed to catch +// control flow bugs (like early returns). Such a test would require mocking the +// multicluster runtime manager. This test serves as a unit test for the +// individual components. +func TestDNSRecordSetReplicator_EnsureDownstreamRecordSet_CalledAfterAnnotationUpdate(t *testing.T) { + t.Parallel() + + scheme := newTestScheme(t) + + // Create zone and recordset without display annotations + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-zone", + Namespace: "default", + }, + Spec: dnsv1alpha1.DNSZoneSpec{ + DomainName: "example.com", + DNSZoneClassName: "pdns", + }, + } + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "www-record", + Namespace: "default", + // NO annotations - this triggers the annotation update path + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: zone.Name}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, + }, + }, + } + + upstreamClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&dnsv1alpha1.DNSRecordSet{}). + WithObjects(zone, rs). + Build() + + downstreamBase := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&dnsv1alpha1.DNSRecordSet{}). + Build() + downstreamClient := &trackingClient{Client: downstreamBase} + + strategy := fakeStrategy{ + namespace: "downstream", + name: "shadow-www-record", + client: downstreamClient, + } + + r := &DNSRecordSetReplicator{ + DownstreamClient: downstreamClient, + } + + // Simulate what happens in Reconcile: + // 1. Fetch upstream recordset + // 2. Check/set display annotations (this used to cause early return) + // 3. Call ensureDownstreamRecordSet + + ctx := context.Background() + + // Re-fetch to get current state + var upstream dnsv1alpha1.DNSRecordSet + if err := upstreamClient.Get(ctx, client.ObjectKeyFromObject(rs), &upstream); err != nil { + t.Fatalf("get upstream: %v", err) + } + + // Step 1: Check annotations - should need update + base := upstream.DeepCopy() + needsAnnotations := ensureDisplayAnnotations(&upstream, zone.Spec.DomainName) + if !needsAnnotations { + t.Fatal("expected annotations to need update for new recordset") + } + + // Step 2: Patch annotations (simulating what Reconcile does) + if err := upstreamClient.Patch(ctx, &upstream, client.MergeFrom(base)); err != nil { + t.Fatalf("patch annotations: %v", err) + } + + // Step 3: THIS IS THE CRITICAL CHECK - ensure downstream is still created + // In the buggy code, we would have returned early before reaching this point + _, err := r.ensureDownstreamRecordSet(ctx, strategy, &upstream) + if err != nil { + t.Fatalf("ensureDownstreamRecordSet: %v", err) + } + + // Verify downstream object was created + if downstreamClient.createCount == 0 && downstreamClient.patchCount == 0 { + t.Fatal("ensureDownstreamRecordSet did not create or patch downstream object") + } + + // Verify annotations are set on upstream + var updated dnsv1alpha1.DNSRecordSet + if err := upstreamClient.Get(ctx, client.ObjectKeyFromObject(rs), &updated); err != nil { + t.Fatalf("get updated: %v", err) + } + + if updated.Annotations[AnnotationDisplayName] != "www.example.com" { + t.Errorf("display-name = %q, want %q", updated.Annotations[AnnotationDisplayName], "www.example.com") + } + if updated.Annotations[AnnotationDisplayValue] != "192.0.2.10" { + t.Errorf("display-value = %q, want %q", updated.Annotations[AnnotationDisplayValue], "192.0.2.10") + } +} diff --git a/internal/controller/dnszone_replicator_controller.go b/internal/controller/dnszone_replicator_controller.go index 2baf729..56367fa 100644 --- a/internal/controller/dnszone_replicator_controller.go +++ b/internal/controller/dnszone_replicator_controller.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -64,6 +65,10 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque return ctrl.Result{}, client.IgnoreNotFound(err) } + // Obtain a per-cluster event recorder so events are written to the upstream + // (user-facing) cluster API server, not just the deployment cluster. + recorder := upstreamCl.GetEventRecorderFor("dns-operator") + // --- Ensure finalizer on creation/update (non-deletion path) --- if upstream.DeletionTimestamp.IsZero() { if !controllerutil.ContainsFinalizer(&upstream, dnsZoneFinalizer) { @@ -77,6 +82,7 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque // Re-run with updated object return ctrl.Result{}, nil } + } else { // Deletion guard: ensure downstream is gone before removing finalizer if controllerutil.ContainsFinalizer(&upstream, dnsZoneFinalizer) { @@ -204,6 +210,18 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque if perr := upstreamCl.GetClient().Status().Patch(ctx, &upstream, client.MergeFrom(base)); perr != nil { return ctrl.Result{}, perr } + // Emit activity event so users get real-time feedback about the conflict. + recordZoneActivityEventWithData(recorder, + ZoneEventData{ + Zone: &upstream, + FailReason: "DNSZone claimed by another resource", + }, + corev1.EventTypeWarning, + EventReasonZoneProgrammingFailed, + ActivityTypeZoneProgrammingFailed, + "DNSZone %q programming failed: domain already claimed by another DNSZone", + upstream.Name, + ) } return ctrl.Result{}, nil } @@ -237,6 +255,15 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque return ctrl.Result{}, err } + // Capture the Programmed condition before the final status update so we can + // detect a transition after the update completes. + prevProgrammed := apimeta.FindStatusCondition(upstream.Status.Conditions, CondProgrammed) + var prevProgrammedCopy *metav1.Condition + if prevProgrammed != nil { + c := *prevProgrammed + prevProgrammedCopy = &c + } + // Recompute status to set Programmed based on record presence if err := r.updateStatus(ctx, upstreamCl.GetClient(), strategy, &upstream); err != nil { if !apierrors.IsNotFound(err) { @@ -244,6 +271,37 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque } } + // Emit Programmed/ProgrammingFailed events on condition transitions. + currProgrammed := apimeta.FindStatusCondition(upstream.Status.Conditions, CondProgrammed) + if programmedConditionTransitioned(prevProgrammedCopy, currProgrammed) { + if currProgrammed.Status == metav1.ConditionTrue { + recordZoneActivityEventWithData(recorder, + ZoneEventData{ + Zone: &upstream, + Nameservers: upstream.Status.Nameservers, + }, + corev1.EventTypeNormal, + EventReasonZoneProgrammed, + ActivityTypeZoneProgrammed, + "DNSZone %q successfully programmed with nameservers: %s", + upstream.Name, strings.Join(upstream.Status.Nameservers, ", "), + ) + } else if currProgrammed.Status == metav1.ConditionFalse && + prevProgrammedCopy != nil && prevProgrammedCopy.Status == metav1.ConditionTrue { + recordZoneActivityEventWithData(recorder, + ZoneEventData{ + Zone: &upstream, + FailReason: currProgrammed.Message, + }, + corev1.EventTypeWarning, + EventReasonZoneProgrammingFailed, + ActivityTypeZoneProgrammingFailed, + "DNSZone %q programming failed: %s", + upstream.Name, currProgrammed.Message, + ) + } + } + return ctrl.Result{}, nil } diff --git a/internal/controller/events.go b/internal/controller/events.go new file mode 100644 index 0000000..b18b89f --- /dev/null +++ b/internal/controller/events.go @@ -0,0 +1,405 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "fmt" + "strconv" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" +) + +// Annotation keys used by the DNS service for ActivityPolicy event matching. +// These are DNS service-specific annotations that the DNS service configures +// via ActivityPolicy to generate human-readable activity summaries. +const ( + AnnotationEventType = "dns.networking.miloapis.com/event-type" + AnnotationObservedGeneration = "dns.networking.miloapis.com/observed-generation" + AnnotationDomainName = "dns.networking.miloapis.com/domain-name" + AnnotationRecordType = "dns.networking.miloapis.com/record-type" + + // Resource identity annotations present on all events. + AnnotationResourceName = "dns.networking.miloapis.com/resource-name" + AnnotationResourceNamespace = "dns.networking.miloapis.com/resource-namespace" + + // DNSZone-specific annotations. + AnnotationZoneClass = "dns.networking.miloapis.com/zone-class" + AnnotationNameservers = "dns.networking.miloapis.com/nameservers" + + // DNSRecordSet-specific annotations. + AnnotationZoneRef = "dns.networking.miloapis.com/zone-ref" + AnnotationRecordCount = "dns.networking.miloapis.com/record-count" + AnnotationRecordNames = "dns.networking.miloapis.com/record-names" + AnnotationIPAddresses = "dns.networking.miloapis.com/ip-addresses" + + // Display annotations for human-readable activity summaries. + // These are set on DNSRecordSet resources by the controller for use in + // ActivityPolicy audit rule templates. They provide pre-formatted + // user-friendly values that would be complex to compute in CEL. + AnnotationDisplayName = "dns.networking.miloapis.com/display-name" + AnnotationDisplayValue = "dns.networking.miloapis.com/display-value" + + // Type-specific event annotations for record values. + // These supplement ip-addresses for non-IP record types. + AnnotationCNAMETarget = "dns.networking.miloapis.com/cname-target" + AnnotationMXHosts = "dns.networking.miloapis.com/mx-hosts" + + // Shared failure annotation. + AnnotationFailureReason = "dns.networking.miloapis.com/failure-reason" +) + +// Kubernetes Event reason codes (PascalCase per k8s convention). +// CRUD lifecycle events (created/updated/deleted) are intentionally absent; +// those are captured by audit logs. Only async outcome events that audit logs +// cannot capture are emitted here. +const ( + // DNSZone event reasons. + EventReasonZoneProgrammed = "ZoneProgrammed" + EventReasonZoneProgrammingFailed = "ZoneProgrammingFailed" + + // DNSRecordSet event reasons. + EventReasonRecordSetProgrammed = "RecordSetProgrammed" + EventReasonRecordSetProgrammingFailed = "RecordSetProgrammingFailed" +) + +// Activity type strings (dot-notation for platform Activity selector matching). +// CRUD lifecycle activity types (created/updated/deleted) are intentionally absent; +// those are captured by audit logs. Only async outcome events that audit logs +// cannot capture are emitted here. +const ( + // DNSZone activity types. + ActivityTypeZoneProgrammed = "dns.zone.programmed" + ActivityTypeZoneProgrammingFailed = "dns.zone.programming_failed" + + // DNSRecordSet activity types. + ActivityTypeRecordSetProgrammed = "dns.recordset.programmed" + ActivityTypeRecordSetProgrammingFailed = "dns.recordset.programming_failed" +) + +// ZoneEventData holds all the data needed to emit an annotated activity event +// for a DNSZone. Nameservers and FailReason are optional and only used for +// specific event types (programmed and programming_failed respectively). +type ZoneEventData struct { + Zone *dnsv1alpha1.DNSZone + Nameservers []string // for programmed events; mirrors zone.Status.Nameservers + FailReason string // for programming_failed events; from condition Message +} + +// RecordSetEventData holds all the data needed to emit an annotated activity +// event for a DNSRecordSet. DomainName and FailReason are optional. +type RecordSetEventData struct { + RecordSet *dnsv1alpha1.DNSRecordSet + DomainName string // parent zone's domain; from zone.Spec.DomainName + FailReason string // for programming_failed events; from condition Message +} + +// recordZoneActivityEvent emits an annotated Kubernetes event for a DNSZone. +// It attaches resource identity, domain-name, zone-class, and (where available) +// nameservers and failure-reason annotations so the DNS service ActivityPolicy +// can filter, route and template the event. +func recordZoneActivityEvent( + recorder record.EventRecorder, + zone *dnsv1alpha1.DNSZone, + eventType, reason, activityType string, + messageFmt string, + args ...interface{}, +) { + recordZoneActivityEventWithData(recorder, ZoneEventData{Zone: zone}, eventType, reason, activityType, messageFmt, args...) +} + +// recordZoneActivityEventWithData is the full-featured variant that accepts a +// ZoneEventData struct containing optional nameservers and failure reason. +func recordZoneActivityEventWithData( + recorder record.EventRecorder, + data ZoneEventData, + eventType, reason, activityType string, + messageFmt string, + args ...interface{}, +) { + if recorder == nil { + return + } + zone := data.Zone + annotations := map[string]string{ + AnnotationEventType: activityType, + AnnotationObservedGeneration: strconv.FormatInt(zone.Generation, 10), + AnnotationDomainName: zone.Spec.DomainName, + AnnotationResourceName: zone.Name, + AnnotationResourceNamespace: zone.Namespace, + } + if zone.Spec.DNSZoneClassName != "" { + annotations[AnnotationZoneClass] = zone.Spec.DNSZoneClassName + } + if len(data.Nameservers) > 0 { + annotations[AnnotationNameservers] = strings.Join(data.Nameservers, ",") + } + if data.FailReason != "" { + annotations[AnnotationFailureReason] = data.FailReason + } + recorder.AnnotatedEventf(zone, annotations, eventType, reason, messageFmt, args...) +} + +// recordRecordSetActivityEvent emits an annotated Kubernetes event for a +// DNSRecordSet. It attaches resource identity and record-type annotations so +// the DNS service ActivityPolicy can filter events by DNS record type. +func recordRecordSetActivityEvent( + recorder record.EventRecorder, + rs *dnsv1alpha1.DNSRecordSet, + eventType, reason, activityType string, + messageFmt string, + args ...interface{}, +) { + recordRecordSetActivityEventWithData(recorder, RecordSetEventData{RecordSet: rs}, eventType, reason, activityType, messageFmt, args...) +} + +// recordRecordSetActivityEventWithData is the full-featured variant that accepts +// a RecordSetEventData struct containing optional domain name and failure reason. +func recordRecordSetActivityEventWithData( + recorder record.EventRecorder, + data RecordSetEventData, + eventType, reason, activityType string, + messageFmt string, + args ...interface{}, +) { + if recorder == nil { + return + } + rs := data.RecordSet + annotations := map[string]string{ + AnnotationEventType: activityType, + AnnotationObservedGeneration: strconv.FormatInt(rs.Generation, 10), + AnnotationRecordType: string(rs.Spec.RecordType), + AnnotationResourceName: rs.Name, + AnnotationResourceNamespace: rs.Namespace, + AnnotationZoneRef: rs.Spec.DNSZoneRef.Name, + } + if data.DomainName != "" { + annotations[AnnotationDomainName] = data.DomainName + } + if len(rs.Spec.Records) > 0 { + annotations[AnnotationRecordCount] = strconv.Itoa(len(rs.Spec.Records)) + // Collect unique record names preserving order of first occurrence. + annotations[AnnotationRecordNames] = strings.Join(uniqueRecordNames(rs), ",") + } + // Extract IP addresses for A/AAAA record types. + if ips := extractIPAddresses(rs); len(ips) > 0 { + annotations[AnnotationIPAddresses] = strings.Join(ips, ",") + } + // Extract type-specific record values for activity summaries. + switch rs.Spec.RecordType { + case dnsv1alpha1.RRTypeCNAME: + if target := extractCNAMETarget(rs); target != "" { + annotations[AnnotationCNAMETarget] = target + } + case dnsv1alpha1.RRTypeMX: + if hosts := extractMXHosts(rs); hosts != "" { + annotations[AnnotationMXHosts] = hosts + } + } + // Add display-name annotation (pre-computed FQDN) for easier template usage. + if data.DomainName != "" { + annotations[AnnotationDisplayName] = computeDisplayName(rs, data.DomainName) + } + if data.FailReason != "" { + annotations[AnnotationFailureReason] = data.FailReason + } + recorder.AnnotatedEventf(rs, annotations, eventType, reason, messageFmt, args...) +} + +// uniqueRecordNames returns a deduplicated list of record names from the +// DNSRecordSet, preserving the order of first occurrence. This handles the +// common case where multiple records share the same name (e.g., round-robin A +// records) as well as the less common case of different names in one set. +func uniqueRecordNames(rs *dnsv1alpha1.DNSRecordSet) []string { + seen := make(map[string]struct{}) + var names []string + for _, r := range rs.Spec.Records { + if _, ok := seen[r.Name]; !ok { + seen[r.Name] = struct{}{} + names = append(names, r.Name) + } + } + return names +} + +// extractIPAddresses collects IP address content values from A and AAAA record +// entries in the given DNSRecordSet. Returns nil for all other record types. +func extractIPAddresses(rs *dnsv1alpha1.DNSRecordSet) []string { + switch rs.Spec.RecordType { + case dnsv1alpha1.RRTypeA: + var ips []string + for _, r := range rs.Spec.Records { + if r.A != nil && r.A.Content != "" { + ips = append(ips, r.A.Content) + } + } + return ips + case dnsv1alpha1.RRTypeAAAA: + var ips []string + for _, r := range rs.Spec.Records { + if r.AAAA != nil && r.AAAA.Content != "" { + ips = append(ips, r.AAAA.Content) + } + } + return ips + default: + return nil + } +} + +// extractCNAMETarget returns the CNAME target from the first record entry. +// Returns empty string if not a CNAME record or if no valid target exists. +func extractCNAMETarget(rs *dnsv1alpha1.DNSRecordSet) string { + if rs.Spec.RecordType != dnsv1alpha1.RRTypeCNAME { + return "" + } + if len(rs.Spec.Records) > 0 && rs.Spec.Records[0].CNAME != nil { + return rs.Spec.Records[0].CNAME.Content + } + return "" +} + +// extractMXHosts returns a formatted string of MX hosts with their preferences. +// Format: "10 mail.example.com, 20 mail2.example.com" +func extractMXHosts(rs *dnsv1alpha1.DNSRecordSet) string { + if rs.Spec.RecordType != dnsv1alpha1.RRTypeMX { + return "" + } + var hosts []string + for _, r := range rs.Spec.Records { + if r.MX != nil && r.MX.Exchange != "" { + hosts = append(hosts, fmt.Sprintf("%d %s", r.MX.Preference, r.MX.Exchange)) + } + } + return strings.Join(hosts, ", ") +} + +// computeDisplayName returns a human-friendly name for the DNSRecordSet. +// For most records this is the FQDN (e.g., "www.example.com"). +// For multiple unique names, returns comma-separated FQDNs. +func computeDisplayName(rs *dnsv1alpha1.DNSRecordSet, zoneDomainName string) string { + names := uniqueRecordNames(rs) + if len(names) == 0 { + return "" + } + fqdns := make([]string, 0, len(names)) + for _, name := range names { + fqdn := buildFQDN(name, zoneDomainName) + fqdns = append(fqdns, fqdn) + } + return strings.Join(fqdns, ", ") +} + +// computeDisplayValue returns a human-friendly value for the DNSRecordSet +// based on its record type. This is used in activity summaries to show +// what the record points to (IP addresses, CNAME targets, MX hosts, etc.). +func computeDisplayValue(rs *dnsv1alpha1.DNSRecordSet) string { + const maxLength = 200 + + switch rs.Spec.RecordType { + case dnsv1alpha1.RRTypeA, dnsv1alpha1.RRTypeAAAA: + ips := extractIPAddresses(rs) + result := strings.Join(ips, ", ") + if len(result) > maxLength { + return result[:maxLength-3] + "..." + } + return result + + case dnsv1alpha1.RRTypeCNAME: + return extractCNAMETarget(rs) + + case dnsv1alpha1.RRTypeALIAS: + if len(rs.Spec.Records) > 0 && rs.Spec.Records[0].ALIAS != nil { + return rs.Spec.Records[0].ALIAS.Content + } + return "" + + case dnsv1alpha1.RRTypeMX: + result := extractMXHosts(rs) + if len(result) > maxLength { + return result[:maxLength-3] + "..." + } + return result + + case dnsv1alpha1.RRTypeTXT: + if len(rs.Spec.Records) > 0 && rs.Spec.Records[0].TXT != nil { + content := rs.Spec.Records[0].TXT.Content + if len(content) > 60 { + return fmt.Sprintf("\"%s...\"", content[:57]) + } + return fmt.Sprintf("\"%s\"", content) + } + return "" + + case dnsv1alpha1.RRTypeNS: + var servers []string + for _, r := range rs.Spec.Records { + if r.NS != nil && r.NS.Content != "" { + servers = append(servers, r.NS.Content) + } + } + result := strings.Join(servers, ", ") + if len(result) > maxLength { + return result[:maxLength-3] + "..." + } + return result + + case dnsv1alpha1.RRTypeSRV: + var entries []string + for _, r := range rs.Spec.Records { + if r.SRV != nil { + entries = append(entries, fmt.Sprintf("%d %d %d %s", + r.SRV.Priority, r.SRV.Weight, r.SRV.Port, r.SRV.Target)) + } + } + result := strings.Join(entries, ", ") + if len(result) > maxLength { + return result[:maxLength-3] + "..." + } + return result + + default: + if len(rs.Spec.Records) > 1 { + return fmt.Sprintf("%d records", len(rs.Spec.Records)) + } + return "(see details)" + } +} + +// buildFQDN constructs a fully-qualified domain name from a record name +// and zone domain. Handles the special "@" name for zone apex. +func buildFQDN(recordName, zoneDomainName string) string { + if recordName == "@" { + return zoneDomainName + } + return recordName + "." + zoneDomainName +} + +// programmedConditionTransitioned returns true when the Programmed condition changed +// in a way that warrants an event: +// - nil/non-True -> True: emit "programmed" (first successful programming) +// - True -> False: emit "programming_failed" +// - nil -> False: returns false (no event for first-time failures before any success) +// - curr nil: returns false (condition removed, no event) +func programmedConditionTransitioned(prev, curr *metav1.Condition) bool { + if curr == nil { + return false + } + if curr.Status == metav1.ConditionTrue { + // Emit on any transition to True (including first-time nil->True) + prevStatus := metav1.ConditionFalse + if prev != nil { + prevStatus = prev.Status + } + return prevStatus != metav1.ConditionTrue + } + // Emit True->False only; nil->False and False->False do not trigger + if curr.Status == metav1.ConditionFalse { + return prev != nil && prev.Status == metav1.ConditionTrue + } + return false +} diff --git a/internal/controller/events_test.go b/internal/controller/events_test.go new file mode 100644 index 0000000..81e4a4c --- /dev/null +++ b/internal/controller/events_test.go @@ -0,0 +1,1303 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "strconv" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" +) + +// -------------------------------------------------------------------------- +// programmedConditionTransitioned tests +// -------------------------------------------------------------------------- + +// TestProgrammedConditionTransitioned verifies that +// programmedConditionTransitioned returns true only when the Programmed +// condition status changes between two reconcile observations (e.g. nil -> True, +// True -> False, False -> True), and false when the status is unchanged or when +// the current condition is absent. +func TestProgrammedConditionTransitioned(t *testing.T) { + t.Parallel() + + condTrue := &metav1.Condition{ + Type: CondProgrammed, + Status: metav1.ConditionTrue, + } + condFalse := &metav1.Condition{ + Type: CondProgrammed, + Status: metav1.ConditionFalse, + } + + // Each case describes a (prev, curr) pair and whether a transition event + // should be emitted. The subtest name doubles as documentation of the rule. + tests := []struct { + name string + prev *metav1.Condition + curr *metav1.Condition + want bool + }{ + { + name: "nil prev, True curr: first-time programming emits event", + prev: nil, + curr: condTrue, + want: true, + }, + { + name: "nil prev, False curr: no event on first failure", + prev: nil, + curr: condFalse, + want: false, + }, + { + name: "True prev, False curr: programming failure emits event", + prev: condTrue, + curr: condFalse, + want: true, + }, + { + name: "False prev, True curr: recovery emits event", + prev: condFalse, + curr: condTrue, + want: true, + }, + { + name: "False prev, False curr: no transition, no event", + prev: condFalse, + curr: condFalse, + want: false, + }, + { + name: "True prev, True curr: no transition, no event", + prev: condTrue, + curr: condTrue, + want: false, + }, + { + name: "True prev, nil curr: condition removed, no event", + prev: condTrue, + curr: nil, + want: false, + }, + { + name: "False prev, nil curr: condition removed, no event", + prev: condFalse, + curr: nil, + want: false, + }, + { + name: "nil prev, nil curr: both absent, no event", + prev: nil, + curr: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := programmedConditionTransitioned(tt.prev, tt.curr) + if got != tt.want { + t.Errorf("programmedConditionTransitioned(%v, %v) = %v, want %v", + tt.prev, tt.curr, got, tt.want) + } + }) + } +} + +// -------------------------------------------------------------------------- +// recordZoneActivityEvent tests +// -------------------------------------------------------------------------- + +// TestRecordZoneActivityEvent_NilRecorder verifies that recordZoneActivityEvent +// does not panic when called with a nil recorder, so callers that omit the +// recorder (e.g. in unit tests or during startup) are safe. +func TestRecordZoneActivityEvent_NilRecorder(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-zone", + Namespace: "default", + Generation: 1, + }, + Spec: dnsv1alpha1.DNSZoneSpec{ + DomainName: "example.com", + DNSZoneClassName: "pdns", + }, + } + + // Should not panic with nil recorder. + recordZoneActivityEvent(nil, zone, + corev1.EventTypeNormal, + EventReasonZoneProgrammed, + ActivityTypeZoneProgrammed, + "DNSZone %q successfully programmed", + zone.Name, + ) +} + +// TestRecordZoneActivityEvent_EmitsWithCorrectAnnotations verifies that +// recordZoneActivityEvent emits exactly one AnnotatedEventf call and that the +// resulting event carries the correct eventType, reason, activity-type +// annotation, domain-name annotation, observed-generation annotation, and does +// NOT include the record-type annotation (which is exclusive to record-set events). +func TestRecordZoneActivityEvent_EmitsWithCorrectAnnotations(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-zone", + Namespace: "default", + Generation: 3, + }, + Spec: dnsv1alpha1.DNSZoneSpec{ + DomainName: "example.com", + DNSZoneClassName: "pdns", + }, + } + + // Each case exercises a different event reason/activity-type combination for + // a DNSZone. The assertions after the call check both the Kubernetes event + // fields (eventType, reason) and the activity annotations attached to it. + tests := []struct { + name string + eventType string + reason string + activityType string + messageFmt string + wantReason string + wantEventType string + wantActvType string + wantDomainName string + wantGeneration string + }{ + { + name: "ZoneProgrammed normal event", + eventType: corev1.EventTypeNormal, + reason: EventReasonZoneProgrammed, + activityType: ActivityTypeZoneProgrammed, + messageFmt: "DNSZone %q successfully programmed", + wantReason: EventReasonZoneProgrammed, + wantEventType: corev1.EventTypeNormal, + wantActvType: ActivityTypeZoneProgrammed, + wantDomainName: "example.com", + wantGeneration: "3", + }, + { + name: "ZoneProgrammingFailed warning event", + eventType: corev1.EventTypeWarning, + reason: EventReasonZoneProgrammingFailed, + activityType: ActivityTypeZoneProgrammingFailed, + messageFmt: "DNSZone %q programming failed", + wantReason: EventReasonZoneProgrammingFailed, + wantEventType: corev1.EventTypeWarning, + wantActvType: ActivityTypeZoneProgrammingFailed, + wantDomainName: "example.com", + wantGeneration: "3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fakeRecorder := record.NewFakeRecorder(10) + // Capture annotations via a wrapping recorder that records annotated calls. + annotatedRecorder := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEvent(annotatedRecorder, zone, + tt.eventType, + tt.reason, + tt.activityType, + tt.messageFmt, + zone.Name, + ) + + if len(annotatedRecorder.calls) != 1 { + t.Fatalf("expected 1 annotated event call, got %d", len(annotatedRecorder.calls)) + } + call := annotatedRecorder.calls[0] + + if call.annotations[AnnotationEventType] != tt.wantActvType { + t.Errorf("annotation %q = %q, want %q", AnnotationEventType, call.annotations[AnnotationEventType], tt.wantActvType) + } + if call.annotations[AnnotationObservedGeneration] != tt.wantGeneration { + t.Errorf("annotation %q = %q, want %q", AnnotationObservedGeneration, call.annotations[AnnotationObservedGeneration], tt.wantGeneration) + } + if call.annotations[AnnotationDomainName] != tt.wantDomainName { + t.Errorf("annotation %q = %q, want %q", AnnotationDomainName, call.annotations[AnnotationDomainName], tt.wantDomainName) + } + if _, hasRecordType := call.annotations[AnnotationRecordType]; hasRecordType { + t.Errorf("zone event should not have %q annotation", AnnotationRecordType) + } + if call.eventType != tt.wantEventType { + t.Errorf("eventType = %q, want %q", call.eventType, tt.wantEventType) + } + if call.reason != tt.wantReason { + t.Errorf("reason = %q, want %q", call.reason, tt.wantReason) + } + }) + } +} + +// TestRecordZoneActivityEvent_GenerationInAnnotation verifies that the +// observed-generation annotation is populated from the DNSZone's +// ObjectMeta.Generation field, confirming the annotation tracks spec +// revisions correctly across updates. +func TestRecordZoneActivityEvent_GenerationInAnnotation(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gen-zone", + Namespace: "default", + Generation: 42, + }, + Spec: dnsv1alpha1.DNSZoneSpec{ + DomainName: "gen.example.com", + DNSZoneClassName: "pdns", + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + annotatedRecorder := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEvent(annotatedRecorder, zone, + corev1.EventTypeNormal, + EventReasonZoneProgrammed, + ActivityTypeZoneProgrammed, + "DNSZone %q successfully programmed", + zone.Name, + ) + + if len(annotatedRecorder.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(annotatedRecorder.calls)) + } + wantGen := strconv.FormatInt(42, 10) + if got := annotatedRecorder.calls[0].annotations[AnnotationObservedGeneration]; got != wantGen { + t.Errorf("observed-generation annotation = %q, want %q", got, wantGen) + } +} + +// -------------------------------------------------------------------------- +// recordRecordSetActivityEvent tests +// -------------------------------------------------------------------------- + +// TestRecordRecordSetActivityEvent_NilRecorder verifies that +// recordRecordSetActivityEvent does not panic when called with a nil recorder, +// matching the same nil-safety guarantee provided for zone events. +func TestRecordRecordSetActivityEvent_NilRecorder(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rs", + Namespace: "default", + Generation: 1, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "test-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}, + }, + }, + } + + // Should not panic with nil recorder. + recordRecordSetActivityEvent(nil, rs, + corev1.EventTypeNormal, + EventReasonRecordSetProgrammed, + ActivityTypeRecordSetProgrammed, + "DNSRecordSet %q successfully programmed to DNS provider", + rs.Name, + ) +} + +// TestRecordRecordSetActivityEvent_EmitsWithCorrectAnnotations verifies that +// recordRecordSetActivityEvent emits exactly one AnnotatedEventf call carrying +// the correct eventType, reason, activity-type annotation, record-type +// annotation, and observed-generation annotation, and does NOT include the +// domain-name annotation (which is exclusive to zone events). +func TestRecordRecordSetActivityEvent_EmitsWithCorrectAnnotations(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "www-records", + Namespace: "default", + Generation: 5, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}, + }, + }, + } + + // Each case exercises a different event reason/activity-type combination for + // a DNSRecordSet. Assertions confirm both the Kubernetes event fields and the + // activity annotations, including the record-type annotation that zone events omit. + tests := []struct { + name string + eventType string + reason string + activityType string + messageFmt string + wantReason string + wantEventType string + wantActvType string + wantRecordType string + wantGeneration string + }{ + { + name: "RecordSetProgrammed normal event", + eventType: corev1.EventTypeNormal, + reason: EventReasonRecordSetProgrammed, + activityType: ActivityTypeRecordSetProgrammed, + messageFmt: "DNSRecordSet %q programmed", + wantReason: EventReasonRecordSetProgrammed, + wantEventType: corev1.EventTypeNormal, + wantActvType: ActivityTypeRecordSetProgrammed, + wantRecordType: "A", + wantGeneration: "5", + }, + { + name: "RecordSetProgrammingFailed warning event", + eventType: corev1.EventTypeWarning, + reason: EventReasonRecordSetProgrammingFailed, + activityType: ActivityTypeRecordSetProgrammingFailed, + messageFmt: "DNSRecordSet %q programming failed", + wantReason: EventReasonRecordSetProgrammingFailed, + wantEventType: corev1.EventTypeWarning, + wantActvType: ActivityTypeRecordSetProgrammingFailed, + wantRecordType: "A", + wantGeneration: "5", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fakeRecorder := record.NewFakeRecorder(10) + annotatedRecorder := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEvent(annotatedRecorder, rs, + tt.eventType, + tt.reason, + tt.activityType, + tt.messageFmt, + rs.Name, + ) + + if len(annotatedRecorder.calls) != 1 { + t.Fatalf("expected 1 annotated event call, got %d", len(annotatedRecorder.calls)) + } + call := annotatedRecorder.calls[0] + + if call.annotations[AnnotationEventType] != tt.wantActvType { + t.Errorf("annotation %q = %q, want %q", AnnotationEventType, call.annotations[AnnotationEventType], tt.wantActvType) + } + if call.annotations[AnnotationObservedGeneration] != tt.wantGeneration { + t.Errorf("annotation %q = %q, want %q", AnnotationObservedGeneration, call.annotations[AnnotationObservedGeneration], tt.wantGeneration) + } + if call.annotations[AnnotationRecordType] != tt.wantRecordType { + t.Errorf("annotation %q = %q, want %q", AnnotationRecordType, call.annotations[AnnotationRecordType], tt.wantRecordType) + } + if _, hasDomainName := call.annotations[AnnotationDomainName]; hasDomainName { + t.Errorf("recordset event should not have %q annotation", AnnotationDomainName) + } + if call.eventType != tt.wantEventType { + t.Errorf("eventType = %q, want %q", call.eventType, tt.wantEventType) + } + if call.reason != tt.wantReason { + t.Errorf("reason = %q, want %q", call.reason, tt.wantReason) + } + }) + } +} + +// TestRecordRecordSetActivityEvent_RecordTypePropagated verifies that the +// record-type annotation reflects the DNSRecordSet's Spec.RecordType for every +// supported RRType value, ensuring the annotation is not hardcoded and correctly +// follows whatever type the caller specifies. +func TestRecordRecordSetActivityEvent_RecordTypePropagated(t *testing.T) { + t.Parallel() + + recordTypes := []dnsv1alpha1.RRType{ + dnsv1alpha1.RRTypeA, + dnsv1alpha1.RRTypeAAAA, + dnsv1alpha1.RRTypeCNAME, + dnsv1alpha1.RRTypeTXT, + dnsv1alpha1.RRTypeNS, + } + + for _, rt := range recordTypes { + t.Run(string(rt), func(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rs", + Namespace: "default", + Generation: 1, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "test-zone"}, + RecordType: rt, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@"}, + }, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + annotatedRecorder := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEvent(annotatedRecorder, rs, + corev1.EventTypeNormal, + EventReasonRecordSetProgrammed, + ActivityTypeRecordSetProgrammed, + "DNSRecordSet %q successfully programmed to DNS provider", + rs.Name, + ) + + if len(annotatedRecorder.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(annotatedRecorder.calls)) + } + got := annotatedRecorder.calls[0].annotations[AnnotationRecordType] + if got != string(rt) { + t.Errorf("record-type annotation = %q, want %q", got, string(rt)) + } + }) + } +} + +// -------------------------------------------------------------------------- +// recordZoneActivityEventWithData tests +// -------------------------------------------------------------------------- + +// TestRecordZoneActivityEventWithData_ResourceIdentityAnnotations verifies that the +// resource-name and resource-namespace annotations are set from the zone object +// and that zone-class is set when DNSZoneClassName is non-empty. +func TestRecordZoneActivityEventWithData_ResourceIdentityAnnotations(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-zone", + Namespace: "project-ns", + Generation: 2, + }, + Spec: dnsv1alpha1.DNSZoneSpec{ + DomainName: "example.com", + DNSZoneClassName: "pdns", + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEventWithData(ar, ZoneEventData{Zone: zone}, + corev1.EventTypeNormal, + EventReasonZoneProgrammed, + ActivityTypeZoneProgrammed, + "DNSZone %q successfully programmed", + zone.Name, + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + ann := ar.calls[0].annotations + + if got := ann[AnnotationResourceName]; got != "my-zone" { + t.Errorf("%s = %q, want %q", AnnotationResourceName, got, "my-zone") + } + if got := ann[AnnotationResourceNamespace]; got != "project-ns" { + t.Errorf("%s = %q, want %q", AnnotationResourceNamespace, got, "project-ns") + } + if got := ann[AnnotationZoneClass]; got != "pdns" { + t.Errorf("%s = %q, want %q", AnnotationZoneClass, got, "pdns") + } + if _, ok := ann[AnnotationNameservers]; ok { + t.Errorf("%s should be absent when no nameservers are supplied", AnnotationNameservers) + } + if _, ok := ann[AnnotationFailureReason]; ok { + t.Errorf("%s should be absent when no fail reason is supplied", AnnotationFailureReason) + } +} + +// TestRecordZoneActivityEventWithData_ZoneClassAbsentWhenEmpty verifies that the +// zone-class annotation is omitted when DNSZoneClassName is not set. +func TestRecordZoneActivityEventWithData_ZoneClassAbsentWhenEmpty(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{Name: "z", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "example.com", DNSZoneClassName: ""}, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEventWithData(ar, ZoneEventData{Zone: zone}, + corev1.EventTypeNormal, EventReasonZoneProgrammed, ActivityTypeZoneProgrammed, "msg") + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + if _, ok := ar.calls[0].annotations[AnnotationZoneClass]; ok { + t.Errorf("%s should be absent when DNSZoneClassName is empty", AnnotationZoneClass) + } +} + +// TestRecordZoneActivityEventWithData_NameserversAnnotation verifies that the +// nameservers annotation is set as a comma-separated list when provided. +func TestRecordZoneActivityEventWithData_NameserversAnnotation(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{Name: "z", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "example.com", DNSZoneClassName: "pdns"}, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEventWithData(ar, + ZoneEventData{ + Zone: zone, + Nameservers: []string{"ns1.example.com", "ns2.example.com"}, + }, + corev1.EventTypeNormal, EventReasonZoneProgrammed, ActivityTypeZoneProgrammed, + "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + got := ar.calls[0].annotations[AnnotationNameservers] + want := "ns1.example.com,ns2.example.com" + if got != want { + t.Errorf("%s = %q, want %q", AnnotationNameservers, got, want) + } +} + +// TestRecordZoneActivityEventWithData_FailReasonAnnotation verifies that the +// failure-reason annotation is set when FailReason is non-empty. +func TestRecordZoneActivityEventWithData_FailReasonAnnotation(t *testing.T) { + t.Parallel() + + zone := &dnsv1alpha1.DNSZone{ + ObjectMeta: metav1.ObjectMeta{Name: "z", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "example.com", DNSZoneClassName: "pdns"}, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordZoneActivityEventWithData(ar, + ZoneEventData{ + Zone: zone, + FailReason: "upstream provider rejected the zone", + }, + corev1.EventTypeWarning, + EventReasonZoneProgrammingFailed, + ActivityTypeZoneProgrammingFailed, + "programming failed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + got := ar.calls[0].annotations[AnnotationFailureReason] + want := "upstream provider rejected the zone" + if got != want { + t.Errorf("%s = %q, want %q", AnnotationFailureReason, got, want) + } +} + +// -------------------------------------------------------------------------- +// recordRecordSetActivityEventWithData tests +// -------------------------------------------------------------------------- + +// TestRecordRecordSetActivityEventWithData_ResourceIdentityAnnotations verifies +// that resource-name, resource-namespace, and zone-ref annotations are always +// present, that record-count and record-name are set when records exist, and +// that domain-name is present only when DomainName is supplied. +func TestRecordRecordSetActivityEventWithData_ResourceIdentityAnnotations(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "www-records", + Namespace: "project-ns", + Generation: 3, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "my-zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}, + {Name: "api", A: &dnsv1alpha1.ARecordSpec{Content: "5.6.7.8"}}, + }, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, + RecordSetEventData{ + RecordSet: rs, + DomainName: "example.com", + }, + corev1.EventTypeNormal, + EventReasonRecordSetProgrammed, + ActivityTypeRecordSetProgrammed, + "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + ann := ar.calls[0].annotations + + if got := ann[AnnotationResourceName]; got != "www-records" { + t.Errorf("%s = %q, want %q", AnnotationResourceName, got, "www-records") + } + if got := ann[AnnotationResourceNamespace]; got != "project-ns" { + t.Errorf("%s = %q, want %q", AnnotationResourceNamespace, got, "project-ns") + } + if got := ann[AnnotationZoneRef]; got != "my-zone" { + t.Errorf("%s = %q, want %q", AnnotationZoneRef, got, "my-zone") + } + if got := ann[AnnotationDomainName]; got != "example.com" { + t.Errorf("%s = %q, want %q", AnnotationDomainName, got, "example.com") + } + if got := ann[AnnotationRecordCount]; got != "2" { + t.Errorf("%s = %q, want %q", AnnotationRecordCount, got, "2") + } + if got := ann[AnnotationRecordNames]; got != "www,api" { + t.Errorf("%s = %q, want %q", AnnotationRecordNames, got, "www,api") + } +} + +// TestRecordRecordSetActivityEventWithData_DomainNameAbsentWhenEmpty verifies +// that the domain-name annotation is omitted when DomainName is empty. +func TestRecordRecordSetActivityEventWithData_DomainNameAbsentWhenEmpty(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "rs", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "1.1.1.1"}}}, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, + RecordSetEventData{RecordSet: rs}, + corev1.EventTypeNormal, EventReasonRecordSetProgrammed, ActivityTypeRecordSetProgrammed, "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + if _, ok := ar.calls[0].annotations[AnnotationDomainName]; ok { + t.Errorf("%s should be absent when DomainName is empty", AnnotationDomainName) + } +} + +// TestRecordRecordSetActivityEventWithData_IPAddressesForARecords verifies that +// the ip-addresses annotation is set to a comma-separated list of A record IPs. +func TestRecordRecordSetActivityEventWithData_IPAddressesForARecords(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "rs", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "10.0.0.1"}}, + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "10.0.0.2"}}, + }, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, RecordSetEventData{RecordSet: rs}, + corev1.EventTypeNormal, EventReasonRecordSetProgrammed, ActivityTypeRecordSetProgrammed, "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + got := ar.calls[0].annotations[AnnotationIPAddresses] + want := "10.0.0.1,10.0.0.2" + if got != want { + t.Errorf("%s = %q, want %q", AnnotationIPAddresses, got, want) + } +} + +// TestRecordRecordSetActivityEventWithData_IPAddressesForAAAARecords verifies +// that the ip-addresses annotation is set correctly for AAAA record types. +func TestRecordRecordSetActivityEventWithData_IPAddressesForAAAARecords(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "rs", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "zone"}, + RecordType: dnsv1alpha1.RRTypeAAAA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", AAAA: &dnsv1alpha1.AAAARecordSpec{Content: "2001:db8::1"}}, + {Name: "@", AAAA: &dnsv1alpha1.AAAARecordSpec{Content: "2001:db8::2"}}, + }, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, RecordSetEventData{RecordSet: rs}, + corev1.EventTypeNormal, EventReasonRecordSetProgrammed, ActivityTypeRecordSetProgrammed, "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + got := ar.calls[0].annotations[AnnotationIPAddresses] + want := "2001:db8::1,2001:db8::2" + if got != want { + t.Errorf("%s = %q, want %q", AnnotationIPAddresses, got, want) + } +} + +// TestRecordRecordSetActivityEventWithData_IPAddressesAbsentForNonIPRecords +// verifies that the ip-addresses annotation is omitted for record types that +// are not A or AAAA. +func TestRecordRecordSetActivityEventWithData_IPAddressesAbsentForNonIPRecords(t *testing.T) { + t.Parallel() + + nonIPTypes := []dnsv1alpha1.RRType{ + dnsv1alpha1.RRTypeCNAME, + dnsv1alpha1.RRTypeTXT, + dnsv1alpha1.RRTypeNS, + dnsv1alpha1.RRTypeMX, + } + + for _, rt := range nonIPTypes { + t.Run(string(rt), func(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "rs", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "zone"}, + RecordType: rt, + Records: []dnsv1alpha1.RecordEntry{{Name: "@"}}, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, RecordSetEventData{RecordSet: rs}, + corev1.EventTypeNormal, EventReasonRecordSetProgrammed, ActivityTypeRecordSetProgrammed, "programmed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + if _, ok := ar.calls[0].annotations[AnnotationIPAddresses]; ok { + t.Errorf("%s should be absent for %s record type", AnnotationIPAddresses, rt) + } + }) + } +} + +// TestRecordRecordSetActivityEventWithData_FailReasonAnnotation verifies that +// the failure-reason annotation is set when FailReason is non-empty. +func TestRecordRecordSetActivityEventWithData_FailReasonAnnotation(t *testing.T) { + t.Parallel() + + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{Name: "rs", Namespace: "ns", Generation: 1}, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "zone"}, + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "1.1.1.1"}}}, + }, + } + + fakeRecorder := record.NewFakeRecorder(10) + ar := &annotationCapturingRecorder{FakeRecorder: fakeRecorder} + + recordRecordSetActivityEventWithData(ar, + RecordSetEventData{ + RecordSet: rs, + FailReason: "provider API returned 503", + }, + corev1.EventTypeWarning, + EventReasonRecordSetProgrammingFailed, + ActivityTypeRecordSetProgrammingFailed, + "programming failed", + ) + + if len(ar.calls) != 1 { + t.Fatalf("expected 1 event, got %d", len(ar.calls)) + } + got := ar.calls[0].annotations[AnnotationFailureReason] + want := "provider API returned 503" + if got != want { + t.Errorf("%s = %q, want %q", AnnotationFailureReason, got, want) + } +} + +// TestExtractIPAddresses verifies the extractIPAddresses helper for all covered +// record types, including both A and AAAA, and for non-IP types. +func TestExtractIPAddresses(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + wantIPs []string + }{ + { + name: "A records: extracts content values", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}, + {Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "5.6.7.8"}}, + }, + }, + }, + wantIPs: []string{"1.2.3.4", "5.6.7.8"}, + }, + { + name: "AAAA records: extracts content values", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeAAAA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", AAAA: &dnsv1alpha1.AAAARecordSpec{Content: "::1"}}, + }, + }, + }, + wantIPs: []string{"::1"}, + }, + { + name: "A record with nil A field: skipped", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", A: nil}}, + }, + }, + wantIPs: nil, + }, + { + name: "CNAME: returns nil", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", CNAME: &dnsv1alpha1.CNAMERecordSpec{Content: "target.example.com"}}}, + }, + }, + wantIPs: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := extractIPAddresses(tt.rs) + if len(got) != len(tt.wantIPs) { + t.Fatalf("extractIPAddresses() = %v, want %v", got, tt.wantIPs) + } + for i := range got { + if got[i] != tt.wantIPs[i] { + t.Errorf("extractIPAddresses()[%d] = %q, want %q", i, got[i], tt.wantIPs[i]) + } + } + }) + } +} + +// -------------------------------------------------------------------------- +// Display annotation helper tests +// -------------------------------------------------------------------------- + +// TestExtractCNAMETarget verifies extraction of CNAME targets. +func TestExtractCNAMETarget(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + want string + }{ + { + name: "CNAME record returns target", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "api", CNAME: &dnsv1alpha1.CNAMERecordSpec{Content: "api.internal.example.com"}}, + }, + }, + }, + want: "api.internal.example.com", + }, + { + name: "A record returns empty", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}, + }, + }, + }, + want: "", + }, + { + name: "CNAME with nil spec returns empty", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{{Name: "api", CNAME: nil}}, + }, + }, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := extractCNAMETarget(tt.rs) + if got != tt.want { + t.Errorf("extractCNAMETarget() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestExtractMXHosts verifies extraction of MX hosts with preferences. +func TestExtractMXHosts(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + want string + }{ + { + name: "single MX record", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeMX, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", MX: &dnsv1alpha1.MXRecordSpec{Preference: 10, Exchange: "mail.example.com"}}, + }, + }, + }, + want: "10 mail.example.com", + }, + { + name: "multiple MX records", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeMX, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", MX: &dnsv1alpha1.MXRecordSpec{Preference: 10, Exchange: "mail.example.com"}}, + {Name: "@", MX: &dnsv1alpha1.MXRecordSpec{Preference: 20, Exchange: "mail2.example.com"}}, + }, + }, + }, + want: "10 mail.example.com, 20 mail2.example.com", + }, + { + name: "non-MX record returns empty", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", A: &dnsv1alpha1.ARecordSpec{Content: "1.2.3.4"}}}, + }, + }, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := extractMXHosts(tt.rs) + if got != tt.want { + t.Errorf("extractMXHosts() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBuildFQDN verifies FQDN construction from record name and zone domain. +func TestBuildFQDN(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + recordName string + zoneDomain string + want string + }{ + { + name: "subdomain", + recordName: "www", + zoneDomain: "example.com", + want: "www.example.com", + }, + { + name: "apex record", + recordName: "@", + zoneDomain: "example.com", + want: "example.com", + }, + { + name: "nested subdomain", + recordName: "api.v2", + zoneDomain: "example.com", + want: "api.v2.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := buildFQDN(tt.recordName, tt.zoneDomain) + if got != tt.want { + t.Errorf("buildFQDN(%q, %q) = %q, want %q", tt.recordName, tt.zoneDomain, got, tt.want) + } + }) + } +} + +// TestComputeDisplayName verifies FQDN computation for display annotations. +func TestComputeDisplayName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + zoneDomain string + want string + }{ + { + name: "single record", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + Records: []dnsv1alpha1.RecordEntry{{Name: "www"}}, + }, + }, + zoneDomain: "example.com", + want: "www.example.com", + }, + { + name: "multiple records same name", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www"}, + {Name: "www"}, + }, + }, + }, + zoneDomain: "example.com", + want: "www.example.com", + }, + { + name: "multiple records different names", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www"}, + {Name: "api"}, + }, + }, + }, + zoneDomain: "example.com", + want: "www.example.com, api.example.com", + }, + { + name: "apex record", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + Records: []dnsv1alpha1.RecordEntry{{Name: "@"}}, + }, + }, + zoneDomain: "example.com", + want: "example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := computeDisplayName(tt.rs, tt.zoneDomain) + if got != tt.want { + t.Errorf("computeDisplayName() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestComputeDisplayValue verifies display value computation for different record types. +func TestComputeDisplayValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rs *dnsv1alpha1.DNSRecordSet + want string + }{ + { + name: "A record single IP", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{{Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}}, + }, + }, + want: "192.0.2.10", + }, + { + name: "A record multiple IPs", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeA, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.10"}}, + {Name: "www", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.11"}}, + }, + }, + }, + want: "192.0.2.10, 192.0.2.11", + }, + { + name: "CNAME record", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{{Name: "api", CNAME: &dnsv1alpha1.CNAMERecordSpec{Content: "api.internal.example.com"}}}, + }, + }, + want: "api.internal.example.com", + }, + { + name: "MX records", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeMX, + Records: []dnsv1alpha1.RecordEntry{ + {Name: "@", MX: &dnsv1alpha1.MXRecordSpec{Preference: 10, Exchange: "mail.example.com"}}, + }, + }, + }, + want: "10 mail.example.com", + }, + { + name: "TXT record short", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeTXT, + Records: []dnsv1alpha1.RecordEntry{{Name: "@", TXT: &dnsv1alpha1.TXTRecordSpec{Content: "v=spf1 include:_spf.example.com ~all"}}}, + }, + }, + want: "\"v=spf1 include:_spf.example.com ~all\"", + }, + { + name: "NS record", + rs: &dnsv1alpha1.DNSRecordSet{ + Spec: dnsv1alpha1.DNSRecordSetSpec{ + RecordType: dnsv1alpha1.RRTypeNS, + Records: []dnsv1alpha1.RecordEntry{{Name: "sub", NS: &dnsv1alpha1.NSRecordSpec{Content: "ns1.example.com"}}}, + }, + }, + want: "ns1.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := computeDisplayValue(tt.rs) + if got != tt.want { + t.Errorf("computeDisplayValue() = %q, want %q", got, tt.want) + } + }) + } +} + +// -------------------------------------------------------------------------- +// test helpers +// -------------------------------------------------------------------------- + +// annotatedCall records a single invocation of AnnotatedEventf so tests can +// assert on the annotation map and event fields without inspecting a raw string +// channel. +type annotatedCall struct { + annotations map[string]string + eventType string + reason string + messageFmt string +} + +// annotationCapturingRecorder wraps record.FakeRecorder and additionally +// captures the annotations passed to AnnotatedEventf so tests can assert on them. +type annotationCapturingRecorder struct { + *record.FakeRecorder + calls []annotatedCall +} + +func (r *annotationCapturingRecorder) AnnotatedEventf( + object runtime.Object, + annotations map[string]string, + eventType, reason, messageFmt string, + args ...interface{}, +) { + // Copy annotations so the caller cannot mutate our captured copy. + copied := make(map[string]string, len(annotations)) + for k, v := range annotations { + copied[k] = v + } + r.calls = append(r.calls, annotatedCall{ + annotations: copied, + eventType: eventType, + reason: reason, + messageFmt: messageFmt, + }) + // Also delegate to FakeRecorder so the Events channel receives something. + r.FakeRecorder.AnnotatedEventf(object, annotations, eventType, reason, messageFmt, args...) +} diff --git a/test/e2e/display-annotations/chainsaw-test.yaml b/test/e2e/display-annotations/chainsaw-test.yaml new file mode 100644 index 0000000..ec94c3d --- /dev/null +++ b/test/e2e/display-annotations/chainsaw-test.yaml @@ -0,0 +1,239 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: dns-operator-display-annotations +spec: + description: | + Verifies that DNSRecordSet resources get display annotations set AND are + replicated to the downstream cluster. This test catches a bug where the + controller returned early after patching annotations, skipping downstream + replication. + clusters: + upstream: + kubeconfig: ../kubeconfig-upstream + downstream: + kubeconfig: ../kubeconfig-downstream + cluster: upstream + steps: + - name: Prereq - create DNSZoneClass in upstream + try: + - create: + cluster: upstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + metadata: + name: powerdns-annotations-test + spec: + controllerName: powerdns + nameServerPolicy: + mode: Static + static: + servers: + - ns1.example.com + - ns2.example.com + defaults: + defaultTTL: 300 + - assert: + cluster: upstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + metadata: + name: powerdns-annotations-test + + - name: Prereq - create DNSZoneClass in downstream + try: + - create: + cluster: downstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + metadata: + name: powerdns-annotations-test + spec: + controllerName: powerdns + nameServerPolicy: + mode: Static + static: + servers: + - ns1.example.com + - ns2.example.com + defaults: + defaultTTL: 300 + - assert: + cluster: downstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + metadata: + name: powerdns-annotations-test + + - name: Create DNSZone upstream + try: + - create: + cluster: upstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZone + metadata: + name: annotations-test-zone + spec: + domainName: annotations-test.example.com + dnsZoneClassName: powerdns-annotations-test + - assert: + cluster: upstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZone + metadata: + name: annotations-test-zone + + - name: Wait for DNSZone to be ready + try: + - assert: + cluster: upstream + timeout: 60s + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZone + metadata: + name: annotations-test-zone + status: + conditions: + - type: Accepted + status: "True" + + - name: Create DNSRecordSet upstream (A record) + try: + - create: + cluster: upstream + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + metadata: + name: www-annotations-test + spec: + dnsZoneRef: + name: annotations-test-zone + recordType: A + records: + - name: www + ttl: 300 + a: + content: 192.0.2.42 + + - name: Assert display annotations are set on upstream DNSRecordSet + description: | + The controller must set display-name and display-value annotations + for ActivityPolicy templates to render human-friendly summaries. + try: + - assert: + cluster: upstream + timeout: 30s + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + metadata: + name: www-annotations-test + annotations: + dns.networking.miloapis.com/display-name: www.annotations-test.example.com + dns.networking.miloapis.com/display-value: "192.0.2.42" + + - name: Confirm DNSRecordSet replicated downstream + description: | + CRITICAL: This step catches the bug where the controller returned early + after patching annotations, skipping ensureDownstreamRecordSet(). + If annotations are set but downstream replication is missing, this fails. + try: + - script: + cluster: upstream + skipCommandOutput: true + skipLogOutput: true + content: | + kubectl get ns $NAMESPACE -o json + outputs: + - name: downstreamNamespaceName + value: (join('-', ['ns', json_parse($stdout).metadata.uid])) + - assert: + cluster: downstream + timeout: 30s + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + metadata: + name: www-annotations-test + namespace: ($downstreamNamespaceName) + + - name: Verify DNSRecordSet becomes Programmed + try: + - assert: + cluster: upstream + timeout: 60s + resource: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + metadata: + name: www-annotations-test + status: + conditions: + - type: Accepted + status: "True" + - type: Programmed + status: "True" + + - name: Teardown - delete resources + try: + - delete: + cluster: upstream + ref: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + name: www-annotations-test + expect: + - match: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSRecordSet + check: + ($error == null): true + - sleep: + duration: 5s + - delete: + cluster: upstream + ref: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZone + name: annotations-test-zone + expect: + - match: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZone + check: + ($error == null): true + - sleep: + duration: 5s + - delete: + cluster: downstream + ref: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + name: powerdns-annotations-test + expect: + - match: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + check: + ($error == null): true + - delete: + cluster: upstream + ref: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + name: powerdns-annotations-test + expect: + - match: + apiVersion: dns.networking.miloapis.com/v1alpha1 + kind: DNSZoneClass + check: + ($error == null): true