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