From 8203649c5e463ee2e14ea9b2280d4dce577d0d3d Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 18:42:44 +0900 Subject: [PATCH 1/6] feat(endpoint): aggregate record readiness and deletion --- app/operator/cmd/manager/main.go | 1 + ...t.dns.appthrust.io_endpointrecordsets.yaml | 18 +++ app/operator/config/rbac/role.yaml | 6 + ...t.dns.appthrust.io_endpointrecordsets.yaml | 18 +++ deploy/charts/dns-api/templates/rbac.yaml | 6 + docs/design/endpoint-api.md | 18 +++ .../endpointrecordset/controller.go | 144 +++++++++++++++++- .../endpointrecordset/controller_test.go | 131 ++++++++++++++++ .../v1alpha1/endpointrecordset_types.go | 15 ++ 9 files changed, 353 insertions(+), 4 deletions(-) diff --git a/app/operator/cmd/manager/main.go b/app/operator/cmd/manager/main.go index a4385cf..e4fa30a 100644 --- a/app/operator/cmd/manager/main.go +++ b/app/operator/cmd/manager/main.go @@ -53,6 +53,7 @@ import ( // +kubebuilder:rbac:groups=dns.appthrust.io,resources=zoneunits/status,verbs=get;watch;patch;update // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointprovidercapabilities,verbs=get;list;watch // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets,verbs=create;delete;get;list;watch;patch;update +// +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets/finalizers,verbs=update // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets/status,verbs=get;patch;update // +kubebuilder:rbac:groups=endpoint.route53.dns.appthrust.io,resources=endpointrecordsetconversions,verbs=create // +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;update diff --git a/app/operator/config/crd/bases/endpoint.dns.appthrust.io_endpointrecordsets.yaml b/app/operator/config/crd/bases/endpoint.dns.appthrust.io_endpointrecordsets.yaml index cb95758..9926a7c 100644 --- a/app/operator/config/crd/bases/endpoint.dns.appthrust.io_endpointrecordsets.yaml +++ b/app/operator/config/crd/bases/endpoint.dns.appthrust.io_endpointrecordsets.yaml @@ -141,6 +141,12 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + generatedRecordSetCount: + description: |- + GeneratedRecordSetCount is the number of generated RecordSets that are + represented by this status. + format: int32 + type: integer hostnameCount: description: HostnameCount is the number of hostname status entries. format: int32 @@ -357,9 +363,21 @@ spec: rule: self.type != 'AAAA' || has(self.aaaa) || has(self.options) - message: cname is required when type is CNAME rule: self.type != 'CNAME' || has(self.cname) + generation: + description: |- + Generation is the current metadata generation of the generated RecordSet. + It is zero until the generated RecordSet has been observed. + format: int64 + type: integer name: description: Name is the zone-relative DNS owner name. type: string + observedGeneration: + description: |- + ObservedGeneration is the generated RecordSet generation reflected by its + mirrored status conditions. + format: int64 + type: integer ref: description: Ref points to the generated Core RecordSet. properties: diff --git a/app/operator/config/rbac/role.yaml b/app/operator/config/rbac/role.yaml index 6b2ff41..5fc0429 100644 --- a/app/operator/config/rbac/role.yaml +++ b/app/operator/config/rbac/role.yaml @@ -127,6 +127,12 @@ rules: - patch - update - watch +- apiGroups: + - endpoint.dns.appthrust.io + resources: + - endpointrecordsets/finalizers + verbs: + - update - apiGroups: - endpoint.dns.appthrust.io resources: diff --git a/deploy/charts/dns-api/crds/endpoint.dns.appthrust.io_endpointrecordsets.yaml b/deploy/charts/dns-api/crds/endpoint.dns.appthrust.io_endpointrecordsets.yaml index cb95758..9926a7c 100644 --- a/deploy/charts/dns-api/crds/endpoint.dns.appthrust.io_endpointrecordsets.yaml +++ b/deploy/charts/dns-api/crds/endpoint.dns.appthrust.io_endpointrecordsets.yaml @@ -141,6 +141,12 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + generatedRecordSetCount: + description: |- + GeneratedRecordSetCount is the number of generated RecordSets that are + represented by this status. + format: int32 + type: integer hostnameCount: description: HostnameCount is the number of hostname status entries. format: int32 @@ -357,9 +363,21 @@ spec: rule: self.type != 'AAAA' || has(self.aaaa) || has(self.options) - message: cname is required when type is CNAME rule: self.type != 'CNAME' || has(self.cname) + generation: + description: |- + Generation is the current metadata generation of the generated RecordSet. + It is zero until the generated RecordSet has been observed. + format: int64 + type: integer name: description: Name is the zone-relative DNS owner name. type: string + observedGeneration: + description: |- + ObservedGeneration is the generated RecordSet generation reflected by its + mirrored status conditions. + format: int64 + type: integer ref: description: Ref points to the generated Core RecordSet. properties: diff --git a/deploy/charts/dns-api/templates/rbac.yaml b/deploy/charts/dns-api/templates/rbac.yaml index 157063d..bd630fe 100644 --- a/deploy/charts/dns-api/templates/rbac.yaml +++ b/deploy/charts/dns-api/templates/rbac.yaml @@ -135,6 +135,12 @@ rules: - patch - update - watch + - apiGroups: + - endpoint.dns.appthrust.io + resources: + - endpointrecordsets/finalizers + verbs: + - update - apiGroups: - endpoint.dns.appthrust.io resources: diff --git a/docs/design/endpoint-api.md b/docs/design/endpoint-api.md index 7c5ceb2..835b19d 100644 --- a/docs/design/endpoint-api.md +++ b/docs/design/endpoint-api.md @@ -62,6 +62,24 @@ The endpoint controller reconciles `EndpointRecordSet` resources: The endpoint controller does not decide provider-specific record shape. It does not hard-code Route 53 alias hosted zone IDs or Cloudflare TTL behavior. +### Aggregate readiness and deletion + +`EndpointRecordSet.status.conditions` contains generation-aware `Accepted`, +`Programmed`, and `Ready` summaries. `Ready=True` requires every generated +`RecordSet` to have current `status.observedGeneration` and current +`Accepted=True` and `Programmed=True` conditions. Missing, empty, `Unknown`, or +prior-generation child status is not ready. Each generated child status exposes +both its current metadata `generation` and mirrored `observedGeneration` so a +consumer can independently verify the aggregate. + +The endpoint controller holds the +`endpoint.dns.appthrust.io/generated-recordsets` finalizer while generated +`RecordSet` resources exist. On deletion it requests deletion of every generated +child and removes the finalizer only after a label-selected read proves that no +child remains in the Kubernetes API. Provider cleanup failures therefore retain +both the child provider finalizer and the parent endpoint finalizer; parent +disappearance alone is never used as inferred provider cleanup evidence. + ## EndpointProviderCapability `EndpointProviderCapability` is a cluster-scoped discovery object that describes how one Core Provider version participates in endpoint Apps. diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go index 95d16b0..cca102c 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "slices" "sort" "strings" @@ -37,6 +38,7 @@ const ( generatedLabelEndpointRecordSetNamespace = "endpoint.dns.appthrust.io/endpointrecordset-namespace" generatedLabelEndpointRecordSetName = "endpoint.dns.appthrust.io/endpointrecordset-name" route53RecordSetAdoptionAnnotation = "endpoint.dns.appthrust.io/route53-recordset-adoption" + generatedRecordSetsFinalizer = "endpoint.dns.appthrust.io/generated-recordsets" ) type Reconciler struct { @@ -48,6 +50,7 @@ type Reconciler struct { // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets,verbs=get;list;watch;patch;update // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets/status,verbs=get;patch;update +// +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets/finalizers,verbs=update // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointprovidercapabilities,verbs=get;list;watch // +kubebuilder:rbac:groups=endpoint.route53.dns.appthrust.io,resources=endpointrecordsetconversions,verbs=create // +kubebuilder:rbac:groups=dns.appthrust.io,resources=zones,verbs=get;list;watch @@ -67,7 +70,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } if !endpointRecordSet.DeletionTimestamp.IsZero() { - return ctrl.Result{}, r.cleanupForEndpointRecordSet(ctx, recordSetNamespace, endpointRecordSet.Namespace, endpointRecordSet.Name) + return ctrl.Result{}, r.reconcileDeletion(ctx, &endpointRecordSet, recordSetNamespace) + } + if !slices.Contains(endpointRecordSet.Finalizers, generatedRecordSetsFinalizer) { + base := endpointRecordSet.DeepCopy() + endpointRecordSet.Finalizers = append(endpointRecordSet.Finalizers, generatedRecordSetsFinalizer) + if err := r.Patch(ctx, &endpointRecordSet, client.MergeFrom(base)); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } status, desired, err := r.buildStatusAndRecordSets(ctx, &endpointRecordSet, recordSetNamespace) @@ -77,6 +88,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err := r.applyRecordSets(ctx, &endpointRecordSet, recordSetNamespace, desired); err != nil { return ctrl.Result{}, err } + if err := r.refreshGeneratedRecordSetStatus(ctx, &status); err != nil { + return ctrl.Result{}, err + } + setAggregateConditions(&status, endpointRecordSet.Generation) if err := r.patchStatus(ctx, &endpointRecordSet, status); err != nil { return ctrl.Result{}, err } @@ -131,8 +146,12 @@ func (r *Reconciler) buildStatusAndRecordSets(ctx context.Context, endpointRecor } } status.HostnameCount = int32(len(status.Hostnames)) + for _, hostnameStatus := range status.Hostnames { + status.GeneratedRecordSetCount += int32(len(hostnameStatus.RecordSets)) + } if len(status.Hostnames) == 0 { meta.SetStatusCondition(&status.Conditions, metav1.Condition{Type: "Resolved", Status: metav1.ConditionFalse, ObservedGeneration: endpointRecordSet.Generation, Reason: "NoHostname", Message: "EndpointRecordSet has no hostnames."}) + setAggregateConditions(&status, endpointRecordSet.Generation) return status, nil, nil } allResolved := true @@ -148,6 +167,7 @@ func (r *Reconciler) buildStatusAndRecordSets(ctx context.Context, endpointRecor } else { meta.SetStatusCondition(&status.Conditions, metav1.Condition{Type: "Resolved", Status: metav1.ConditionFalse, ObservedGeneration: endpointRecordSet.Generation, Reason: "HostnameNotResolved", Message: "At least one hostname could not be resolved."}) } + setAggregateConditions(&status, endpointRecordSet.Generation) desired := make([]dnsv1alpha1.RecordSet, 0, len(desiredByName)) for _, recordSet := range desiredByName { desired = append(desired, recordSet) @@ -235,6 +255,8 @@ func (r *Reconciler) recordSetsForHostname(ctx context.Context, endpointRecordSe } var existing dnsv1alpha1.RecordSet if err := r.Get(ctx, client.ObjectKey{Namespace: recordSet.Namespace, Name: recordSet.Name}, &existing); err == nil { + item.Generation = existing.Generation + item.ObservedGeneration = existing.Status.ObservedGeneration item.Conditions = existing.Status.Conditions } status.RecordSets = append(status.RecordSets, item) @@ -450,22 +472,136 @@ func (r *Reconciler) applyRecordSets(ctx context.Context, endpointRecordSet *end return nil } +func (r *Reconciler) refreshGeneratedRecordSetStatus(ctx context.Context, status *endpointv1alpha1.EndpointRecordSetStatus) error { + for hostnameIndex := range status.Hostnames { + for recordSetIndex := range status.Hostnames[hostnameIndex].RecordSets { + item := &status.Hostnames[hostnameIndex].RecordSets[recordSetIndex] + var current dnsv1alpha1.RecordSet + if err := r.Get(ctx, client.ObjectKey{Namespace: item.Ref.Namespace, Name: item.Ref.Name}, ¤t); err != nil { + if apierrors.IsNotFound(err) { + item.Generation = 0 + item.ObservedGeneration = 0 + item.Conditions = nil + continue + } + return err + } + item.Generation = current.Generation + item.ObservedGeneration = current.Status.ObservedGeneration + item.Conditions = current.Status.Conditions + } + } + return nil +} + func (r *Reconciler) cleanupForEndpointRecordSet(ctx context.Context, recordSetNamespace, endpointRecordSetNamespace, endpointRecordSetName string) error { + _, err := r.deleteGeneratedRecordSets(ctx, recordSetNamespace, endpointRecordSetNamespace, endpointRecordSetName) + return err +} + +func (r *Reconciler) reconcileDeletion(ctx context.Context, endpointRecordSet *endpointv1alpha1.EndpointRecordSet, recordSetNamespace string) error { + if !slices.Contains(endpointRecordSet.Finalizers, generatedRecordSetsFinalizer) { + return nil + } + remaining, err := r.deleteGeneratedRecordSets(ctx, recordSetNamespace, endpointRecordSet.Namespace, endpointRecordSet.Name) + if err != nil { + return err + } + if remaining > 0 { + status := endpointRecordSet.Status + meta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionFalse, + ObservedGeneration: endpointRecordSet.Generation, + Reason: "Deleting", + Message: fmt.Sprintf("Waiting for %d generated RecordSet(s) to reach terminal deletion.", remaining), + }) + return r.patchStatus(ctx, endpointRecordSet, status) + } + base := endpointRecordSet.DeepCopy() + endpointRecordSet.Finalizers = slices.DeleteFunc(endpointRecordSet.Finalizers, func(finalizer string) bool { + return finalizer == generatedRecordSetsFinalizer + }) + return r.Patch(ctx, endpointRecordSet, client.MergeFrom(base)) +} + +func (r *Reconciler) deleteGeneratedRecordSets(ctx context.Context, recordSetNamespace, endpointRecordSetNamespace, endpointRecordSetName string) (int, error) { var existing dnsv1alpha1.RecordSetList if err := r.List(ctx, &existing, client.InNamespace(recordSetNamespace), client.MatchingLabels{ managedByLabel: managedByValue, generatedLabelEndpointRecordSetNamespace: endpointRecordSetNamespace, generatedLabelEndpointRecordSetName: endpointRecordSetName, }); err != nil { - return err + return 0, err } for _, item := range existing.Items { item := item if err := r.Delete(ctx, &item); err != nil && !apierrors.IsNotFound(err) { - return err + return 0, err } } - return nil + var remaining dnsv1alpha1.RecordSetList + if err := r.List(ctx, &remaining, client.InNamespace(recordSetNamespace), client.MatchingLabels{ + managedByLabel: managedByValue, + generatedLabelEndpointRecordSetNamespace: endpointRecordSetNamespace, + generatedLabelEndpointRecordSetName: endpointRecordSetName, + }); err != nil { + return 0, err + } + return len(remaining.Items), nil +} + +func setAggregateConditions(status *endpointv1alpha1.EndpointRecordSetStatus, generation int64) { + resolved := meta.FindStatusCondition(status.Conditions, "Resolved") + if resolved == nil || resolved.Status != metav1.ConditionTrue || resolved.ObservedGeneration != generation { + setAggregateCondition(status, "Accepted", metav1.ConditionFalse, generation, "NotResolved", "EndpointRecordSet hostnames are not currently resolved.") + setAggregateCondition(status, "Programmed", metav1.ConditionFalse, generation, "NotAccepted", "EndpointRecordSet is not currently accepted.") + setAggregateCondition(status, "Ready", metav1.ConditionFalse, generation, "NotProgrammed", "EndpointRecordSet is not currently programmed.") + return + } + if status.GeneratedRecordSetCount == 0 { + setAggregateCondition(status, "Accepted", metav1.ConditionFalse, generation, "RecordSetsPending", "No generated RecordSets have been observed.") + setAggregateCondition(status, "Programmed", metav1.ConditionFalse, generation, "RecordSetsPending", "No generated RecordSets have been observed.") + setAggregateCondition(status, "Ready", metav1.ConditionFalse, generation, "RecordSetsPending", "No generated RecordSets have been observed.") + return + } + + accepted, acceptedReason, acceptedMessage := aggregateGeneratedCondition(status.Hostnames, string(dnsv1alpha1.ConditionAccepted)) + setAggregateCondition(status, "Accepted", accepted, generation, acceptedReason, acceptedMessage) + programmed, programmedReason, programmedMessage := aggregateGeneratedCondition(status.Hostnames, string(dnsv1alpha1.ConditionProgrammed)) + if accepted != metav1.ConditionTrue && programmed == metav1.ConditionTrue { + programmed = metav1.ConditionFalse + programmedReason = "NotAccepted" + programmedMessage = "At least one generated RecordSet is not currently accepted." + } + setAggregateCondition(status, "Programmed", programmed, generation, programmedReason, programmedMessage) + if accepted == metav1.ConditionTrue && programmed == metav1.ConditionTrue { + setAggregateCondition(status, "Ready", metav1.ConditionTrue, generation, "Ready", "All generated RecordSets are currently accepted and programmed.") + } else { + setAggregateCondition(status, "Ready", metav1.ConditionFalse, generation, "RecordSetsNotReady", "At least one generated RecordSet is not currently accepted and programmed.") + } +} + +func aggregateGeneratedCondition(hostnames []endpointv1alpha1.EndpointRecordSetHostnameStatus, conditionType string) (metav1.ConditionStatus, string, string) { + for _, hostname := range hostnames { + for _, recordSet := range hostname.RecordSets { + if recordSet.Generation == 0 || recordSet.ObservedGeneration != recordSet.Generation { + return metav1.ConditionFalse, "RecordSetStatusStale", fmt.Sprintf("Generated RecordSet %s/%s status is not current.", recordSet.Ref.Namespace, recordSet.Ref.Name) + } + condition := meta.FindStatusCondition(recordSet.Conditions, conditionType) + if condition == nil || condition.ObservedGeneration != recordSet.Generation { + return metav1.ConditionFalse, "RecordSetConditionStale", fmt.Sprintf("Generated RecordSet %s/%s %s condition is missing or stale.", recordSet.Ref.Namespace, recordSet.Ref.Name, conditionType) + } + if condition.Status != metav1.ConditionTrue { + return metav1.ConditionFalse, "RecordSet" + conditionType + "False", fmt.Sprintf("Generated RecordSet %s/%s is not %s.", recordSet.Ref.Namespace, recordSet.Ref.Name, strings.ToLower(conditionType)) + } + } + } + return metav1.ConditionTrue, conditionType, "All generated RecordSets are currently " + strings.ToLower(conditionType) + "." +} + +func setAggregateCondition(status *endpointv1alpha1.EndpointRecordSetStatus, conditionType string, conditionStatus metav1.ConditionStatus, generation int64, reason, message string) { + meta.SetStatusCondition(&status.Conditions, metav1.Condition{Type: conditionType, Status: conditionStatus, ObservedGeneration: generation, Reason: reason, Message: message}) } func (r *Reconciler) patchStatus(ctx context.Context, endpointRecordSet *endpointv1alpha1.EndpointRecordSet, status endpointv1alpha1.EndpointRecordSetStatus) error { diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go b/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go index 1ba7002..96b6eab 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go @@ -3,14 +3,145 @@ package endpointrecordset import ( "context" "encoding/json" + "slices" "testing" dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" endpointv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/endpoint/v1alpha1" route53v1alpha1 "github.com/appthrust/dns-api/pkg/go/api/route53/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +func TestSetAggregateConditionsRequiresEveryChildConditionToBeCurrent(t *testing.T) { + status := endpointv1alpha1.EndpointRecordSetStatus{ + GeneratedRecordSetCount: 2, + Conditions: []metav1.Condition{{Type: "Resolved", Status: metav1.ConditionTrue, ObservedGeneration: 7, Reason: "Resolved"}}, + Hostnames: []endpointv1alpha1.EndpointRecordSetHostnameStatus{{ + Hostname: "reo.appthrust.app", + RecordSets: []endpointv1alpha1.EndpointRecordSetGeneratedRecordSetStatus{ + readyGeneratedRecordSetStatus("apex", 3), + readyGeneratedRecordSetStatus("wildcard", 4), + }, + }}, + } + + setAggregateConditions(&status, 7) + assertEndpointCondition(t, status.Conditions, "Accepted", metav1.ConditionTrue, 7) + assertEndpointCondition(t, status.Conditions, "Programmed", metav1.ConditionTrue, 7) + assertEndpointCondition(t, status.Conditions, "Ready", metav1.ConditionTrue, 7) + + status.Hostnames[0].RecordSets[1].Conditions[1].ObservedGeneration = 3 + setAggregateConditions(&status, 7) + assertEndpointCondition(t, status.Conditions, "Programmed", metav1.ConditionFalse, 7) + assertEndpointCondition(t, status.Conditions, "Ready", metav1.ConditionFalse, 7) + + status.Hostnames[0].RecordSets[1] = readyGeneratedRecordSetStatus("wildcard", 4) + status.Hostnames[0].RecordSets[1].ObservedGeneration = 3 + setAggregateConditions(&status, 7) + assertEndpointCondition(t, status.Conditions, "Accepted", metav1.ConditionFalse, 7) + assertEndpointCondition(t, status.Conditions, "Programmed", metav1.ConditionFalse, 7) + assertEndpointCondition(t, status.Conditions, "Ready", metav1.ConditionFalse, 7) +} + +func TestSetAggregateConditionsDoesNotTreatEmptyChildrenAsReady(t *testing.T) { + status := endpointv1alpha1.EndpointRecordSetStatus{ + Conditions: []metav1.Condition{{Type: "Resolved", Status: metav1.ConditionTrue, ObservedGeneration: 2, Reason: "Resolved"}}, + } + + setAggregateConditions(&status, 2) + assertEndpointCondition(t, status.Conditions, "Accepted", metav1.ConditionFalse, 2) + assertEndpointCondition(t, status.Conditions, "Programmed", metav1.ConditionFalse, 2) + assertEndpointCondition(t, status.Conditions, "Ready", metav1.ConditionFalse, 2) +} + +func TestReconcileDeletionRetainsFinalizerUntilGeneratedRecordSetsAreGone(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + if err := endpointv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add endpoint scheme: %v", err) + } + if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add dns scheme: %v", err) + } + endpointRecordSet := &endpointv1alpha1.EndpointRecordSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: "dns-system", Name: "reo", Finalizers: []string{generatedRecordSetsFinalizer}}, + } + generated := &dnsv1alpha1.RecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "dns-system", + Name: "reo-apex", + Finalizers: []string{"dns.appthrust.io/provider-cleanup"}, + Labels: map[string]string{ + managedByLabel: managedByValue, + generatedLabelEndpointRecordSetNamespace: endpointRecordSet.Namespace, + generatedLabelEndpointRecordSetName: endpointRecordSet.Name, + }, + }, + } + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(endpointRecordSet).WithObjects(endpointRecordSet, generated).Build() + reconciler := &Reconciler{Client: k8sClient, Scheme: scheme, RecordSetNamespace: "dns-system"} + + if err := k8sClient.Delete(ctx, endpointRecordSet); err != nil { + t.Fatalf("delete EndpointRecordSet: %v", err) + } + var deleting endpointv1alpha1.EndpointRecordSet + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(endpointRecordSet), &deleting); err != nil { + t.Fatalf("get deleting EndpointRecordSet: %v", err) + } + if err := reconciler.reconcileDeletion(ctx, &deleting, "dns-system"); err != nil { + t.Fatalf("reconcile deletion with child: %v", err) + } + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(endpointRecordSet), &deleting); err != nil { + t.Fatalf("get retained EndpointRecordSet: %v", err) + } + if !slices.Contains(deleting.Finalizers, generatedRecordSetsFinalizer) { + t.Fatal("generated RecordSet finalizer was removed before child terminal deletion") + } + + var deletingChild dnsv1alpha1.RecordSet + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(generated), &deletingChild); err != nil { + t.Fatalf("get deleting child: %v", err) + } + deletingChild.Finalizers = nil + if err := k8sClient.Update(ctx, &deletingChild); err != nil { + t.Fatalf("remove child finalizer: %v", err) + } + if err := reconciler.reconcileDeletion(ctx, &deleting, "dns-system"); err != nil { + t.Fatalf("reconcile deletion after child removal: %v", err) + } + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(endpointRecordSet), &deleting); err == nil { + t.Fatal("EndpointRecordSet still exists after generated children reached terminal deletion") + } +} + +func readyGeneratedRecordSetStatus(name string, generation int64) endpointv1alpha1.EndpointRecordSetGeneratedRecordSetStatus { + return endpointv1alpha1.EndpointRecordSetGeneratedRecordSetStatus{ + Ref: dnsv1alpha1.ObjectReference{Namespace: "dns-system", Name: name}, + Generation: generation, + ObservedGeneration: generation, + Conditions: []metav1.Condition{ + {Type: string(dnsv1alpha1.ConditionAccepted), Status: metav1.ConditionTrue, ObservedGeneration: generation, Reason: "Accepted"}, + {Type: string(dnsv1alpha1.ConditionProgrammed), Status: metav1.ConditionTrue, ObservedGeneration: generation, Reason: "Programmed"}, + }, + } +} + +func assertEndpointCondition(t *testing.T, conditions []metav1.Condition, conditionType string, status metav1.ConditionStatus, generation int64) { + t.Helper() + for _, condition := range conditions { + if condition.Type == conditionType { + if condition.Status != status || condition.ObservedGeneration != generation { + t.Fatalf("condition %s = (%s, generation %d), want (%s, generation %d)", conditionType, condition.Status, condition.ObservedGeneration, status, generation) + } + return + } + } + t.Fatalf("condition %s not found", conditionType) +} + func TestRecordSetFromFragmentAddsRoute53AdoptionWhenZoneOptedIn(t *testing.T) { reconciler := &Reconciler{} endpointRecordSet := &endpointv1alpha1.EndpointRecordSet{ diff --git a/pkg/go/api/endpoint/v1alpha1/endpointrecordset_types.go b/pkg/go/api/endpoint/v1alpha1/endpointrecordset_types.go index 5bd16a4..bd76470 100644 --- a/pkg/go/api/endpoint/v1alpha1/endpointrecordset_types.go +++ b/pkg/go/api/endpoint/v1alpha1/endpointrecordset_types.go @@ -59,6 +59,11 @@ type EndpointRecordSetStatus struct { // +optional HostnameCount int32 `json:"hostnameCount,omitempty"` + // GeneratedRecordSetCount is the number of generated RecordSets that are + // represented by this status. + // +optional + GeneratedRecordSetCount int32 `json:"generatedRecordSetCount,omitempty"` + // Hostnames contains per-hostname resolution and generated RecordSet status. // +optional // +listType=map @@ -116,6 +121,16 @@ type EndpointRecordSetGeneratedRecordSetStatus struct { // Type is the DNS record type. Type EndpointRecordSetType `json:"type"` + // Generation is the current metadata generation of the generated RecordSet. + // It is zero until the generated RecordSet has been observed. + // +optional + Generation int64 `json:"generation,omitempty"` + + // ObservedGeneration is the generated RecordSet generation reflected by its + // mirrored status conditions. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // Fragment is the Provider-converted RecordSet.spec fragment. // +optional Fragment *RecordSetSpecFragment `json:"fragment,omitempty"` From 9641b83acc119765ef8928fb25a3b8fbe5df897a Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 18:53:31 +0900 Subject: [PATCH 2/6] test(endpoint): qualify shared appthrust zone policy --- docs/design/core-api.md | 9 + .../endpointrecordset/controller.go | 4 + .../endpointrecordset/controller_test.go | 154 ++++++++++++++++++ internal/go/core/declaration/policy_test.go | 83 ++++++++++ .../route53/conversion/handler_test.go | 31 ++++ 5 files changed, 281 insertions(+) create mode 100644 internal/go/core/declaration/policy_test.go diff --git a/docs/design/core-api.md b/docs/design/core-api.md index e255f5a..3b4a834 100644 --- a/docs/design/core-api.md +++ b/docs/design/core-api.md @@ -188,6 +188,15 @@ For cross-namespace access, `records` is required. When a shared zone is exposed `allowedRecordSets` is cross-namespace policy only. If a `RecordSet` references a `Zone` in the same namespace, `allowedRecordSets` is not evaluated. Same namespace is treated as one trust boundary. This allows application engineers to own a custom-domain `Zone` and manage `RecordSet` resources in the same namespace. Shared zones are placed in a platform namespace and limited with `allowedRecordSets`. +For a shared parent zone, the platform root-record owner and generated endpoint +writer must use different namespaces. A namespace-label selector, a full-match +record-name pattern such as one Organization label or `*.`, and +`types: [A, AAAA]` can then grant only Organization endpoint aliases. Parent +apex records, CAA, TXT, delegated NS, and deeper names remain outside that +grant; provider-owned SOA is not a tenant `RecordSet` type. Co-locating the +endpoint writer with the `Zone` would intentionally bypass this policy and is +therefore not a valid shared-zone topology. + Admission validates only the object-local shape of `allowedRecordSets`: required fields, selector syntax, record name pattern syntax, and record type values. It does not reject a `RecordSet` create/update because the current `Zone`, `RecordSet` namespace labels, record name, or record type are outside the policy. The Core ZoneUnit Controller evaluates the saved objects and sets `RecordSet.status.conditions[Accepted]` to `False`, reason `NotAllowedByZone`, when a cross-namespace `RecordSet` is not allowed by its referenced `Zone`. When updating `Zone.spec.allowedRecordSets`, admission does not list existing `RecordSet` resources and does not reject policy shrink. If the new policy makes existing cross-namespace `RecordSet` resources disallowed, the Core ZoneUnit Controller re-evaluates them and returns `Accepted=False`, reason `NotAllowedByZone`. Same-namespace `RecordSet` resources are not affected by `allowedRecordSets`. diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go index cca102c..dda09c5 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go @@ -46,6 +46,7 @@ type Reconciler struct { Scheme *runtime.Scheme RESTConfig *rest.Config RecordSetNamespace string + conversionFunc func(context.Context, *endpointv1alpha1.EndpointProviderCapability, endpointv1alpha1.EndpointRecordSetConversionInput) ([]endpointv1alpha1.RecordSetSpecFragment, string, error) } // +kubebuilder:rbac:groups=endpoint.dns.appthrust.io,resources=endpointrecordsets,verbs=get;list;watch;patch;update @@ -269,6 +270,9 @@ func (r *Reconciler) recordSetsForHostname(ctx context.Context, endpointRecordSe } func (r *Reconciler) convertEndpointRecordSet(ctx context.Context, capability *endpointv1alpha1.EndpointProviderCapability, input endpointv1alpha1.EndpointRecordSetConversionInput) ([]endpointv1alpha1.RecordSetSpecFragment, string, error) { + if r.conversionFunc != nil { + return r.conversionFunc(ctx, capability, input) + } if r.RESTConfig == nil { return nil, "EndpointRecordSet conversion API is configured but no REST config is available", nil } diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go b/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go index 96b6eab..ec93e5d 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/controller_test.go @@ -9,12 +9,166 @@ import ( dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" endpointv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/endpoint/v1alpha1" route53v1alpha1 "github.com/appthrust/dns-api/pkg/go/api/route53/v1alpha1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +func TestEndpointRecordSetApexWildcardAliasLifecycleSurvivesControllerRestart(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("add core scheme: %v", err) + } + if err := endpointv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add endpoint scheme: %v", err) + } + if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add dns scheme: %v", err) + } + endpointRecordSet := &endpointv1alpha1.EndpointRecordSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: "allocation-system", Name: "reo"}, + Spec: endpointv1alpha1.EndpointRecordSetSpec{ + Hostnames: []string{"reo.appthrust.app", "*.reo.appthrust.app"}, + Targets: []endpointv1alpha1.EndpointTarget{{Type: endpointv1alpha1.EndpointTargetTypeHostname, Value: "k8s-public-123456.ap-northeast-1.elb.amazonaws.com"}}, + }, + } + zone := &dnsv1alpha1.Zone{ + ObjectMeta: metav1.ObjectMeta{Namespace: "platform-root-dns", Name: "appthrust-app"}, + Spec: dnsv1alpha1.ZoneSpec{ + DomainName: "appthrust.app", + Provider: route53v1alpha1.ProviderRef, + AllowedRecordSets: []dnsv1alpha1.AllowedRecordSet{{ + Namespaces: dnsv1alpha1.AllowedRecordSetNamespaces{Selector: metav1.LabelSelector{MatchLabels: map[string]string{"appthrust.io/dns-writer": "organization-endpoints"}}}, + Records: []dnsv1alpha1.AllowedRecord{{ + Name: dnsv1alpha1.RecordNamePolicy{Pattern: `(reo|\*\.reo)`}, + Types: []dnsv1alpha1.RecordType{dnsv1alpha1.RecordTypeA, dnsv1alpha1.RecordTypeAAAA}, + }}, + }}, + }, + } + capability := &endpointv1alpha1.EndpointProviderCapability{ + ObjectMeta: metav1.ObjectMeta{Name: "route53-v1alpha1"}, + Spec: endpointv1alpha1.EndpointProviderCapabilitySpec{ + Provider: route53v1alpha1.ProviderRef, + Conversion: endpointv1alpha1.EndpointRecordSetConversionAPI{ + Group: "endpoint.route53.dns.appthrust.io", Resource: "endpointrecordsetconversions", + }, + }, + } + writerNamespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "organization-endpoints", Labels: map[string]string{"appthrust.io/dns-writer": "organization-endpoints"}}} + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&endpointv1alpha1.EndpointRecordSet{}, &dnsv1alpha1.RecordSet{}). + WithObjects(endpointRecordSet, zone, capability, writerNamespace). + Build() + conversion := func(_ context.Context, _ *endpointv1alpha1.EndpointProviderCapability, input endpointv1alpha1.EndpointRecordSetConversionInput) ([]endpointv1alpha1.RecordSetSpecFragment, string, error) { + options := runtime.RawExtension{Raw: []byte(`{"alias":{"dnsName":"dualstack.k8s-public-123456.ap-northeast-1.elb.amazonaws.com.","hostedZoneID":"Z14GRHDCWA56QT","evaluateTargetHealth":true}}`)} + return []endpointv1alpha1.RecordSetSpecFragment{ + {Type: endpointv1alpha1.EndpointRecordSetTypeA, Name: input.Name, Options: options}, + {Type: endpointv1alpha1.EndpointRecordSetTypeAAAA, Name: input.Name, Options: options}, + }, "", nil + } + reconcile := func(reconciler *Reconciler) { + t.Helper() + if _, err := reconciler.Reconcile(ctx, ctrlRequest(endpointRecordSet)); err != nil { + t.Fatalf("reconcile EndpointRecordSet: %v", err) + } + } + reconciler := &Reconciler{Client: k8sClient, Scheme: scheme, RecordSetNamespace: writerNamespace.Name, conversionFunc: conversion} + reconcile(reconciler) // establish lifecycle ownership before creating children + reconcile(reconciler) // create apex and wildcard A/AAAA children + + var generated dnsv1alpha1.RecordSetList + if err := k8sClient.List(ctx, &generated, client.InNamespace(writerNamespace.Name)); err != nil { + t.Fatalf("list generated RecordSets: %v", err) + } + if len(generated.Items) != 4 { + t.Fatalf("generated RecordSets = %d, want apex/wildcard A/AAAA", len(generated.Items)) + } + wantNames := map[string]bool{"reo/A": false, "reo/AAAA": false, "*.reo/A": false, "*.reo/AAAA": false} + for index := range generated.Items { + item := &generated.Items[index] + key := item.Spec.Name + "/" + string(item.Spec.Type) + if _, ok := wantNames[key]; !ok { + t.Fatalf("unexpected generated RecordSet %s", key) + } + wantNames[key] = true + if item.Generation == 0 { + item.Generation = 1 // fake client does not apply apiserver generation defaults + if err := k8sClient.Update(ctx, item); err != nil { + t.Fatalf("set generated RecordSet generation: %v", err) + } + } + item.Status.ObservedGeneration = item.Generation + item.Status.Conditions = []metav1.Condition{ + {Type: string(dnsv1alpha1.ConditionAccepted), Status: metav1.ConditionTrue, ObservedGeneration: item.Generation, Reason: "Accepted"}, + {Type: string(dnsv1alpha1.ConditionProgrammed), Status: metav1.ConditionTrue, ObservedGeneration: item.Generation, Reason: "Programmed"}, + } + if err := k8sClient.Status().Update(ctx, item); err != nil { + t.Fatalf("set generated RecordSet status: %v", err) + } + } + for key, found := range wantNames { + if !found { + t.Fatalf("generated RecordSet %s was not created", key) + } + } + + // A fresh reconciler represents a controller restart over persisted children. + restarted := &Reconciler{Client: k8sClient, Scheme: scheme, RecordSetNamespace: writerNamespace.Name, conversionFunc: conversion} + reconcile(restarted) + var got endpointv1alpha1.EndpointRecordSet + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(endpointRecordSet), &got); err != nil { + t.Fatalf("get reconciled EndpointRecordSet: %v", err) + } + assertEndpointCondition(t, got.Status.Conditions, "Accepted", metav1.ConditionTrue, got.Generation) + assertEndpointCondition(t, got.Status.Conditions, "Programmed", metav1.ConditionTrue, got.Generation) + assertEndpointCondition(t, got.Status.Conditions, "Ready", metav1.ConditionTrue, got.Generation) + + if err := k8sClient.Delete(ctx, &got); err != nil { + t.Fatalf("delete EndpointRecordSet: %v", err) + } + reconcile(restarted) + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(endpointRecordSet), &got); !apierrors.IsNotFound(err) { + t.Fatalf("EndpointRecordSet get after terminal child deletion = %v, want NotFound", err) + } + generated = dnsv1alpha1.RecordSetList{} + if err := k8sClient.List(ctx, &generated, client.InNamespace(writerNamespace.Name)); err != nil { + t.Fatalf("list generated RecordSets after deletion: %v", err) + } + if len(generated.Items) != 0 { + t.Fatalf("generated RecordSets after deletion = %d, want 0", len(generated.Items)) + } +} + +func TestSortedZonesUsesLongestSuffix(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add dns scheme: %v", err) + } + parent := &dnsv1alpha1.Zone{ObjectMeta: metav1.ObjectMeta{Namespace: "dns", Name: "parent"}, Spec: dnsv1alpha1.ZoneSpec{DomainName: "appthrust.app"}} + organization := &dnsv1alpha1.Zone{ObjectMeta: metav1.ObjectMeta{Namespace: "dns", Name: "organization"}, Spec: dnsv1alpha1.ZoneSpec{DomainName: "reo.appthrust.app"}} + reconciler := &Reconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(parent, organization).Build()} + + zones, err := reconciler.sortedZones(ctx, "api.reo.appthrust.app") + if err != nil { + t.Fatalf("sortedZones: %v", err) + } + if len(zones) != 2 || zones[0].Name != organization.Name || zones[1].Name != parent.Name { + t.Fatalf("sorted zones = %#v, want organization before parent", zones) + } +} + +func ctrlRequest(object client.Object) ctrl.Request { + return ctrl.Request{NamespacedName: client.ObjectKeyFromObject(object)} +} + func TestSetAggregateConditionsRequiresEveryChildConditionToBeCurrent(t *testing.T) { status := endpointv1alpha1.EndpointRecordSetStatus{ GeneratedRecordSetCount: 2, diff --git a/internal/go/core/declaration/policy_test.go b/internal/go/core/declaration/policy_test.go new file mode 100644 index 0000000..319afe8 --- /dev/null +++ b/internal/go/core/declaration/policy_test.go @@ -0,0 +1,83 @@ +package declaration + +import ( + "context" + "testing" + + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestSharedParentZonePolicyAllowsOnlyOrganizationAliasRecords(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("add core scheme: %v", err) + } + writerNamespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "organization-endpoints", + Labels: map[string]string{"appthrust.io/dns-writer": "organization-endpoints"}, + }} + tenantNamespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "tenant-project"}} + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(writerNamespace, tenantNamespace).Build() + zone := &dnsv1alpha1.Zone{ + ObjectMeta: metav1.ObjectMeta{Namespace: "platform-root-dns", Name: "appthrust-app"}, + Spec: dnsv1alpha1.ZoneSpec{ + DomainName: "appthrust.app", + AllowedRecordSets: []dnsv1alpha1.AllowedRecordSet{{ + Namespaces: dnsv1alpha1.AllowedRecordSetNamespaces{Selector: metav1.LabelSelector{MatchLabels: map[string]string{"appthrust.io/dns-writer": "organization-endpoints"}}}, + Records: []dnsv1alpha1.AllowedRecord{{ + Name: dnsv1alpha1.RecordNamePolicy{Pattern: `([a-z0-9]([-a-z0-9]*[a-z0-9])?|\*\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)`}, + Types: []dnsv1alpha1.RecordType{dnsv1alpha1.RecordTypeA, dnsv1alpha1.RecordTypeAAAA}, + }}, + }}, + }, + } + + tests := []struct { + name string + namespace string + recordName string + recordType dnsv1alpha1.RecordType + allowed bool + }{ + {name: "organization apex alias A", namespace: writerNamespace.Name, recordName: "reo", recordType: dnsv1alpha1.RecordTypeA, allowed: true}, + {name: "organization wildcard alias AAAA", namespace: writerNamespace.Name, recordName: "*.reo", recordType: dnsv1alpha1.RecordTypeAAAA, allowed: true}, + {name: "tenant cannot forge organization apex", namespace: tenantNamespace.Name, recordName: "reo", recordType: dnsv1alpha1.RecordTypeA}, + {name: "parent apex", recordName: "@", recordType: dnsv1alpha1.RecordTypeA}, + {name: "root CAA", recordName: "@", recordType: dnsv1alpha1.RecordTypeCAA}, + {name: "root TXT", recordName: "@", recordType: dnsv1alpha1.RecordTypeTXT}, + {name: "delegated NS", recordName: "reo", recordType: dnsv1alpha1.RecordTypeNS}, + {name: "deeper record", recordName: "api.reo", recordType: dnsv1alpha1.RecordTypeA}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + namespace := tt.namespace + if namespace == "" { + namespace = writerNamespace.Name + } + recordSet := &dnsv1alpha1.RecordSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "candidate"}, + Spec: dnsv1alpha1.RecordSetSpec{Name: tt.recordName, Type: tt.recordType}, + } + allowed, _ := RecordSetAllowedByZone(context.Background(), client, recordSet, zone) + if allowed != tt.allowed { + t.Fatalf("RecordSetAllowedByZone(%q, %q) = %t, want %t", tt.recordName, tt.recordType, allowed, tt.allowed) + } + }) + } +} + +func TestSharedParentZoneRootOwnerUsesSeparateNamespaceTrustBoundary(t *testing.T) { + zone := &dnsv1alpha1.Zone{ObjectMeta: metav1.ObjectMeta{Namespace: "platform-root-dns", Name: "appthrust-app"}} + rootCAA := &dnsv1alpha1.RecordSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: zone.Namespace, Name: "root-caa"}, + Spec: dnsv1alpha1.RecordSetSpec{Name: "@", Type: dnsv1alpha1.RecordTypeCAA}, + } + allowed, _ := RecordSetAllowedByZone(context.Background(), fake.NewClientBuilder().Build(), rootCAA, zone) + if !allowed { + t.Fatal("same-namespace root owner must remain independent from cross-namespace Organization endpoint grants") + } +} diff --git a/internal/go/providers/route53/conversion/handler_test.go b/internal/go/providers/route53/conversion/handler_test.go index 12ccea3..138e8b2 100644 --- a/internal/go/providers/route53/conversion/handler_test.go +++ b/internal/go/providers/route53/conversion/handler_test.go @@ -72,6 +72,37 @@ func TestHandlerConvertsELBHostnameToAliasAAndAAAA(t *testing.T) { } } +func TestConvertOrganizationApexAndWildcardToAliasAAndAAAA(t *testing.T) { + for _, recordName := range []string{"reo", "*.reo"} { + t.Run(recordName, func(t *testing.T) { + fragments, result := convertEndpoint(endpointv1alpha1.EndpointRecordSetConversionInput{ + Hostname: recordName + ".appthrust.app", + Name: recordName, + Zone: endpointv1alpha1.EndpointRecordSetConversionZone{DomainName: "appthrust.app"}, + Targets: []endpointv1alpha1.EndpointTarget{{ + Type: endpointv1alpha1.EndpointTargetTypeHostname, + Value: "k8s-public-123456.ap-northeast-1.elb.amazonaws.com", + }}, + }) + if result.Status != "Success" || len(fragments) != 2 { + t.Fatalf("conversion = (%#v, %#v), want two successful fragments", fragments, result) + } + if fragments[0].Name != recordName || fragments[0].Type != endpointv1alpha1.EndpointRecordSetTypeA || fragments[1].Name != recordName || fragments[1].Type != endpointv1alpha1.EndpointRecordSetTypeAAAA { + t.Fatalf("fragments = %#v, want %s A/AAAA", fragments, recordName) + } + for _, fragment := range fragments { + var options route53v1alpha1.Route53RecordSetOptions + if err := json.Unmarshal(fragment.Options.Raw, &options); err != nil { + t.Fatalf("decode %s options: %v", fragment.Type, err) + } + if options.Alias == nil || options.Alias.DNSName != "dualstack.k8s-public-123456.ap-northeast-1.elb.amazonaws.com." || options.Alias.HostedZoneID != "Z14GRHDCWA56QT" { + t.Fatalf("%s alias = %#v", fragment.Type, options.Alias) + } + } + }) + } +} + func TestHandlerPreservesExistingELBDualstackAliasPrefix(t *testing.T) { review := endpointconversionv1alpha1.EndpointRecordSetConversion{ TypeMeta: metav1.TypeMeta{APIVersion: GroupName + "/" + Version, Kind: "EndpointRecordSetConversion"}, From 88bf199e209c438a9128bf9fd5d4b948a7538705 Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 21:15:14 +0900 Subject: [PATCH 3/6] chore: prepare v0.2.6 release --- deploy/charts/dns-api/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/charts/dns-api/Chart.yaml b/deploy/charts/dns-api/Chart.yaml index 8bef69f..57063d5 100644 --- a/deploy/charts/dns-api/Chart.yaml +++ b/deploy/charts/dns-api/Chart.yaml @@ -2,6 +2,6 @@ apiVersion: v2 name: dns-api description: Kubernetes DNS API controller for Route 53 and Cloudflare type: application -version: 0.2.5 -appVersion: "0.2.5" +version: 0.2.6 +appVersion: "0.2.6" kubeVersion: ">=1.22.0-0" From d4edede39202680668e548d254d2a0206ee5d55f Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 21:18:07 +0900 Subject: [PATCH 4/6] feat(controller): isolate central endpoint sources --- Taskfile.yml | 6 ++++ app/operator/cmd/manager/main.go | 36 +++++++++++-------- .../charts/dns-api/templates/deployment.yaml | 2 ++ deploy/charts/dns-api/templates/rbac.yaml | 8 ++++- deploy/charts/dns-api/values.yaml | 6 ++++ scripts/check-central-profile.sh | 27 ++++++++++++++ 6 files changed, 70 insertions(+), 15 deletions(-) create mode 100755 scripts/check-central-profile.sh diff --git a/Taskfile.yml b/Taskfile.yml index d18b491..ad4d371 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -312,8 +312,14 @@ tasks: cmds: - task: controller-chart:lint - task: controller-chart:template + - task: controller-chart:central-profile-check - task: controller-chart:package + controller-chart:central-profile-check: + desc: Verify the central DNS profile cannot discover Gateway or Service sources. + cmds: + - ./scripts/check-central-profile.sh deploy/charts/dns-api + controller:rollout-status: desc: Wait for the dns-api controller Deployment rollout. vars: diff --git a/app/operator/cmd/manager/main.go b/app/operator/cmd/manager/main.go index e4fa30a..6857aa9 100644 --- a/app/operator/cmd/manager/main.go +++ b/app/operator/cmd/manager/main.go @@ -80,6 +80,8 @@ func main() { var metricsAddr string var probeAddr string var leaderElection bool + var gatewayEndpointControllerEnabled bool + var serviceEndpointControllerEnabled bool endpointRecordSetNamespace := envOrDefault("ENDPOINT_RECORDSET_NAMESPACE", "") route53ControllerName := envOrDefault("ROUTE53_CONTROLLER_NAME", route53zoneunit.DefaultControllerName) route53ProviderName := envOrDefault("ROUTE53_PROVIDER_NAME", route53v1alpha1.ProviderName) @@ -91,6 +93,8 @@ func main() { flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&leaderElection, "leader-elect", false, "Enable leader election for controller manager.") + flag.BoolVar(&gatewayEndpointControllerEnabled, "gateway-endpoint-controller-enabled", true, "Watch Gateway API resources and synthesize EndpointRecordSets from HTTPRoutes.") + flag.BoolVar(&serviceEndpointControllerEnabled, "service-endpoint-controller-enabled", true, "Watch Services and synthesize EndpointRecordSets from their endpoint annotations.") flag.StringVar(&endpointRecordSetNamespace, "endpoint-recordset-namespace", endpointRecordSetNamespace, "Namespace where generated EndpointRecordSet and RecordSet resources are stored. Defaults to the source namespace.") flag.StringVar(&route53ControllerName, "route53-controller-name", route53ControllerName, "ZoneClass.spec.controllerName handled by the Route 53 controller.") flag.StringVar(&route53ProviderName, "route53-provider-name", route53ProviderName, "Provider.metadata.name handled by the Route 53 controller.") @@ -200,22 +204,26 @@ func main() { os.Exit(1) } - if err := (&gatewayendpointcontroller.Reconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - EndpointRecordSetNamespace: endpointRecordSetNamespace, - }).SetupWithManager(mgr); err != nil { - ctrl.Log.Error(err, "unable to set up Gateway Endpoint controller") - os.Exit(1) + if gatewayEndpointControllerEnabled { + if err := (&gatewayendpointcontroller.Reconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + EndpointRecordSetNamespace: endpointRecordSetNamespace, + }).SetupWithManager(mgr); err != nil { + ctrl.Log.Error(err, "unable to set up Gateway Endpoint controller") + os.Exit(1) + } } - if err := (&serviceendpointcontroller.Reconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - EndpointRecordSetNamespace: endpointRecordSetNamespace, - }).SetupWithManager(mgr); err != nil { - ctrl.Log.Error(err, "unable to set up Service Endpoint controller") - os.Exit(1) + if serviceEndpointControllerEnabled { + if err := (&serviceendpointcontroller.Reconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + EndpointRecordSetNamespace: endpointRecordSetNamespace, + }).SetupWithManager(mgr); err != nil { + ctrl.Log.Error(err, "unable to set up Service Endpoint controller") + os.Exit(1) + } } if err := corewebhook.SetupCoreValidationWebhookWithManager(mgr); err != nil { diff --git a/deploy/charts/dns-api/templates/deployment.yaml b/deploy/charts/dns-api/templates/deployment.yaml index d210af8..a2dea37 100644 --- a/deploy/charts/dns-api/templates/deployment.yaml +++ b/deploy/charts/dns-api/templates/deployment.yaml @@ -39,6 +39,8 @@ spec: args: - --leader-elect={{ .Values.leaderElection.enabled }} - --endpoint-recordset-namespace={{ .Values.endpoint.recordSetNamespace }} + - --gateway-endpoint-controller-enabled={{ .Values.gatewayEndpoint.enabled }} + - --service-endpoint-controller-enabled={{ .Values.serviceEndpoint.enabled }} {{- range .Values.extraArgs }} - {{ . | quote }} {{- end }} diff --git a/deploy/charts/dns-api/templates/rbac.yaml b/deploy/charts/dns-api/templates/rbac.yaml index bd630fe..349c6aa 100644 --- a/deploy/charts/dns-api/templates/rbac.yaml +++ b/deploy/charts/dns-api/templates/rbac.yaml @@ -17,17 +17,20 @@ rules: - "" resources: - namespaces - - services - secrets verbs: - get - list - watch + {{- if .Values.serviceEndpoint.enabled }} - apiGroups: - "" resources: - services verbs: + - get + - list + - watch - update - apiGroups: - "" @@ -35,6 +38,7 @@ rules: - services/finalizers verbs: - update + {{- end }} - apiGroups: - cloudflare.dns.appthrust.io resources: @@ -155,6 +159,7 @@ rules: - endpointrecordsetconversions verbs: - create + {{- if .Values.gatewayEndpoint.enabled }} - apiGroups: - gateway.networking.k8s.io resources: @@ -178,6 +183,7 @@ rules: - httproutes/finalizers verbs: - update + {{- end }} - apiGroups: - route53.dns.appthrust.io resources: diff --git a/deploy/charts/dns-api/values.yaml b/deploy/charts/dns-api/values.yaml index 431f083..481736e 100644 --- a/deploy/charts/dns-api/values.yaml +++ b/deploy/charts/dns-api/values.yaml @@ -32,6 +32,12 @@ leaderElection: endpoint: recordSetNamespace: dns-api-system +gatewayEndpoint: + enabled: true + +serviceEndpoint: + enabled: true + metrics: port: 8080 diff --git a/scripts/check-central-profile.sh b/scripts/check-central-profile.sh new file mode 100755 index 0000000..ff9c476 --- /dev/null +++ b/scripts/check-central-profile.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +set -euo pipefail + +chart="${1:-deploy/charts/dns-api}" +scratch="$(mktemp -d /tmp/dns-api-central-profile.XXXXXX)" +trap 'rm -rf "$scratch"' EXIT + +helm template dns-api "$chart" \ + --namespace dns-api-system \ + --set gatewayEndpoint.enabled=false \ + --set serviceEndpoint.enabled=false \ + --show-only templates/rbac.yaml >"$scratch/rbac.yaml" + +helm template dns-api "$chart" \ + --namespace dns-api-system \ + --set gatewayEndpoint.enabled=false \ + --set serviceEndpoint.enabled=false \ + --show-only templates/deployment.yaml >"$scratch/deployment.yaml" + +if grep -Eq 'gateway\.networking\.k8s\.io|httproutes|^[[:space:]]+- services$|services/finalizers' "$scratch/rbac.yaml"; then + echo "central profile grants tenant source-discovery RBAC" >&2 + exit 1 +fi + +grep -q 'endpointrecordsets' "$scratch/rbac.yaml" +grep -q -- '--gateway-endpoint-controller-enabled=false' "$scratch/deployment.yaml" +grep -q -- '--service-endpoint-controller-enabled=false' "$scratch/deployment.yaml" From 2be9db17f3af4f12b2ab8f2661e1b3a3fed5faf0 Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 21:22:01 +0900 Subject: [PATCH 5/6] chore: refresh generated controller RBAC --- app/operator/config/rbac/role.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/operator/config/rbac/role.yaml b/app/operator/config/rbac/role.yaml index 5fc0429..b7df99c 100644 --- a/app/operator/config/rbac/role.yaml +++ b/app/operator/config/rbac/role.yaml @@ -20,6 +20,21 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - services + verbs: + - get + - list + - update + - watch +- apiGroups: + - "" + resources: + - services/finalizers + verbs: + - update - apiGroups: - cloudflare.dns.appthrust.io resources: From 078827904fcb507161aeeb770863517a9292d41a Mon Sep 17 00:00:00 2001 From: reoring Date: Wed, 15 Jul 2026 22:29:37 +0900 Subject: [PATCH 6/6] test(policy): harden shared zone tenant boundary --- deploy/charts/dns-api/rbac_contract_test.go | 88 +++++++++++++++++++++ internal/go/core/declaration/policy_test.go | 5 ++ 2 files changed, 93 insertions(+) create mode 100644 deploy/charts/dns-api/rbac_contract_test.go diff --git a/deploy/charts/dns-api/rbac_contract_test.go b/deploy/charts/dns-api/rbac_contract_test.go new file mode 100644 index 0000000..f8f606b --- /dev/null +++ b/deploy/charts/dns-api/rbac_contract_test.go @@ -0,0 +1,88 @@ +package dnsapichart + +import ( + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// The dns-api chart owns only its controller identity. Cloud's central +// allocation writer is authorized by Cloud deployment RBAC; tenant identities +// must never gain EndpointRecordSet create through this upstream chart. +func TestEndpointRecordSetRBACBindsOnlyControllerServiceAccount(t *testing.T) { + rendered := renderChart(t, + "--set", "serviceAccount.name=dns-api-central-controller", + "--set", "gatewayEndpoint.enabled=false", + "--set", "serviceEndpoint.enabled=false", + ) + role := renderedDocument(t, rendered, "rbac.authorization.k8s.io/v1", "ClusterRole") + endpointRule := ruleForAPIGroup(t, role, "endpoint.dns.appthrust.io") + for _, required := range []string{ + "- endpoint.dns.appthrust.io", + "- endpointrecordsets", + "- create", + } { + if !strings.Contains(endpointRule, required) { + t.Fatalf("controller EndpointRecordSet rule is missing %q:\n%s", required, endpointRule) + } + } + binding := renderedDocument(t, rendered, "rbac.authorization.k8s.io/v1", "ClusterRoleBinding") + for _, required := range []string{ + "kind: ServiceAccount", + "name: dns-api-central-controller", + "namespace: appthrust-dns", + } { + if !strings.Contains(binding, required) { + t.Fatalf("controller binding is missing %q:\n%s", required, binding) + } + } + for _, forbidden := range []string{ + "system:authenticated", + "system:serviceaccounts", + "tenant", + "project", + } { + if strings.Contains(binding, forbidden) { + t.Fatalf("controller binding grants an ambient tenant subject %q:\n%s", forbidden, binding) + } + } +} + +func ruleForAPIGroup(t *testing.T, role, apiGroup string) string { + t.Helper() + start := strings.Index(role, " - apiGroups:\n - "+apiGroup+"\n") + if start < 0 { + t.Fatalf("ClusterRole rule for %s was not found:\n%s", apiGroup, role) + } + remainder := role[start+1:] + if next := strings.Index(remainder, "\n - apiGroups:\n"); next >= 0 { + return role[start : start+1+next] + } + return role[start:] +} + +func renderChart(t *testing.T, extra ...string) string { + t.Helper() + _, file, _, _ := runtime.Caller(0) + args := []string{"template", "dns-api", filepath.Dir(file), "--namespace", "appthrust-dns"} + args = append(args, extra...) + output, err := exec.Command("helm", args...).CombinedOutput() + if err != nil { + t.Fatalf("helm template failed: %v\n%s", err, output) + } + return string(output) +} + +func renderedDocument(t *testing.T, rendered, apiVersion, kind string) string { + t.Helper() + for _, document := range strings.Split(rendered, "\n---") { + if strings.Contains(document, "apiVersion: "+apiVersion+"\n") && + strings.Contains(document, "kind: "+kind+"\n") { + return document + } + } + t.Fatalf("rendered %s %s was not found:\n%s", apiVersion, kind, rendered) + return "" +} diff --git a/internal/go/core/declaration/policy_test.go b/internal/go/core/declaration/policy_test.go index 319afe8..4e114da 100644 --- a/internal/go/core/declaration/policy_test.go +++ b/internal/go/core/declaration/policy_test.go @@ -47,10 +47,15 @@ func TestSharedParentZonePolicyAllowsOnlyOrganizationAliasRecords(t *testing.T) {name: "organization wildcard alias AAAA", namespace: writerNamespace.Name, recordName: "*.reo", recordType: dnsv1alpha1.RecordTypeAAAA, allowed: true}, {name: "tenant cannot forge organization apex", namespace: tenantNamespace.Name, recordName: "reo", recordType: dnsv1alpha1.RecordTypeA}, {name: "parent apex", recordName: "@", recordType: dnsv1alpha1.RecordTypeA}, + {name: "parent apex CNAME", recordName: "@", recordType: dnsv1alpha1.RecordTypeCNAME}, {name: "root CAA", recordName: "@", recordType: dnsv1alpha1.RecordTypeCAA}, {name: "root TXT", recordName: "@", recordType: dnsv1alpha1.RecordTypeTXT}, {name: "delegated NS", recordName: "reo", recordType: dnsv1alpha1.RecordTypeNS}, + {name: "organization CNAME", recordName: "reo", recordType: dnsv1alpha1.RecordTypeCNAME}, + {name: "organization wildcard CNAME", recordName: "*.reo", recordType: dnsv1alpha1.RecordTypeCNAME}, {name: "deeper record", recordName: "api.reo", recordType: dnsv1alpha1.RecordTypeA}, + {name: "deeper CNAME", recordName: "api.reo", recordType: dnsv1alpha1.RecordTypeCNAME}, + {name: "deeper wildcard", recordName: "*.api.reo", recordType: dnsv1alpha1.RecordTypeAAAA}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {