diff --git a/api/v1alpha1/internaldns_naming_validation_envtest_test.go b/api/v1alpha1/internaldns_naming_validation_envtest_test.go new file mode 100644 index 00000000..5677df85 --- /dev/null +++ b/api/v1alpha1/internaldns_naming_validation_envtest_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package v1alpha1 + +import ( + "context" + "strings" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func validateNamingAPISchemas(t *testing.T, cl client.Client) { + t.Helper() + ctx := context.Background() + for _, tc := range []struct { + name string + selector *DNSRegistrationSelector + allowed bool + }{ + {"omitted-selector", nil, true}, + {"empty-selector", &DNSRegistrationSelector{}, true}, + {"in-selector", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn, Values: []string{"web"}}}}, true}, + {"exists-selector", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpExists}}}, true}, + {"invalid-operator", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: "Unsupported"}}}, false}, + {"missing-values", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn}}}, false}, + {"unexpected-values", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpExists, Values: []string{"web"}}}}, false}, + {"invalid-value", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn, Values: []string{"invalid value"}}}}, false}, + {"long-value", &DNSRegistrationSelector{MatchExpressions: []DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn, Values: []string{strings.Repeat("x", 64)}}}}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + p := &DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: tc.name, Namespace: "validation"}, Spec: DNSNamingPolicySpec{ResolverContextRef: DNSNameReference{Name: "context"}, AdditionalNames: []DNSAdditionalNameRule{{DNSZoneRef: DNSNameReference{Name: "zone"}, Selector: tc.selector, NamePrefix: "hosts"}}}} + err := cl.Create(ctx, p) + if tc.allowed { + if err != nil { + t.Fatal(err) + } + } else if !apierrors.IsInvalid(err) { + t.Fatalf("expected API validation rejection, got %v", err) + } + }) + } + t.Run("selector-expression-limit", func(t *testing.T) { + p := &DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "expression-limit", Namespace: "validation"}, Spec: DNSNamingPolicySpec{ResolverContextRef: DNSNameReference{Name: "context"}, AdditionalNames: []DNSAdditionalNameRule{{DNSZoneRef: DNSNameReference{Name: "zone"}, NamePrefix: "hosts", Selector: &DNSRegistrationSelector{}}}}} + for range 64 { + p.Spec.AdditionalNames[0].Selector.MatchExpressions = append(p.Spec.AdditionalNames[0].Selector.MatchExpressions, DNSRegistrationSelectorRequirement{Key: "app", Operator: metav1.LabelSelectorOpExists}) + } + if err := cl.Create(ctx, p); err != nil { + t.Fatal(err) + } + p.Spec.AdditionalNames[0].Selector.MatchExpressions = append(p.Spec.AdditionalNames[0].Selector.MatchExpressions, DNSRegistrationSelectorRequirement{Key: "app", Operator: metav1.LabelSelectorOpExists}) + if err := cl.Update(ctx, p); !apierrors.IsInvalid(err) { + t.Fatalf("expected expression limit rejection, got %v", err) + } + }) + for _, tc := range []struct { + name string + context DNSNameReference + allowed bool + }{ + {"one-context", DNSNameReference{Name: "context"}, true}, + {"no-consumer", DNSNameReference{}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + a := &DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: tc.name, Namespace: "validation"}, Spec: DNSZoneAssociationSpec{DNSZoneRef: DNSNameReference{Name: "zone"}, ResolverContextRef: tc.context}} + err := cl.Create(ctx, a) + if tc.allowed { + if err != nil { + t.Fatal(err) + } + } else if !apierrors.IsInvalid(err) { + t.Fatalf("expected API validation rejection, got %v", err) + } + }) + } +} diff --git a/api/v1alpha1/internaldns_project_types.go b/api/v1alpha1/internaldns_project_types.go index f5d92dbc..9a6668c8 100644 --- a/api/v1alpha1/internaldns_project_types.go +++ b/api/v1alpha1/internaldns_project_types.go @@ -19,21 +19,26 @@ type DNSObjectReference struct { Generation int64 `json:"generation,omitempty"` } -// +kubebuilder:validation:XValidation:rule="!has(self.resolverContextRef) || ((self.resolverContextRef.name.size() == 0 && !has(self.resolverContextRef.uid) && (!has(self.resolverContextRef.generation) || self.resolverContextRef.generation == 0)) || (self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() > 0))",message="a nonempty resolverContextRef must pin name and UID" -// +kubebuilder:validation:XValidation:rule="has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0",message="dnsZoneRef must pin the zone UID" -// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() > 0",message="resolverContextRef is required" -// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name == oldSelf.dnsZoneRef.name && self.dnsZoneRef.uid == oldSelf.dnsZoneRef.uid",message="dnsZoneRef identity is immutable" -// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name == oldSelf.resolverContextRef.name && self.resolverContextRef.uid == oldSelf.resolverContextRef.uid",message="resolverContextRef identity is immutable" +// DNSNameReference selects a resource in the same project namespace. +// DNS records its resolved lifetime in controller-owned status. +type DNSNameReference struct { + // +kubebuilder:validation:MaxLength=253 + Name string `json:"name"` +} + +// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name.size() > 0",message="dnsZoneRef requires a name" +// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name.size() > 0",message="resolverContextRef is required" +// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name == oldSelf.dnsZoneRef.name",message="dnsZoneRef is immutable" +// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name == oldSelf.resolverContextRef.name",message="resolverContextRef is immutable" type DNSZoneAssociationSpec struct { - DNSZoneRef DNSObjectReference `json:"dnsZoneRef"` - ResolverContextRef DNSObjectReference `json:"resolverContextRef"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef"` + ResolverContextRef DNSNameReference `json:"resolverContextRef"` } type DNSZoneAssociationStatus struct { - ResolvedDNSZoneRef DNSObjectReference `json:"resolvedDNSZoneRef,omitempty"` - ResolvedResolverContextRef DNSObjectReference `json:"resolvedResolverContextRef,omitempty"` - BindingRef *DNSObjectReference `json:"bindingRef,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` + ResolvedDNSZoneRef DNSNameReference `json:"resolvedDNSZoneRef,omitempty"` + ResolvedResolverContextRef DNSNameReference `json:"resolvedResolverContextRef,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` } // DNSResolverContext is the DNS-owned serving and namespace boundary created by @@ -58,8 +63,8 @@ type DNSResolverContextManagedNamespace struct { } type DNSResolverContextManagedNamespaceStatus struct { - DNSZoneRef DNSObjectReference `json:"dnsZoneRef,omitempty"` - Suffix string `json:"suffix,omitempty"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef,omitempty"` + Suffix string `json:"suffix,omitempty"` } type DNSResolverContextStatus struct { @@ -112,9 +117,8 @@ type DNSResolverAccessBindingSpec struct { } type DNSResolverAccessBindingStatus struct { - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - BindingRef *DNSObjectReference `json:"bindingRef,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` } // +kubebuilder:object:root=true @@ -153,11 +157,11 @@ type DNSZoneAssociationList struct { const DNSRegistrationMaxTTLSeconds int32 = 30 -// +kubebuilder:validation:XValidation:rule="has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0",message="dnsZoneRef must pin the zone UID" -// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name == oldSelf.dnsZoneRef.name && self.dnsZoneRef.uid == oldSelf.dnsZoneRef.uid",message="dnsZoneRef identity is immutable" +// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name.size() > 0",message="dnsZoneRef requires a name" +// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name == oldSelf.dnsZoneRef.name",message="dnsZoneRef is immutable" // +kubebuilder:validation:XValidation:rule="self.name == oldSelf.name",message="name is immutable" type DNSRegistrationSpec struct { - DNSZoneRef DNSObjectReference `json:"dnsZoneRef"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef"` // Name is the owner name, relative to the zone or fully qualified. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 @@ -245,25 +249,25 @@ type DNSRecordContributionList struct { Items []DNSRecordContribution `json:"items"` } -// +kubebuilder:validation:XValidation:rule="!has(self.resolverContextRef) || ((self.resolverContextRef.name.size() == 0 && !has(self.resolverContextRef.uid) && (!has(self.resolverContextRef.generation) || self.resolverContextRef.generation == 0)) || (self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() > 0))",message="a nonempty resolverContextRef must pin name and UID" -// +kubebuilder:validation:XValidation:rule="!has(self.dnsZoneRef) || self.dnsZoneRef.name.size() == 0 || (has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0)",message="a selected dnsZoneRef must pin the zone UID" -// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() > 0",message="resolverContextRef is required" -// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name == oldSelf.resolverContextRef.name && self.resolverContextRef.uid == oldSelf.resolverContextRef.uid",message="resolverContextRef identity is immutable" -// +kubebuilder:validation:XValidation:rule="(has(self.dnsZoneRef) ? self.dnsZoneRef.name : \"\") == (has(oldSelf.dnsZoneRef) ? oldSelf.dnsZoneRef.name : \"\") && ((has(self.dnsZoneRef) && has(self.dnsZoneRef.uid)) ? self.dnsZoneRef.uid : \"\") == ((has(oldSelf.dnsZoneRef) && has(oldSelf.dnsZoneRef.uid)) ? oldSelf.dnsZoneRef.uid : \"\")",message="dnsZoneRef identity is immutable" +// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name.size() > 0",message="resolverContextRef is required" +// +kubebuilder:validation:XValidation:rule="self.resolverContextRef.name == oldSelf.resolverContextRef.name",message="resolverContextRef is immutable" +// +kubebuilder:validation:XValidation:rule="(has(self.dnsZoneRef) ? self.dnsZoneRef.name : \"\") == (has(oldSelf.dnsZoneRef) ? oldSelf.dnsZoneRef.name : \"\")",message="dnsZoneRef is immutable" type DNSNamingPolicySpec struct { - ResolverContextRef DNSObjectReference `json:"resolverContextRef"` + ResolverContextRef DNSNameReference `json:"resolverContextRef"` // +kubebuilder:validation:MaxItems=256 AdditionalNames []DNSAdditionalNameRule `json:"additionalNames"` - DNSZoneRef DNSObjectReference `json:"dnsZoneRef,omitempty"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef,omitempty"` // +kubebuilder:validation:MaxLength=253 ScopePrefix string `json:"scopePrefix,omitempty"` Priority int32 `json:"priority,omitempty"` } -// +kubebuilder:validation:XValidation:rule="has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0",message="dnsZoneRef must pin the zone UID" +// +kubebuilder:validation:XValidation:rule="self.dnsZoneRef.name.size() > 0",message="dnsZoneRef requires a name" type DNSAdditionalNameRule struct { + // Selector matches DNSRegistration metadata labels within this DNS context. + // An omitted or empty selector matches every registration in the context. Selector *DNSRegistrationSelector `json:"selector,omitempty"` - DNSZoneRef DNSObjectReference `json:"dnsZoneRef"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef"` // +kubebuilder:validation:MaxLength=253 NamePrefix string `json:"namePrefix"` } @@ -304,15 +308,14 @@ func (s *DNSRegistrationSelector) AsLabelSelector() *metav1.LabelSelector { } type DNSNamingPolicyStatus struct { - ResolvedResolverContextRef DNSObjectReference `json:"resolvedResolverContextRef,omitempty"` - ResolvedDNSZoneRef DNSObjectReference `json:"resolvedDNSZoneRef,omitempty"` + ResolvedResolverContextRef DNSNameReference `json:"resolvedResolverContextRef,omitempty"` ResolvedAdditionalNames []DNSResolvedAdditionalNameRule `json:"resolvedAdditionalNames,omitempty"` Conditions []metav1.Condition `json:"conditions,omitempty"` } type DNSResolvedAdditionalNameRule struct { Selector *metav1.LabelSelector `json:"selector,omitempty"` - DNSZoneRef DNSObjectReference `json:"dnsZoneRef"` + DNSZoneRef DNSNameReference `json:"dnsZoneRef"` NamePrefix string `json:"namePrefix"` } diff --git a/api/v1alpha1/internaldns_types_test.go b/api/v1alpha1/internaldns_types_test.go index 14fe6d11..f19f38bd 100644 --- a/api/v1alpha1/internaldns_types_test.go +++ b/api/v1alpha1/internaldns_types_test.go @@ -7,8 +7,8 @@ import ( func TestDNSPoliciesSelectResolverContexts(t *testing.T) { for name, value := range map[string]any{ - "association context": DNSZoneAssociationSpec{ResolverContextRef: DNSObjectReference{Name: "ctx", UID: "ctx-uid"}}, - "naming context": DNSNamingPolicySpec{ResolverContextRef: DNSObjectReference{Name: "ctx", UID: "ctx-uid"}}, + "association context": DNSZoneAssociationSpec{ResolverContextRef: DNSNameReference{Name: "ctx"}}, + "naming context": DNSNamingPolicySpec{ResolverContextRef: DNSNameReference{Name: "ctx"}}, } { t.Run(name, func(t *testing.T) { data, err := json.Marshal(value) diff --git a/api/v1alpha1/internaldns_validation_envtest_test.go b/api/v1alpha1/internaldns_validation_envtest_test.go index caff6b78..a102d157 100644 --- a/api/v1alpha1/internaldns_validation_envtest_test.go +++ b/api/v1alpha1/internaldns_validation_envtest_test.go @@ -116,7 +116,7 @@ func TestPrivateDNSValidationAtAPIServer(t *testing.T) { }) for _, ttl := range []int32{0, 31, 2147483647} { t.Run("ttl", func(t *testing.T) { - a := &DNSRegistration{ObjectMeta: metav1.ObjectMeta{GenerateName: "registration-", Namespace: "validation"}, Spec: DNSRegistrationSpec{DNSZoneRef: DNSObjectReference{Name: "zone", UID: "zone-uid"}, Name: "web", RecordTypes: []RRType{RRTypeAAAA}, TTLSeconds: ttl}} + a := &DNSRegistration{ObjectMeta: metav1.ObjectMeta{GenerateName: "registration-", Namespace: "validation"}, Spec: DNSRegistrationSpec{DNSZoneRef: DNSNameReference{Name: "zone"}, Name: "web", RecordTypes: []RRType{RRTypeAAAA}, TTLSeconds: ttl}} requireInvalid(t, cl.Create(ctx, a)) }) } @@ -165,29 +165,27 @@ func TestPrivateDNSValidationAtAPIServer(t *testing.T) { } }) validateServiceDNSContracts(t, cl) + validateNamingAPISchemas(t, cl) validateReferenceIdentity(t, cl, requireInvalid) validateRegistrationBounds(t, cl, requireInvalid) } -// References may advance a pinned generation, but never retarget another object. +// References select objects by name and never retarget another object. func validateReferenceIdentity(t *testing.T, cl client.Client, requireInvalid func(*testing.T, error)) { ctx := context.Background() - zone := DNSObjectReference{Name: "zone", UID: "zone-uid", Generation: 1} - resolverContext := DNSObjectReference{Name: "context", UID: "context-uid"} + zone := DNSNameReference{Name: "zone"} + resolverContext := DNSNameReference{Name: "context"} t.Run("association-reference-identity", func(t *testing.T) { for name, mutate := range map[string]func(*DNSZoneAssociation){ "zone-name": func(a *DNSZoneAssociation) { a.Spec.DNSZoneRef.Name = "other" }, - "zone-uid": func(a *DNSZoneAssociation) { a.Spec.DNSZoneRef.UID = "other-uid" }, "context-name": func(a *DNSZoneAssociation) { a.Spec.ResolverContextRef.Name = "other" }, - "context-uid": func(a *DNSZoneAssociation) { a.Spec.ResolverContextRef.UID = "other-uid" }, } { a := &DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{GenerateName: "association-", Namespace: "validation"}, Spec: DNSZoneAssociationSpec{DNSZoneRef: zone, ResolverContextRef: resolverContext}} if err := cl.Create(ctx, a); err != nil { t.Fatal(err) } - a.Spec.DNSZoneRef.Generation = 2 if err := cl.Update(ctx, a); err != nil { - t.Fatalf("zone generation cannot advance: %v", err) + t.Fatalf("unchanged association update rejected: %v", err) } mutate(a) t.Run(name, func(t *testing.T) { requireInvalid(t, cl.Update(ctx, a)) }) @@ -196,10 +194,8 @@ func validateReferenceIdentity(t *testing.T, cl client.Client, requireInvalid fu t.Run("naming-policy-reference-identity", func(t *testing.T) { for name, mutate := range map[string]func(*DNSNamingPolicy){ "zone-name": func(p *DNSNamingPolicy) { p.Spec.DNSZoneRef.Name = "other" }, - "zone-uid": func(p *DNSNamingPolicy) { p.Spec.DNSZoneRef.UID = "other-uid" }, - "zone-cleared": func(p *DNSNamingPolicy) { p.Spec.DNSZoneRef = DNSObjectReference{} }, + "zone-cleared": func(p *DNSNamingPolicy) { p.Spec.DNSZoneRef = DNSNameReference{} }, "context-name": func(p *DNSNamingPolicy) { p.Spec.ResolverContextRef.Name = "other" }, - "context-uid": func(p *DNSNamingPolicy) { p.Spec.ResolverContextRef.UID = "other-uid" }, } { p := &DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{GenerateName: "naming-", Namespace: "validation"}, Spec: DNSNamingPolicySpec{ResolverContextRef: resolverContext, DNSZoneRef: zone, AdditionalNames: []DNSAdditionalNameRule{}}} if err := cl.Create(ctx, p); err != nil { @@ -223,16 +219,14 @@ func validateReferenceIdentity(t *testing.T, cl client.Client, requireInvalid fu for name, mutate := range map[string]func(*DNSRegistration){ "name": func(r *DNSRegistration) { r.Spec.Name = "api" }, "zone-name": func(r *DNSRegistration) { r.Spec.DNSZoneRef.Name = "other" }, - "zone-uid": func(r *DNSRegistration) { r.Spec.DNSZoneRef.UID = "other-uid" }, } { r := &DNSRegistration{ObjectMeta: metav1.ObjectMeta{GenerateName: "registration-", Namespace: "validation"}, Spec: DNSRegistrationSpec{DNSZoneRef: zone, Name: "web", RecordTypes: []RRType{RRTypeAAAA}, TTLSeconds: 30}} if err := cl.Create(ctx, r); err != nil { t.Fatal(err) } r.Spec.TTLSeconds = 10 - r.Spec.DNSZoneRef.Generation = 2 if err := cl.Update(ctx, r); err != nil { - t.Fatalf("registration cannot change TTL or zone generation: %v", err) + t.Fatalf("registration cannot change TTL: %v", err) } mutate(r) t.Run(name, func(t *testing.T) { requireInvalid(t, cl.Update(ctx, r)) }) @@ -258,7 +252,7 @@ func validateRegistrationBounds(t *testing.T, cl client.Client, requireInvalid f "many-reserved": func(s *DNSRegistrationSpec) { s.ReservedDescendants = many(65, "r") }, } { t.Run("registration-bounds-"+name, func(t *testing.T) { - r := &DNSRegistration{ObjectMeta: metav1.ObjectMeta{GenerateName: "bounded-", Namespace: "validation"}, Spec: DNSRegistrationSpec{DNSZoneRef: DNSObjectReference{Name: "zone", UID: "zone-uid"}, Name: "web", RecordTypes: []RRType{RRTypeA}, TTLSeconds: 30}} + r := &DNSRegistration{ObjectMeta: metav1.ObjectMeta{GenerateName: "bounded-", Namespace: "validation"}, Spec: DNSRegistrationSpec{DNSZoneRef: DNSNameReference{Name: "zone"}, Name: "web", RecordTypes: []RRType{RRTypeA}, TTLSeconds: 30}} mutate(&r.Spec) requireInvalid(t, cl.Create(ctx, r)) }) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e85932b3..ab7846e0 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -166,6 +166,21 @@ func (in *DNSContributionRecordSet) DeepCopy() *DNSContributionRecordSet { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSNameReference) DeepCopyInto(out *DNSNameReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSNameReference. +func (in *DNSNameReference) DeepCopy() *DNSNameReference { + if in == nil { + return nil + } + out := new(DNSNameReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DNSNamingPolicy) DeepCopyInto(out *DNSNamingPolicy) { *out = *in @@ -253,7 +268,6 @@ func (in *DNSNamingPolicySpec) DeepCopy() *DNSNamingPolicySpec { func (in *DNSNamingPolicyStatus) DeepCopyInto(out *DNSNamingPolicyStatus) { *out = *in out.ResolvedResolverContextRef = in.ResolvedResolverContextRef - out.ResolvedDNSZoneRef = in.ResolvedDNSZoneRef if in.ResolvedAdditionalNames != nil { in, out := &in.ResolvedAdditionalNames, &out.ResolvedAdditionalNames *out = make([]DNSResolvedAdditionalNameRule, len(*in)) @@ -1096,11 +1110,6 @@ func (in *DNSResolverAccessBindingSpec) DeepCopy() *DNSResolverAccessBindingSpec // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DNSResolverAccessBindingStatus) DeepCopyInto(out *DNSResolverAccessBindingStatus) { *out = *in - if in.BindingRef != nil { - in, out := &in.BindingRef, &out.BindingRef - *out = new(DNSObjectReference) - **out = **in - } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) @@ -1679,11 +1688,6 @@ func (in *DNSZoneAssociationStatus) DeepCopyInto(out *DNSZoneAssociationStatus) *out = *in out.ResolvedDNSZoneRef = in.ResolvedDNSZoneRef out.ResolvedResolverContextRef = in.ResolvedResolverContextRef - if in.BindingRef != nil { - in, out := &in.BindingRef, &out.BindingRef - *out = new(DNSObjectReference) - **out = **in - } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/config/crd/bases/dns.networking.miloapis.com_dnsnamingpolicies.yaml b/config/crd/bases/dns.networking.miloapis.com_dnsnamingpolicies.yaml index 6da49ceb..7ee3da67 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnsnamingpolicies.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnsnamingpolicies.yaml @@ -43,23 +43,12 @@ spec: properties: dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -67,8 +56,9 @@ spec: maxLength: 253 type: string selector: - description: DNSRegistrationSelector matches registration labels - using Kubernetes selector semantics. + description: |- + Selector matches DNSRegistration metadata labels within this DNS context. + An omitted or empty selector matches every registration in the context. properties: matchExpressions: items: @@ -115,30 +105,18 @@ spec: - namePrefix type: object x-kubernetes-validations: - - message: dnsZoneRef must pin the zone UID - rule: has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > - 0 + - message: dnsZoneRef requires a name + rule: self.dnsZoneRef.name.size() > 0 maxItems: 256 type: array dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -147,23 +125,12 @@ spec: type: integer resolverContextRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -175,26 +142,13 @@ spec: - resolverContextRef type: object x-kubernetes-validations: - - message: a nonempty resolverContextRef must pin name and UID - rule: '!has(self.resolverContextRef) || ((self.resolverContextRef.name.size() - == 0 && !has(self.resolverContextRef.uid) && (!has(self.resolverContextRef.generation) - || self.resolverContextRef.generation == 0)) || (self.resolverContextRef.name.size() - > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() - > 0))' - - message: a selected dnsZoneRef must pin the zone UID - rule: '!has(self.dnsZoneRef) || self.dnsZoneRef.name.size() == 0 || - (has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0)' - message: resolverContextRef is required - rule: self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) - && self.resolverContextRef.uid.size() > 0 - - message: resolverContextRef identity is immutable + rule: self.resolverContextRef.name.size() > 0 + - message: resolverContextRef is immutable rule: self.resolverContextRef.name == oldSelf.resolverContextRef.name - && self.resolverContextRef.uid == oldSelf.resolverContextRef.uid - - message: dnsZoneRef identity is immutable + - message: dnsZoneRef is immutable rule: '(has(self.dnsZoneRef) ? self.dnsZoneRef.name : "") == (has(oldSelf.dnsZoneRef) - ? oldSelf.dnsZoneRef.name : "") && ((has(self.dnsZoneRef) && has(self.dnsZoneRef.uid)) - ? self.dnsZoneRef.uid : "") == ((has(oldSelf.dnsZoneRef) && has(oldSelf.dnsZoneRef.uid)) - ? oldSelf.dnsZoneRef.uid : "")' + ? oldSelf.dnsZoneRef.name : "")' status: properties: conditions: @@ -258,23 +212,12 @@ spec: properties: dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -334,47 +277,14 @@ spec: - namePrefix type: object type: array - resolvedDNSZoneRef: - description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. - properties: - generation: - format: int64 - minimum: 0 - type: integer - name: - maxLength: 253 - type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string - required: - - name - type: object resolvedResolverContextRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object diff --git a/config/crd/bases/dns.networking.miloapis.com_dnsregistrations.yaml b/config/crd/bases/dns.networking.miloapis.com_dnsregistrations.yaml index b98c6c95..d19a0d62 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnsregistrations.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnsregistrations.yaml @@ -40,23 +40,12 @@ spec: properties: dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -111,11 +100,10 @@ spec: - ttlSeconds type: object x-kubernetes-validations: - - message: dnsZoneRef must pin the zone UID - rule: has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0 - - message: dnsZoneRef identity is immutable - rule: self.dnsZoneRef.name == oldSelf.dnsZoneRef.name && self.dnsZoneRef.uid - == oldSelf.dnsZoneRef.uid + - message: dnsZoneRef requires a name + rule: self.dnsZoneRef.name.size() > 0 + - message: dnsZoneRef is immutable + rule: self.dnsZoneRef.name == oldSelf.dnsZoneRef.name - message: name is immutable rule: self.name == oldSelf.name status: diff --git a/config/crd/bases/dns.networking.miloapis.com_dnsresolveraccessbindings.yaml b/config/crd/bases/dns.networking.miloapis.com_dnsresolveraccessbindings.yaml index f7ae257d..292bbb40 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnsresolveraccessbindings.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnsresolveraccessbindings.yaml @@ -86,28 +86,6 @@ spec: rule: self.region == oldSelf.region && self.destinationAddress == oldSelf.destinationAddress status: properties: - bindingRef: - description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. - properties: - generation: - format: int64 - minimum: 0 - type: integer - name: - maxLength: 253 - type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string - required: - - name - type: object conditions: items: description: Condition contains details for one aspect of the current diff --git a/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml b/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml index 46dee0ed..790b11f1 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml @@ -134,23 +134,12 @@ spec: properties: dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object diff --git a/config/crd/bases/dns.networking.miloapis.com_dnszoneassociations.yaml b/config/crd/bases/dns.networking.miloapis.com_dnszoneassociations.yaml index 14653ea2..f3975448 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnszoneassociations.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnszoneassociations.yaml @@ -40,45 +40,23 @@ spec: properties: dnsZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object resolverContextRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object @@ -87,47 +65,16 @@ spec: - resolverContextRef type: object x-kubernetes-validations: - - message: a nonempty resolverContextRef must pin name and UID - rule: '!has(self.resolverContextRef) || ((self.resolverContextRef.name.size() - == 0 && !has(self.resolverContextRef.uid) && (!has(self.resolverContextRef.generation) - || self.resolverContextRef.generation == 0)) || (self.resolverContextRef.name.size() - > 0 && has(self.resolverContextRef.uid) && self.resolverContextRef.uid.size() - > 0))' - - message: dnsZoneRef must pin the zone UID - rule: has(self.dnsZoneRef.uid) && self.dnsZoneRef.uid.size() > 0 + - message: dnsZoneRef requires a name + rule: self.dnsZoneRef.name.size() > 0 - message: resolverContextRef is required - rule: self.resolverContextRef.name.size() > 0 && has(self.resolverContextRef.uid) - && self.resolverContextRef.uid.size() > 0 - - message: dnsZoneRef identity is immutable - rule: self.dnsZoneRef.name == oldSelf.dnsZoneRef.name && self.dnsZoneRef.uid - == oldSelf.dnsZoneRef.uid - - message: resolverContextRef identity is immutable + rule: self.resolverContextRef.name.size() > 0 + - message: dnsZoneRef is immutable + rule: self.dnsZoneRef.name == oldSelf.dnsZoneRef.name + - message: resolverContextRef is immutable rule: self.resolverContextRef.name == oldSelf.resolverContextRef.name - && self.resolverContextRef.uid == oldSelf.resolverContextRef.uid status: properties: - bindingRef: - description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. - properties: - generation: - format: int64 - minimum: 0 - type: integer - name: - maxLength: 253 - type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string - required: - - name - type: object conditions: items: description: Condition contains details for one aspect of the current @@ -186,45 +133,23 @@ spec: type: array resolvedDNSZoneRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object resolvedResolverContextRef: description: |- - DNSObjectReference pins a namespaced object by API-assigned UID. Generation - is used where a writer must also be fenced against policy changes. + DNSNameReference selects a resource in the same project namespace. + DNS records its resolved lifetime in controller-owned status. properties: - generation: - format: int64 - minimum: 0 - type: integer name: maxLength: 253 type: string - uid: - description: |- - UID is a type that holds unique ID values, including UUIDs. Because we - don't ONLY use UUIDs, this is an alias to string. Being a type captures - intent and helps make sure that UIDs and names do not get conflated. - maxLength: 128 - type: string required: - name type: object diff --git a/config/internal-dns/README.md b/config/internal-dns/README.md index 5449bc3f..4e8c6dfc 100644 --- a/config/internal-dns/README.md +++ b/config/internal-dns/README.md @@ -233,3 +233,35 @@ reports `CapacityAvailable=False` for the affected binding while healthy contexts and withdrawals continue. A pending zone fails only queries beneath its private apex; its names never fall through to public recursion. Bindings, publication targets, and context status use the same stable shard assignment. + +### Control-plane monitoring + +Set `metricsBindAddress` to an internal listener, such as `:8080`, to expose +`/metrics` on the control-plane process. The endpoint is disabled when the +setting is omitted or set to `0`. Restrict access to the monitoring network. + +The endpoint includes controller-runtime reconciliation metrics and: + +- `internal_dns_operations_total{operation,result}`: completed operations, + with `result` set to `success` or `error`. +- `internal_dns_operation_last_success_timestamp_seconds{operation}`: the + latest successful completion time. Errors do not advance this timestamp. + +Operations identify configured discovery, planner, acknowledgement refresh, +transport outbox, and acknowledgement processing loops. A successful loop +can be idle; it does not prove that an endpoint is serving. Combine these +metrics with binding conditions, broker backlog, and DNS queries from a VPC. + +### Reference-state recovery + +An observed association, naming policy, or registration requires its private +reference state to retain the target object's identity. If that state is +missing, DNS rejects the source instead of attaching it to a same-name target. +The rejection persists across retries. Restore a verified reference-state +backup, or recreate the source resource to select its targets explicitly. + +Registration write permission is the publication boundary within a project. +Naming-policy selectors organize names; they do not authorize publishers. +A publisher that can change registration labels can select every matching +rule for the registration's associated context. Restrict registration writes +with RBAC when publishers must not share that authority. diff --git a/config/internal-dns/examples/private-zone.yaml b/config/internal-dns/examples/private-zone.yaml new file mode 100644 index 00000000..b20e20ec --- /dev/null +++ b/config/internal-dns/examples/private-zone.yaml @@ -0,0 +1,38 @@ +apiVersion: dns.networking.miloapis.com/v1alpha1 +kind: DNSZone +metadata: + name: production + namespace: project +spec: + visibility: Private + domainName: prod.internal + dnsZoneClassName: datum-internal-dns +--- +apiVersion: dns.networking.miloapis.com/v1alpha1 +kind: DNSZoneAssociation +metadata: + name: application-production + namespace: project +spec: + # Select resources by name within this project namespace. + dnsZoneRef: + name: production + resolverContextRef: + name: application-network +--- +apiVersion: dns.networking.miloapis.com/v1alpha1 +kind: DNSNamingPolicy +metadata: + name: application-production-names + namespace: project +spec: + resolverContextRef: + name: application-network + additionalNames: + # Match registration labels supplied by any authorized publisher. + - selector: + matchLabels: + app: storefront + dnsZoneRef: + name: production + namePrefix: storefront diff --git a/go.mod b/go.mod index e6bc0765..6d9958d1 100644 --- a/go.mod +++ b/go.mod @@ -66,6 +66,7 @@ require ( github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/jpillora/backoff v1.0.0 // indirect github.com/klauspost/compress v1.20.0 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lufia/plan9stats v0.0.0-20260216142805-b3301c5f2a88 // indirect github.com/magiconair/properties v1.8.10 // indirect github.com/mdlayher/socket v0.6.0 // indirect diff --git a/internal/internaldns/admission/handler.go b/internal/internaldns/admission/handler.go index a1cb691a..85adda0a 100644 --- a/internal/internaldns/admission/handler.go +++ b/internal/internaldns/admission/handler.go @@ -177,21 +177,18 @@ func (h *Handler) association(ctx context.Context, cl client.Client, req kubeadm if err := decode(req, &a); err != nil { return err } - if _, err := currentPrivateZone(ctx, cl, req.Namespace, a.Spec.DNSZoneRef); err != nil { + if err := validateNamedPrivateZone(ctx, cl, req.Namespace, a.Spec.DNSZoneRef); err != nil { return err } - if a.Spec.ResolverContextRef.Name == "" || a.Spec.ResolverContextRef.UID == "" { - return fmt.Errorf("association must pin resolver context name and UID") - } // Readiness is not required here: the context controller creates the // managed zone association before the context itself can become Ready. var resolverContext dnsv1alpha1.DNSResolverContext if err := cl.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: a.Spec.ResolverContextRef.Name}, &resolverContext); err != nil { return fmt.Errorf("resolver context lookup: %w", err) } - if resolverContext.UID != a.Spec.ResolverContextRef.UID || !resolverContext.DeletionTimestamp.IsZero() { - return fmt.Errorf("resolver context lifetime is not current") + if !resolverContext.DeletionTimestamp.IsZero() { + return fmt.Errorf("resolver context is being deleted") } return nil } @@ -203,7 +200,7 @@ func (h *Handler) namingPolicy(ctx context.Context, cl client.Client, req kubead } if p.Spec.DNSZoneRef.Name != "" { - if _, err := currentPrivateZone(ctx, cl, req.Namespace, p.Spec.DNSZoneRef); err != nil { + if err := validateNamedPrivateZone(ctx, cl, req.Namespace, p.Spec.DNSZoneRef); err != nil { return err } } @@ -211,25 +208,36 @@ func (h *Handler) namingPolicy(ctx context.Context, cl client.Client, req kubead if _, err := metav1.LabelSelectorAsSelector(rule.Selector.AsLabelSelector()); err != nil { return fmt.Errorf("invalid naming selector: %w", err) } - if _, err := currentPrivateZone(ctx, cl, req.Namespace, rule.DNSZoneRef); err != nil { + if err := validateNamedPrivateZone(ctx, cl, req.Namespace, rule.DNSZoneRef); err != nil { return err } } - if p.Spec.ResolverContextRef.Name == "" || p.Spec.ResolverContextRef.UID == "" { - return fmt.Errorf("naming policy must pin resolver context name and UID") - } var resolverContext dnsv1alpha1.DNSResolverContext if err := cl.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: p.Spec.ResolverContextRef.Name}, &resolverContext); err != nil { return fmt.Errorf("resolver context lookup: %w", err) } ready := apimeta.FindStatusCondition(resolverContext.Status.Conditions, "Ready") - if resolverContext.UID != p.Spec.ResolverContextRef.UID || !resolverContext.DeletionTimestamp.IsZero() || ready == nil || ready.Status != metav1.ConditionTrue || ready.ObservedGeneration != resolverContext.Generation { + if !resolverContext.DeletionTimestamp.IsZero() || ready == nil || ready.Status != metav1.ConditionTrue || ready.ObservedGeneration != resolverContext.Generation { return fmt.Errorf("resolver context lifetime is not ready") } return nil } +func validateNamedPrivateZone(ctx context.Context, cl client.Client, namespace string, ref dnsv1alpha1.DNSNameReference) error { + if ref.Name == "" { + return fmt.Errorf("zone reference requires a name") + } + var zone dnsv1alpha1.DNSZone + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref.Name}, &zone); err != nil { + return fmt.Errorf("private zone lookup: %w", err) + } + if !zone.DeletionTimestamp.IsZero() || zone.Spec.Visibility != dnsv1alpha1.DNSZoneVisibilityPrivate { + return fmt.Errorf("internal references require a live private zone") + } + return nil +} + func decode(req kubeadmission.Request, obj any) error { data := req.Object.Raw if req.Operation == admissionv1.Delete { @@ -268,16 +276,15 @@ func (h *Handler) registration(ctx context.Context, cl client.Client, req kubead return nil } -// Zone references cannot silently follow a replacement with the same name. -func currentPrivateZone(ctx context.Context, cl client.Client, namespace string, ref dnsv1alpha1.DNSObjectReference) (*dnsv1alpha1.DNSZone, error) { - if ref.Name == "" || ref.UID == "" { - return nil, fmt.Errorf("zone reference must pin name and UID") +func currentPrivateZone(ctx context.Context, cl client.Client, namespace string, ref dnsv1alpha1.DNSNameReference) (*dnsv1alpha1.DNSZone, error) { + if ref.Name == "" { + return nil, fmt.Errorf("zone reference requires a name") } var zone dnsv1alpha1.DNSZone if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref.Name}, &zone); err != nil { return nil, fmt.Errorf("private zone lookup: %w", err) } - if zone.UID != ref.UID || !zone.DeletionTimestamp.IsZero() || (ref.Generation != 0 && ref.Generation != zone.Generation) { + if !zone.DeletionTimestamp.IsZero() { return nil, fmt.Errorf("zone lifetime is not current") } if zone.Spec.Visibility != dnsv1alpha1.DNSZoneVisibilityPrivate { diff --git a/internal/internaldns/admission/handler_test.go b/internal/internaldns/admission/handler_test.go index d709af52..10b3ac51 100644 --- a/internal/internaldns/admission/handler_test.go +++ b/internal/internaldns/admission/handler_test.go @@ -5,6 +5,7 @@ package admission import ( "context" "encoding/json" + "strings" "testing" "time" @@ -112,6 +113,49 @@ func TestResolverAccessRequiresCurrentContext(t *testing.T) { } } +func TestCustomerNamedReferencesValidateLiveResources(t *testing.T) { + scheme := runtime.NewScheme() + if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "project"}, Spec: dnsv1alpha1.DNSZoneSpec{Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "application", Namespace: "project", Generation: 1}} + setAdmissionCondition(&resolver.Status.Conditions, "Ready", metav1.ConditionTrue, 1, time.Now()) + h := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, resolver).Build()} + zoneRef, contextRef := dnsv1alpha1.DNSNameReference{Name: zone.Name}, dnsv1alpha1.DNSNameReference{Name: resolver.Name} + for resource, object := range map[string]any{ + "dnszoneassociations": &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: zoneRef, ResolverContextRef: contextRef}}, + "dnsnamingpolicies": &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: contextRef, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{DNSZoneRef: zoneRef, NamePrefix: "hosts"}}}}, + } { + if response := h.Handle(context.Background(), request(resource, "rbac-authorized-customer", object, nil)); !response.Allowed { + t.Fatalf("valid %s denied: %v", resource, response.Result) + } + missing := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).Build()} + if missing.Handle(context.Background(), request(resource, "rbac-authorized-customer", object, nil)).Allowed { + t.Fatalf("%s accepted missing references", resource) + } + } +} + +func TestNamingPolicyRejectsMalformedSelectors(t *testing.T) { + scheme := runtime.NewScheme() + if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + h := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).Build()} + for _, selector := range []*dnsv1alpha1.DNSRegistrationSelector{ + {MatchLabels: map[string]string{"bad key": "value"}}, + {MatchExpressions: []dnsv1alpha1.DNSRegistrationSelectorRequirement{{Key: "app", Operator: "Unsupported"}}}, + {MatchExpressions: []dnsv1alpha1.DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn}}}, + } { + policy := &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: "application"}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{Selector: selector, DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "production"}, NamePrefix: "application"}}}} + response := h.Handle(context.Background(), request("dnsnamingpolicies", "customer", policy, nil)) + if response.Allowed || response.Result == nil || !strings.Contains(response.Result.Message, "invalid naming selector") { + t.Fatalf("malformed selector was not rejected by validation: %v", response.Result) + } + } +} + func setAdmissionCondition(conditions *[]metav1.Condition, conditionType string, status metav1.ConditionStatus, generation int64, now time.Time) { apimeta.SetStatusCondition(conditions, metav1.Condition{Type: conditionType, Status: status, Reason: "Test", ObservedGeneration: generation, LastTransitionTime: metav1.NewTime(now)}) } @@ -128,7 +172,7 @@ func TestPrivateRegistrationRejectsAuthorityOverrides(t *testing.T) { rt dnsv1alpha1.RRType allowed bool }{{"api", dnsv1alpha1.RRTypeA, true}, {"api", dnsv1alpha1.RRTypeCNAME, true}, {"@", dnsv1alpha1.RRTypeCNAME, false}, {"CORP.INTERNAL.", dnsv1alpha1.RRTypeCNAME, false}, {"@", dnsv1alpha1.RRTypeSOA, false}} { - registration := dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, TTLSeconds: 5, Name: tt.owner, RecordTypes: []dnsv1alpha1.RRType{tt.rt}}} + registration := dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, TTLSeconds: 5, Name: tt.owner, RecordTypes: []dnsv1alpha1.RRType{tt.rt}}} response := h.Handle(context.Background(), request("dnsregistrations", "tenant", registration, nil)) if response.Allowed != tt.allowed { t.Fatalf("owner %s type %s allowed=%v expected=%v", tt.owner, tt.rt, response.Allowed, tt.allowed) @@ -144,15 +188,15 @@ func TestRegistrationRequiresCurrentZone(t *testing.T) { zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", Namespace: "project", UID: "current"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "corp.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} h := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone).Build()} for _, tt := range []struct { - uid types.UID + name string ttl int32 allowed bool }{ - {"current", 1, true}, {"current", 30, true}, {"", 5, false}, {"retired", 5, false}, + {zone.Name, 1, true}, {zone.Name, 30, true}, {"", 5, false}, {"missing", 5, false}, } { - reg := dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: tt.uid}, TTLSeconds: tt.ttl, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} + reg := dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: tt.name}, TTLSeconds: tt.ttl, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} if got := h.Handle(context.Background(), request("dnsregistrations", "tenant", reg, nil)); got.Allowed != tt.allowed { - t.Errorf("UID=%q TTL=%d: allowed=%v: %v", tt.uid, tt.ttl, got.Allowed, got.Result) + t.Errorf("name=%q TTL=%d: allowed=%v: %v", tt.name, tt.ttl, got.Allowed, got.Result) } } } @@ -162,8 +206,8 @@ func TestContributionValidatesReferencesForRBACAuthorizedCallers(t *testing.T) { if err := dnsv1alpha1.AddToScheme(scheme); err != nil { t.Fatal(err) } - zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", Namespace: "project", UID: "zone-uid"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "datum.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration", Generation: 1}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, Name: "web", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} + zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", Namespace: "project"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "datum.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} + registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration", Generation: 1}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Name: "web", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} handler := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, registration).Build()} contribution := &dnsv1alpha1.DNSRecordContribution{Spec: dnsv1alpha1.DNSRecordContributionSpec{RegistrationRef: dnsv1alpha1.DNSObjectReference{Name: registration.Name, UID: registration.UID, Generation: registration.Generation}, RecordSets: []dnsv1alpha1.DNSContributionRecordSet{{RecordType: dnsv1alpha1.RRTypeA, Records: []dnsv1alpha1.RecordEntry{{Name: "web", A: &dnsv1alpha1.ARecordSpec{Content: "10.0.0.8"}}}}}}} for _, caller := range []string{"compute-publisher", "connect-publisher"} { @@ -183,7 +227,7 @@ func TestContributionOwnersStayWithinRegistration(t *testing.T) { t.Fatal(err) } zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", Namespace: "project", UID: "zone-uid"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "datum.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration", Generation: 1}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, Name: "web", ReservedDescendants: []string{"targets.web"}, RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} + registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration", Generation: 1}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Name: "web", ReservedDescendants: []string{"targets.web"}, RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}}} handler := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, registration).Build()} for _, tt := range []struct { owner string @@ -204,27 +248,16 @@ func TestAssociationRequiresCurrentResolverContext(t *testing.T) { zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", Namespace: "project", UID: "zone-uid"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "datum.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} // A context that is not yet Ready is still a valid association target. resolverContext := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "ctx", Namespace: "project", UID: "ctx-uid", Generation: 1}} - association := func(contextUID types.UID) *dnsv1alpha1.DNSZoneAssociation { - return &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, ResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: resolverContext.Name, UID: contextUID}}} + association := func(contextName string) *dnsv1alpha1.DNSZoneAssociation { + return &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: contextName}}} } h := Handler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, resolverContext).Build()} - if got := h.Handle(context.Background(), request("dnszoneassociations", "tenant", association("ctx-uid"), nil)); !got.Allowed { + if got := h.Handle(context.Background(), request("dnszoneassociations", "tenant", association(resolverContext.Name), nil)); !got.Allowed { t.Fatalf("current context rejected: %v", got.Result) } - if h.Handle(context.Background(), request("dnszoneassociations", "tenant", association("replaced"), nil)).Allowed { - t.Fatal("association accepted a replaced resolver context") - } - h.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone).Build() - if h.Handle(context.Background(), request("dnszoneassociations", "tenant", association("ctx-uid"), nil)).Allowed { + if h.Handle(context.Background(), request("dnszoneassociations", "tenant", association("missing"), nil)).Allowed { t.Fatal("association accepted a missing resolver context") } - deleting := resolverContext.DeepCopy() - deleting.Finalizers = []string{"test"} - deleting.DeletionTimestamp = &metav1.Time{Time: time.Now()} - h.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, deleting).Build() - if h.Handle(context.Background(), request("dnszoneassociations", "tenant", association("ctx-uid"), nil)).Allowed { - t.Fatal("association accepted a deleting resolver context") - } } func TestDeletingResourceCanRemoveFinalizersWithoutReferences(t *testing.T) { diff --git a/internal/internaldns/controlplane/access_recovery_envtest_test.go b/internal/internaldns/controlplane/access_recovery_envtest_test.go index f8057d0d..0230e2e1 100644 --- a/internal/internaldns/controlplane/access_recovery_envtest_test.go +++ b/internal/internaldns/controlplane/access_recovery_envtest_test.go @@ -37,7 +37,7 @@ func TestAccessRecoveryPreservesSourceFencesWithCEL(t *testing.T) { } }) scheme := runtime.NewScheme() - requireAccessTestSuccess(t, dnsv1alpha1.AddToScheme(scheme)) + requireAccessTestSuccess(t, addReferenceStateToScheme(scheme)) requireAccessTestSuccess(t, corev1.AddToScheme(scheme)) cl, err := client.New(cfg, client.Options{Scheme: scheme}) if err != nil { @@ -81,6 +81,9 @@ func TestAccessRecoveryPreservesSourceFencesWithCEL(t *testing.T) { requireAccessTestSuccess(t, cl.Status().Update(ctx, c)) } readyContext(resolverContext) + t.Run("name-only customer references", func(t *testing.T) { + verifyNamedReferencesWithAPI(t, cl) + }) access := &dnsv1alpha1.DNSResolverAccessBinding{ObjectMeta: metav1.ObjectMeta{Name: "access", Namespace: "project"}, Spec: dnsv1alpha1.DNSResolverAccessBindingSpec{ContextRef: dnsv1alpha1.DNSObjectReference{Name: resolverContext.Name, UID: resolverContext.UID}, Region: "central", DestinationAddress: "fd70:100::10"}} requireAccessTestSuccess(t, cl.Create(ctx, access)) r := &Reconciler{Client: cl, Scheme: scheme, Options: ReconcilerOptions{ProjectUID: "project-uid", PlatformNamespace: "platform", Region: "central", Shard: "shared", AddressAllocator: &platform.Allocator{Client: cl, Namespace: "platform"}}} diff --git a/internal/internaldns/controlplane/compiler.go b/internal/internaldns/controlplane/compiler.go index f024ea29..95409184 100644 --- a/internal/internaldns/controlplane/compiler.go +++ b/internal/internaldns/controlplane/compiler.go @@ -11,8 +11,6 @@ import ( dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" "go.miloapis.com/dns-operator/internal/internaldns/model" - apimeta "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -56,13 +54,15 @@ type RegistrationFence struct { } type CompileInput struct { - Zone *dnsv1alpha1.DNSZone - Associations []dnsv1alpha1.DNSZoneAssociation - Registrations []dnsv1alpha1.DNSRegistration - Contributions []dnsv1alpha1.DNSRecordContribution - StaticRecords []dnsv1alpha1.DNSRecordSet - Previous *PublicationPlan - Now time.Time + Zone *dnsv1alpha1.DNSZone + Associations []dnsv1alpha1.DNSZoneAssociation + AssociationReferences map[types.UID]associationReferences + RegistrationZoneReferences map[types.UID]dnsv1alpha1.DNSObjectReference + Registrations []dnsv1alpha1.DNSRegistration + Contributions []dnsv1alpha1.DNSRecordContribution + StaticRecords []dnsv1alpha1.DNSRecordSet + Previous *PublicationPlan + Now time.Time } type CompileResult struct { @@ -107,7 +107,7 @@ func compileInputs(in CompileInput, rejected map[types.UID]string) (CompileResul result.Plan.ZoneUID = in.Zone.UID result.Plan.ZoneApex = canonicalName(in.Zone.Spec.DomainName) - result.Plan.ContextUIDs = associatedContextUIDs(in.Zone, in.Associations) + result.Plan.ContextUIDs = associatedContextUIDs(in.Zone, in.Associations, in.AssociationReferences) claims := map[ownershipKey]types.UID{} staticClaim := map[ownershipKey]types.UID{} @@ -135,13 +135,17 @@ func compileInputs(in CompileInput, rejected map[types.UID]string) (CompileResul result.Reasons[reg.UID] = "InvalidTTL" continue } - if !refMatches(reg.Spec.DNSZoneRef, in.Zone.Name, in.Zone.UID, in.Zone.Generation) { + if reg.Spec.DNSZoneRef.Name != in.Zone.Name { continue } if reg.UID == "" { result.Reasons[reg.UID] = "MissingUID" continue } + if !refMatches(in.RegistrationZoneReferences[reg.UID], in.Zone.Name, in.Zone.UID, in.Zone.Generation) { + result.Reasons[reg.UID] = "InvalidReference" + continue + } name := zoneOwner(reg.Spec.Name, result.Plan.ZoneApex) valid := true for _, rt := range reg.Spec.RecordTypes { @@ -487,13 +491,13 @@ func sortAggregate(records []dnsv1alpha1.RecordEntry, uids []types.UID) { } } -func associatedContextUIDs(zone *dnsv1alpha1.DNSZone, associations []dnsv1alpha1.DNSZoneAssociation) []types.UID { +func associatedContextUIDs(zone *dnsv1alpha1.DNSZone, associations []dnsv1alpha1.DNSZoneAssociation, references map[types.UID]associationReferences) []types.UID { contextsByUID := map[types.UID]struct{}{} for i := range associations { a := &associations[i] - accepted := apimeta.FindStatusCondition(a.Status.Conditions, "Accepted") - consumerUID := a.Status.ResolvedResolverContextRef.UID - if a.DeletionTimestamp.IsZero() && accepted != nil && accepted.Status == metav1.ConditionTrue && refMatches(a.Status.ResolvedDNSZoneRef, zone.Name, zone.UID, zone.Generation) && consumerUID != "" { + refs := references[a.UID] + consumerUID := refs.Context.UID + if associationMatchesZone(a, zone, refs) && consumerUID != "" { contextsByUID[consumerUID] = struct{}{} } } diff --git a/internal/internaldns/controlplane/compiler_test.go b/internal/internaldns/controlplane/compiler_test.go index 17839ed2..fd119610 100644 --- a/internal/internaldns/controlplane/compiler_test.go +++ b/internal/internaldns/controlplane/compiler_test.go @@ -184,18 +184,34 @@ func TestCompileSRVTargetUsesAbsoluteZoneIdentity(t *testing.T) { func fixture(now time.Time) CompileInput { zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "private", UID: "zone-a", Generation: 2}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "corp.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - assoc := func(name string, vpc types.UID) dnsv1alpha1.DNSZoneAssociation { - return dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(name)}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, ResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: name, UID: vpc}}, Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID, Generation: zone.Generation}, ResolvedResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: name, UID: vpc}, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}}} + assoc := func(name string) dnsv1alpha1.DNSZoneAssociation { + return dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(name)}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: name}}, Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{Name: name}, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}}} } - reg := dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "api", UID: "registration-a", Generation: 3}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID, Generation: zone.Generation}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeAAAA}, TTLSeconds: 30}} + reg := dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "api", UID: "registration-a", Generation: 3}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeAAAA}, TTLSeconds: 30}} c := dnsv1alpha1.DNSRecordContribution{ObjectMeta: metav1.ObjectMeta{Name: "endpoint-a", UID: "contribution-a", Generation: 18}, Spec: dnsv1alpha1.DNSRecordContributionSpec{RegistrationRef: dnsv1alpha1.DNSObjectReference{Name: reg.Name, UID: reg.UID, Generation: reg.Generation}, RecordSets: []dnsv1alpha1.DNSContributionRecordSet{{RecordType: dnsv1alpha1.RRTypeAAAA, Records: []dnsv1alpha1.RecordEntry{{Name: "api.corp.internal", AAAA: &dnsv1alpha1.AAAARecordSpec{Content: "fd00::1"}}}}}, Eligible: true}, Status: dnsv1alpha1.DNSRecordContributionStatus{ObservedGeneration: 4}} - return CompileInput{Zone: zone, Associations: []dnsv1alpha1.DNSZoneAssociation{assoc("b", "vpc-b"), assoc("a", "vpc-a")}, Registrations: []dnsv1alpha1.DNSRegistration{reg}, Contributions: []dnsv1alpha1.DNSRecordContribution{c}, Now: now} + zoneRef := dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID, Generation: zone.Generation} + return CompileInput{Zone: zone, Associations: []dnsv1alpha1.DNSZoneAssociation{assoc("b"), assoc("a")}, AssociationReferences: map[types.UID]associationReferences{ + "a": {Zone: zoneRef, Context: dnsv1alpha1.DNSObjectReference{Name: "a", UID: "vpc-a"}}, + "b": {Zone: zoneRef, Context: dnsv1alpha1.DNSObjectReference{Name: "b", UID: "vpc-b"}}, + }, RegistrationZoneReferences: map[types.UID]dnsv1alpha1.DNSObjectReference{reg.UID: zoneRef}, Registrations: []dnsv1alpha1.DNSRegistration{reg}, Contributions: []dnsv1alpha1.DNSRecordContribution{c}, Now: now} } func structLocal(name string) corev1.LocalObjectReference { return corev1.LocalObjectReference{Name: name} } +func TestCompileRejectsReplacementBehindANameOnlyReference(t *testing.T) { + in := fixture(time.Now().UTC()) + in.Zone.UID = "replacement-zone" + result, err := Compile(in) + if err != nil { + t.Fatal(err) + } + if result.Accepted[in.Registrations[0].UID] || result.Reasons[in.Registrations[0].UID] != "InvalidReference" || len(result.Plan.RRsets) != 0 { + t.Fatalf("replacement reused a registration's private reference: %#v", result) + } +} + func TestCompileRejectsPrivateAuthorityOverridesWithoutBlockingHealthyRecords(t *testing.T) { for _, tt := range []struct { name string @@ -214,6 +230,7 @@ func TestCompileRejectsPrivateAuthorityOverridesWithoutBlockingHealthyRecords(t bad.Spec.Name = tt.entry.Name bad.Spec.RecordTypes = []dnsv1alpha1.RRType{tt.recordType} in.Registrations = append(in.Registrations, *bad) + in.RegistrationZoneReferences[bad.UID] = in.RegistrationZoneReferences[in.Registrations[0].UID] got, err := Compile(in) if err != nil { t.Fatal(err) @@ -330,6 +347,8 @@ func TestCompileRejectedRegistrationDoesNotClaimOtherTypes(t *testing.T) { valid.CreationTimestamp = metav1.NewTime(time.Unix(3, 0)) valid.Spec.RecordTypes = []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA} in.Registrations = []dnsv1alpha1.DNSRegistration{rejected, valid, original} + in.RegistrationZoneReferences[rejected.UID] = in.RegistrationZoneReferences[original.UID] + in.RegistrationZoneReferences[valid.UID] = in.RegistrationZoneReferences[original.UID] got, err := Compile(in) if err != nil { t.Fatal(err) @@ -352,6 +371,7 @@ func TestCompileInvalidSRVDoesNotBlockWithdrawal(t *testing.T) { reg.Name, reg.UID, reg.Spec.Name = "srv", "srv", "_grpc._tcp.svc" reg.Spec.RecordTypes = []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeSRV} in.Registrations = append(in.Registrations, reg) + in.RegistrationZoneReferences[reg.UID] = in.RegistrationZoneReferences[in.Registrations[0].UID] c := *in.Contributions[0].DeepCopy() c.Name, c.UID, c.Spec.Eligible = "srv", "srv-contribution", true c.Spec.RegistrationRef = dnsv1alpha1.DNSObjectReference{Name: reg.Name, UID: reg.UID, Generation: reg.Generation} diff --git a/internal/internaldns/controlplane/context_reconciler.go b/internal/internaldns/controlplane/context_reconciler.go index 33a2d8dd..6ccd1127 100644 --- a/internal/internaldns/controlplane/context_reconciler.go +++ b/internal/internaldns/controlplane/context_reconciler.go @@ -27,57 +27,85 @@ import ( func jsonEqual(a, b any) bool { return reflect.DeepEqual(a, b) } +// pinNamedReference preserves a resolved lifetime while the selected name is unchanged. +// Recreate the association or policy to attach a replacement with the same name. +func pinNamedReference(name string, previous, current dnsv1alpha1.DNSObjectReference) (dnsv1alpha1.DNSObjectReference, bool) { + if name == "" { + return dnsv1alpha1.DNSObjectReference{}, false + } + if previous.Name != name { + previous = dnsv1alpha1.DNSObjectReference{Name: name} + } + if current.Name != name || current.UID == "" || (previous.UID != "" && previous.UID != current.UID) { + return previous, false + } + return current, true +} + func (r *Reconciler) reconcileNamingPolicies(ctx context.Context, items []dnsv1alpha1.DNSNamingPolicy, assocs []dnsv1alpha1.DNSZoneAssociation, zones map[string]*dnsv1alpha1.DNSZone, now time.Time) error { + associationPins, err := r.associationReferences(ctx, assocs) + if err != nil { + return err + } for i := range items { p := &items[i] - contextUID, contextName := types.UID(""), "" - valid := false - + base := p.DeepCopy() var resolverContext dnsv1alpha1.DNSResolverContext err := r.Get(ctx, client.ObjectKey{Namespace: p.Namespace, Name: p.Spec.ResolverContextRef.Name}, &resolverContext) - ready := apimeta.FindStatusCondition(resolverContext.Status.Conditions, "Ready") - valid = err == nil && p.Spec.ResolverContextRef.UID != "" && p.Spec.ResolverContextRef.UID == resolverContext.UID && resolverContext.DeletionTimestamp.IsZero() && ready != nil && ready.Status == metav1.ConditionTrue && ready.ObservedGeneration == resolverContext.Generation - if valid { - contextUID = resolverContext.UID - contextName = resolverContext.Name + if err != nil && !apierrors.IsNotFound(err) { + return err } - - resolved := []dnsv1alpha1.DNSResolvedAdditionalNameRule{} + current := dnsv1alpha1.DNSObjectReference{Name: p.Spec.ResolverContextRef.Name} + if err == nil && resolverContext.DeletionTimestamp.IsZero() { + current = dnsv1alpha1.DNSObjectReference{Name: resolverContext.Name, UID: resolverContext.UID} + } + selected := map[string]dnsv1alpha1.DNSObjectReference{"context": current} + for _, rule := range p.Spec.AdditionalNames { + ref := dnsv1alpha1.DNSObjectReference{Name: rule.DNSZoneRef.Name} + if zone := zones[rule.DNSZoneRef.Name]; zone != nil && zone.DeletionTimestamp.IsZero() && zone.Spec.Visibility == dnsv1alpha1.DNSZoneVisibilityPrivate { + ref.UID, ref.Generation = zone.UID, zone.Generation + } + selected["zone/"+rule.DNSZoneRef.Name] = ref + } + pins, contextCurrent, err := r.pinReferences(ctx, p, selected) + if err != nil { + return err + } + contextPin := pins["context"] + ready := apimeta.FindStatusCondition(resolverContext.Status.Conditions, "Ready") + valid := contextCurrent && p.DeletionTimestamp.IsZero() && ready != nil && ready.Status == metav1.ConditionTrue && ready.ObservedGeneration == resolverContext.Generation + p.Status.ResolvedResolverContextRef = dnsv1alpha1.DNSNameReference{Name: contextPin.Name} + resolved := make([]dnsv1alpha1.DNSResolvedAdditionalNameRule, 0, len(p.Spec.AdditionalNames)) seen := map[string]bool{} for _, rule := range p.Spec.AdditionalNames { - z := zones[rule.DNSZoneRef.Name] - associated := associatedWithContext(assocs, zUID(z), contextUID) - - if z == nil || z.Spec.Visibility != dnsv1alpha1.DNSZoneVisibilityPrivate || !refMatches(rule.DNSZoneRef, z.Name, z.UID, z.Generation) || !associated { + zonePin := pins["zone/"+rule.DNSZoneRef.Name] + if !contextCurrent || !associatedWithContext(assocs, associationPins, zonePin.UID, contextPin.UID) { valid = false - continue } - selector, err := metav1.LabelSelectorAsSelector(rule.Selector.AsLabelSelector()) - if err != nil { + selector, selectorErr := namingSelector(rule.Selector.AsLabelSelector()) + selectorKey := "" + if selectorErr != nil { valid = false - continue + } else { + selectorKey = selector.String() } - key := rule.DNSZoneRef.Name + "\x00" + canonicalName(rule.NamePrefix) + "\x00" + selector.String() + key := rule.DNSZoneRef.Name + "\x00" + canonicalName(rule.NamePrefix) + "\x00" + selectorKey if rule.NamePrefix == "" || seen[key] { valid = false - continue } seen[key] = true - resolved = append(resolved, dnsv1alpha1.DNSResolvedAdditionalNameRule{Selector: rule.Selector.AsLabelSelector().DeepCopy(), DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: z.Name, UID: z.UID, Generation: z.Generation}, NamePrefix: canonicalName(rule.NamePrefix)}) + resolved = append(resolved, dnsv1alpha1.DNSResolvedAdditionalNameRule{Selector: rule.Selector.AsLabelSelector().DeepCopy(), DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zonePin.Name}, NamePrefix: canonicalName(rule.NamePrefix)}) } - base := p.DeepCopy() - - p.Status.ResolvedResolverContextRef = dnsv1alpha1.DNSObjectReference{Name: contextName, UID: contextUID} - p.Status.ResolvedAdditionalNames = resolved status := metav1.ConditionTrue reason, msg := dnsValueAccepted, "naming policy is valid" if !valid { status = metav1.ConditionFalse reason = "InvalidPolicy" - msg = "all rules require an associated private zone and unique naming rules" + msg = "rules require current resource lifetimes, associated private zones, valid label selectors, and unique naming rules" } - if setCondition(&p.Status.Conditions, dnsValueAccepted, status, reason, msg, p.Generation, now) { + setCondition(&p.Status.Conditions, dnsValueAccepted, status, reason, msg, p.Generation, now) + if !jsonEqual(base.Status, p.Status) { if err := r.Status().Patch(ctx, p, client.MergeFrom(base)); err != nil { return err } @@ -86,17 +114,19 @@ func (r *Reconciler) reconcileNamingPolicies(ctx context.Context, items []dnsv1a return nil } -func zUID(z *dnsv1alpha1.DNSZone) types.UID { - if z == nil { - return "" - } - return z.UID +func associationAccepted(a *dnsv1alpha1.DNSZoneAssociation) bool { + accepted := apimeta.FindStatusCondition(a.Status.Conditions, dnsValueAccepted) + return a.DeletionTimestamp.IsZero() && accepted != nil && accepted.Status == metav1.ConditionTrue && accepted.ObservedGeneration == a.Generation } -func associatedWithContext(assocs []dnsv1alpha1.DNSZoneAssociation, zoneUID, contextUID types.UID) bool { +func associationMatchesZone(a *dnsv1alpha1.DNSZoneAssociation, z *dnsv1alpha1.DNSZone, refs associationReferences) bool { + return z != nil && associationAccepted(a) && refMatches(refs.Zone, z.Name, z.UID, z.Generation) +} + +func associatedWithContext(assocs []dnsv1alpha1.DNSZoneAssociation, pins map[types.UID]associationReferences, zoneUID, contextUID types.UID) bool { for _, a := range assocs { - c := apimeta.FindStatusCondition(a.Status.Conditions, dnsValueAccepted) - if c != nil && c.Status == metav1.ConditionTrue && a.Status.ResolvedDNSZoneRef.UID == zoneUID && a.Status.ResolvedResolverContextRef.UID == contextUID { + refs := pins[a.UID] + if associationAccepted(&a) && refs.Zone.UID == zoneUID && refs.Context.UID == contextUID { return true } } @@ -171,46 +201,50 @@ func (r *Reconciler) reconcileResolverContexts(ctx context.Context, ns string, n func (r *Reconciler) reconcileAssociations(ctx context.Context, items []dnsv1alpha1.DNSZoneAssociation, zones map[string]*dnsv1alpha1.DNSZone, now time.Time) error { type apexKey struct { - resolverContextUID types.UID - apex string + context types.UID + apex string } owners := map[apexKey]types.UID{} sort.Slice(items, func(i, j int) bool { return items[i].Name < items[j].Name }) for i := range items { a := &items[i] + base := a.DeepCopy() z := zones[a.Spec.DNSZoneRef.Name] - status := metav1.ConditionTrue - reason, msg := dnsValueAccepted, "association accepted" - contextUID, contextErr := types.UID(""), error(nil) - + currentZone := dnsv1alpha1.DNSObjectReference{Name: a.Spec.DNSZoneRef.Name} + if z != nil && z.DeletionTimestamp.IsZero() && z.Spec.Visibility == dnsv1alpha1.DNSZoneVisibilityPrivate { + currentZone = dnsv1alpha1.DNSObjectReference{Name: z.Name, UID: z.UID, Generation: z.Generation} + } var resolverContext dnsv1alpha1.DNSResolverContext - contextErr = r.Get(ctx, client.ObjectKey{Namespace: a.Namespace, Name: a.Spec.ResolverContextRef.Name}, &resolverContext) - if contextErr == nil && (a.Spec.ResolverContextRef.UID == "" || a.Spec.ResolverContextRef.UID != resolverContext.UID || !resolverContext.DeletionTimestamp.IsZero()) { - contextErr = errors.New("resolver context lifetime is not current") + lookupErr := r.Get(ctx, client.ObjectKey{Namespace: a.Namespace, Name: a.Spec.ResolverContextRef.Name}, &resolverContext) + if lookupErr != nil && !apierrors.IsNotFound(lookupErr) { + return lookupErr } - if contextErr == nil { - contextUID = resolverContext.UID + currentContext := dnsv1alpha1.DNSObjectReference{Name: a.Spec.ResolverContextRef.Name} + if lookupErr == nil && resolverContext.DeletionTimestamp.IsZero() { + currentContext = dnsv1alpha1.DNSObjectReference{Name: resolverContext.Name, UID: resolverContext.UID} } - - if !a.DeletionTimestamp.IsZero() || z == nil || z.Spec.Visibility != dnsv1alpha1.DNSZoneVisibilityPrivate || contextErr != nil || !refMatches(a.Spec.DNSZoneRef, z.Name, z.UID, z.Generation) { + pins, referencesCurrent, err := r.pinReferences(ctx, a, map[string]dnsv1alpha1.DNSObjectReference{"zone": currentZone, "context": currentContext}) + if err != nil { + return err + } + zonePin, contextPin := pins["zone"], pins["context"] + a.Status.ResolvedDNSZoneRef = dnsv1alpha1.DNSNameReference{Name: zonePin.Name} + a.Status.ResolvedResolverContextRef = dnsv1alpha1.DNSNameReference{Name: contextPin.Name} + status := metav1.ConditionTrue + reason, msg := dnsValueAccepted, "association accepted" + if !a.DeletionTimestamp.IsZero() || !referencesCurrent { status = metav1.ConditionFalse reason = "InvalidReference" - msg = "private zone and pinned context/zone UIDs are required" - } else if old := owners[apexKey{contextUID, canonicalName(z.Spec.DomainName)}]; old != "" && old != z.UID { + msg = "private zone and resolver context must match their resolved lifetimes; recreate the association to reattach replacements" + } else if old := owners[apexKey{contextPin.UID, canonicalName(z.Spec.DomainName)}]; old != "" && old != z.UID { status = metav1.ConditionFalse reason = "ApexConflict" - msg = "another zone owns this apex in the resolver context" + msg = "another zone owns this apex in the DNS context" } else { - owners[apexKey{contextUID, canonicalName(z.Spec.DomainName)}] = z.UID - } - base := a.DeepCopy() - - a.Status.ResolvedResolverContextRef = dnsv1alpha1.DNSObjectReference{Name: a.Spec.ResolverContextRef.Name, UID: contextUID} - - if z != nil { - a.Status.ResolvedDNSZoneRef = dnsv1alpha1.DNSObjectReference{Name: z.Name, UID: z.UID, Generation: z.Generation} + owners[apexKey{contextPin.UID, canonicalName(z.Spec.DomainName)}] = z.UID } - if setCondition(&a.Status.Conditions, dnsValueAccepted, status, reason, msg, a.Generation, now) { + setCondition(&a.Status.Conditions, dnsValueAccepted, status, reason, msg, a.Generation, now) + if !jsonEqual(base.Status, a.Status) { if err := r.Status().Patch(ctx, a, client.MergeFrom(base)); err != nil { return err } @@ -224,6 +258,10 @@ func (r *Reconciler) reconcileBindings(ctx context.Context, ns string, assocs [] } func (r *Reconciler) reconcileContextBindings(ctx context.Context, ns string, assocs []dnsv1alpha1.DNSZoneAssociation, zones map[string]*dnsv1alpha1.DNSZone, now time.Time) error { + associationPins, err := r.associationReferences(ctx, assocs) + if err != nil { + return err + } var accesses dnsv1alpha1.DNSResolverAccessBindingList var existing dnsv1alpha1.DNSResolverBindingList if err := r.List(ctx, &accesses, client.InNamespace(ns)); err != nil { @@ -235,9 +273,9 @@ func (r *Reconciler) reconcileContextBindings(ctx context.Context, ns string, as zonesByContext := map[types.UID][]types.UID{} for _, a := range assocs { z := zones[a.Spec.DNSZoneRef.Name] - accepted := apimeta.FindStatusCondition(a.Status.Conditions, dnsValueAccepted) - if z != nil && accepted != nil && accepted.Status == metav1.ConditionTrue { - zonesByContext[a.Status.ResolvedResolverContextRef.UID] = append(zonesByContext[a.Status.ResolvedResolverContextRef.UID], z.UID) + refs := associationPins[a.UID] + if associationMatchesZone(&a, z, refs) { + zonesByContext[refs.Context.UID] = append(zonesByContext[refs.Context.UID], z.UID) } } sort.Slice(accesses.Items, func(i, j int) bool { return accesses.Items[i].Name < accesses.Items[j].Name }) @@ -299,7 +337,6 @@ func (r *Reconciler) reconcileContextBindings(ctx context.Context, ns string, as } setCondition(&a.Status.Conditions, dnsValueAccepted, status, reason, msg, a.Generation, now) if !valid { - a.Status.BindingRef = nil setCondition(&a.Status.Conditions, "Ready", metav1.ConditionFalse, "AccessRejected", "access binding is not accepted", a.Generation, now) if !jsonEqual(base.Status, a.Status) { if err := r.Status().Patch(ctx, a, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})); err != nil { @@ -364,12 +401,12 @@ func (r *Reconciler) reconcileContextBindings(ctx context.Context, ns string, as binding = &old } a.Status.ObservedGeneration = a.Generation - a.Status.BindingRef = &dnsv1alpha1.DNSObjectReference{Name: binding.Name, UID: binding.UID} - setCondition(&a.Status.Conditions, "Ready", metav1.ConditionFalse, "PendingReplicaVerification", "serving fleet acknowledgement is pending", a.Generation, now) if c := apimeta.FindStatusCondition(binding.Status.Conditions, "Serving"); c != nil && c.Status == metav1.ConditionTrue && binding.Status.ObservedConfigurationRevision == binding.Spec.Configuration.Revision && bindingAckCurrent(binding, now) { setCondition(&a.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "serving capacity applied this access generation", a.Generation, now) } else if c != nil && c.Status == metav1.ConditionUnknown { setCondition(&a.Status.Conditions, "Ready", metav1.ConditionUnknown, "ObservationUnavailable", "current serving health is unavailable", a.Generation, now) + } else { + setCondition(&a.Status.Conditions, "Ready", metav1.ConditionFalse, "PendingReplicaVerification", "serving fleet acknowledgement is pending", a.Generation, now) } if !jsonEqual(base.Status, a.Status) { if err := r.Status().Patch(ctx, a, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})); err != nil { @@ -551,8 +588,8 @@ func (r *Reconciler) createIfAbsent(ctx context.Context, o client.Object) error return err } -func setCondition(cs *[]metav1.Condition, t string, status metav1.ConditionStatus, reason, msg string, generation int64, now time.Time) bool { - return apimeta.SetStatusCondition(cs, metav1.Condition{Type: t, Status: status, Reason: reason, Message: msg, ObservedGeneration: generation, LastTransitionTime: metav1.NewTime(now)}) +func setCondition(cs *[]metav1.Condition, t string, status metav1.ConditionStatus, reason, msg string, generation int64, now time.Time) { + apimeta.SetStatusCondition(cs, metav1.Condition{Type: t, Status: status, Reason: reason, Message: msg, ObservedGeneration: generation, LastTransitionTime: metav1.NewTime(now)}) } // BindingAddressCandidates supplies stable IPv6 candidates to a durable global @@ -634,7 +671,7 @@ func (r *Reconciler) reconcileManagedZone(ctx context.Context, c *dnsv1alpha1.DN if err := r.Get(ctx, client.ObjectKeyFromObject(zone), zone); err != nil { return false, err } - association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: zoneName + "-context", Namespace: c.Namespace, Labels: map[string]string{"dns.networking.miloapis.com/managed": "true"}}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, ResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: c.Name, UID: c.UID}}} + association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: zoneName + "-context", Namespace: c.Namespace, Labels: map[string]string{"dns.networking.miloapis.com/managed": "true"}}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: c.Name}}} if r.Scheme != nil { if err := controllerutil.SetControllerReference(c, association, r.Scheme, controllerutil.WithBlockOwnerDeletion(false)); err != nil { return false, err @@ -647,9 +684,13 @@ func (r *Reconciler) reconcileManagedZone(ctx context.Context, c *dnsv1alpha1.DN return false, err } accepted := apimeta.FindStatusCondition(association.Status.Conditions, dnsValueAccepted) - ready := metav1.IsControlledBy(zone, c) && metav1.IsControlledBy(association, c) && zone.Spec.Visibility == dnsv1alpha1.DNSZoneVisibilityPrivate && zone.Spec.DomainName == suffix && zone.DeletionTimestamp.IsZero() && accepted != nil && accepted.Status == metav1.ConditionTrue && accepted.ObservedGeneration == association.Generation && association.Status.ResolvedDNSZoneRef.UID == zone.UID && association.Status.ResolvedResolverContextRef.UID == c.UID + refs, err := r.readReferences(ctx, association) + if err != nil { + return false, err + } + ready := metav1.IsControlledBy(zone, c) && metav1.IsControlledBy(association, c) && zone.Spec.Visibility == dnsv1alpha1.DNSZoneVisibilityPrivate && zone.Spec.DomainName == suffix && zone.DeletionTimestamp.IsZero() && accepted != nil && accepted.Status == metav1.ConditionTrue && accepted.ObservedGeneration == association.Generation && refMatches(refs["zone"], zone.Name, zone.UID, zone.Generation) && refMatches(refs["context"], c.Name, c.UID, 0) if ready { - c.Status.ManagedNamespace = dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, Suffix: suffix} + c.Status.ManagedNamespace = dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Suffix: suffix} } return ready, nil } diff --git a/internal/internaldns/controlplane/context_reconciler_test.go b/internal/internaldns/controlplane/context_reconciler_test.go index c65c6ae0..0b76fa04 100644 --- a/internal/internaldns/controlplane/context_reconciler_test.go +++ b/internal/internaldns/controlplane/context_reconciler_test.go @@ -60,17 +60,18 @@ func TestContextAccessPersistsUntilExplicitWithdrawal(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } contextObject := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "ctx", Namespace: "project", UID: "context-uid", Generation: 1}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer-uid"}, Status: dnsv1alpha1.DNSResolverContextStatus{}} setCondition(&contextObject.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "", 1, now) zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "zone", Namespace: "project", UID: "zone-uid", Generation: 1}, Spec: dnsv1alpha1.DNSZoneSpec{Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: "assoc", Namespace: "project", UID: "assoc-uid", Generation: 1}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: "zone", UID: "zone-uid"}, ResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: "ctx", UID: "context-uid"}}, Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: "zone", UID: "zone-uid", Generation: 1}, ResolvedResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: "ctx", UID: "context-uid"}}} + association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: "assoc", Namespace: "project", UID: "assoc-uid", Generation: 1}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "zone"}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: "ctx"}}, Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "zone"}, ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{Name: "ctx"}}} setCondition(&association.Status.Conditions, "Accepted", metav1.ConditionTrue, "Accepted", "", 1, now) access := &dnsv1alpha1.DNSResolverAccessBinding{ObjectMeta: metav1.ObjectMeta{Name: "access", Namespace: "project", UID: "access-uid", Generation: 1}, Spec: dnsv1alpha1.DNSResolverAccessBindingSpec{ContextRef: dnsv1alpha1.DNSObjectReference{Name: "ctx", UID: "context-uid"}, Region: "central", DestinationAddress: "fd70:100::10"}} c := fake.NewClientBuilder().WithScheme(s).WithObjects(contextObject, zone, association, access).WithStatusSubresource(&dnsv1alpha1.DNSResolverContext{}, &dnsv1alpha1.DNSResolverAccessBinding{}, &dnsv1alpha1.DNSResolverAccessBinding{}, &dnsv1alpha1.DNSResolverBinding{}).Build() r := &Reconciler{Client: c, Scheme: s, Options: ReconcilerOptions{ProjectUID: "project-uid", PlatformNamespace: "platform", Region: "central", Shard: "shared", AddressAllocator: testAllocator{}}} + seedReferenceState(t, r, association, map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: zone.Name, UID: zone.UID, Generation: zone.Generation}, "context": {Name: contextObject.Name, UID: contextObject.UID}}) if err := r.reconcileContextBindings(ctx, "project", []dnsv1alpha1.DNSZoneAssociation{*association}, map[string]*dnsv1alpha1.DNSZone{"zone": zone}, now); err != nil { t.Fatal(err) } @@ -94,13 +95,39 @@ func TestContextAccessPersistsUntilExplicitWithdrawal(t *testing.T) { if access.Status.ObservedGeneration != 1 { t.Fatalf("source generation not observed: %#v", access.Status) } + until := metav1.NewTime(now.Add(time.Minute)) + b.Status.ObservedConfigurationRevision = b.Spec.Configuration.Revision + b.Status.MemberAcknowledgements = []dnsv1alpha1.DNSApplyAcknowledgement{{Phase: string(model.AckVerified), Revision: b.Spec.Configuration.Revision, AccessEpoch: b.Spec.Version.Epoch, AccessSequence: b.Spec.Version.Sequence, ValidUntil: &until}} + setCondition(&b.Status.Conditions, "Serving", metav1.ConditionTrue, "Ready", "", b.Generation, now) + if err := c.Status().Update(ctx, &b); err != nil { + t.Fatal(err) + } + for i := 0; i < 2; i++ { + if err := r.reconcileContextBindings(ctx, "project", []dnsv1alpha1.DNSZoneAssociation{*association}, map[string]*dnsv1alpha1.DNSZone{"zone": zone}, now.Add(time.Duration(i)*time.Second)); err != nil { + t.Fatal(err) + } + previousVersion := access.ResourceVersion + if err := c.Get(ctx, client.ObjectKeyFromObject(access), access); err != nil { + t.Fatal(err) + } + ready := apimeta.FindStatusCondition(access.Status.Conditions, "Ready") + if ready == nil || ready.Status != metav1.ConditionTrue || !ready.LastTransitionTime.Time.Equal(now) { + t.Fatalf("unchanged serving state changed its transition: %#v", ready) + } + if i > 0 && access.ResourceVersion != previousVersion { + t.Fatal("unchanged serving state wrote access status") + } + } + extraZone := zone.DeepCopy() extraZone.Name = "extra" extraZone.UID = "extra-uid" extraAssociation := association.DeepCopy() extraAssociation.Name = "extra-assoc" - extraAssociation.Spec.DNSZoneRef = dnsv1alpha1.DNSObjectReference{Name: extraZone.Name, UID: extraZone.UID} - extraAssociation.Status.ResolvedDNSZoneRef = extraAssociation.Spec.DNSZoneRef + extraAssociation.Spec.DNSZoneRef = dnsv1alpha1.DNSNameReference{Name: extraZone.Name} + extraAssociation.UID = "extra-association-uid" + extraAssociation.Status.ResolvedDNSZoneRef = dnsv1alpha1.DNSNameReference{Name: extraZone.Name} + seedReferenceState(t, r, extraAssociation, map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: extraZone.Name, UID: extraZone.UID, Generation: extraZone.Generation}, "context": {Name: contextObject.Name, UID: contextObject.UID}}) if err := r.reconcileContextBindings(ctx, "project", []dnsv1alpha1.DNSZoneAssociation{*association, *extraAssociation}, map[string]*dnsv1alpha1.DNSZone{"zone": zone, "extra": extraZone}, now); err != nil { t.Fatal(err) } @@ -229,20 +256,29 @@ func TestContextNamingPolicyResolvesMultipleZonesWithoutNetworking(t *testing.T) ctx := context.Background() now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } resolverContext := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "ctx", Namespace: "project", UID: "ctx-uid", Generation: 2}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer-uid"}} setCondition(&resolverContext.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "", 2, now) z1 := dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "managed", Namespace: "project", UID: "zone-1", Generation: 1}, Spec: dnsv1alpha1.DNSZoneSpec{Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} z2 := dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "services", Namespace: "project", UID: "zone-2", Generation: 3}, Spec: dnsv1alpha1.DNSZoneSpec{Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - associations := []dnsv1alpha1.DNSZoneAssociation{{Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSObjectReference{UID: z1.UID}, ResolvedResolverContextRef: dnsv1alpha1.DNSObjectReference{UID: resolverContext.UID}}}, {Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSObjectReference{UID: z2.UID}, ResolvedResolverContextRef: dnsv1alpha1.DNSObjectReference{UID: resolverContext.UID}}}} + associations := []dnsv1alpha1.DNSZoneAssociation{{Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSNameReference{}, ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{}}}, {Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSNameReference{}, ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{}}}} for i := range associations { + zone := []dnsv1alpha1.DNSZone{z1, z2}[i] + associations[i].Name = zone.Name + "-association" + associations[i].Namespace = "project" + associations[i].UID = types.UID(associations[i].Name) + associations[i].Generation = 1 setCondition(&associations[i].Status.Conditions, "Accepted", metav1.ConditionTrue, "Accepted", "", 1, now) } - policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "names", Namespace: "project", UID: "policy-uid", Generation: 1}, Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: resolverContext.Name, UID: resolverContext.UID}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{Selector: nil, DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: z1.Name, UID: z1.UID, Generation: z1.Generation}, NamePrefix: "instances"}, {Selector: nil, DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: z2.Name, UID: z2.UID, Generation: z2.Generation}, NamePrefix: "services"}}}} + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "names", Namespace: "project", UID: "policy-uid", Generation: 1}, Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolverContext.Name}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: z1.Name}, NamePrefix: "instances"}, {DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: z2.Name}, NamePrefix: "services"}}}} c := fake.NewClientBuilder().WithScheme(s).WithObjects(resolverContext, policy).WithStatusSubresource(&dnsv1alpha1.DNSResolverContext{}, &dnsv1alpha1.DNSResolverAccessBinding{}, &dnsv1alpha1.DNSNamingPolicy{}).Build() r := &Reconciler{Client: c, Options: ReconcilerOptions{}} + for i := range associations { + zone := []dnsv1alpha1.DNSZone{z1, z2}[i] + seedReferenceState(t, r, &associations[i], map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: zone.Name, UID: zone.UID, Generation: zone.Generation}, "context": {Name: resolverContext.Name, UID: resolverContext.UID}}) + } if err := r.reconcileNamingPolicies(ctx, []dnsv1alpha1.DNSNamingPolicy{*policy}, associations, map[string]*dnsv1alpha1.DNSZone{z1.Name: &z1, z2.Name: &z2}, now); err != nil { t.Fatal(err) } @@ -253,7 +289,7 @@ func TestContextNamingPolicyResolvesMultipleZonesWithoutNetworking(t *testing.T) if accepted == nil || accepted.Status != metav1.ConditionTrue || len(policy.Status.ResolvedAdditionalNames) != 2 { t.Fatalf("context naming policy not resolved: %#v", policy.Status) } - if policy.Status.ResolvedResolverContextRef.UID != resolverContext.UID { + if policy.Status.ResolvedResolverContextRef.Name != resolverContext.Name { t.Fatalf("identity projection incorrect: %#v", policy.Status) } } @@ -289,6 +325,18 @@ func TestStableOneShardPerRegionAssignment(t *testing.T) { } } +func TestAccessPlacementMatchesContextPublicationTargets(t *testing.T) { + r := &Reconciler{Options: ReconcilerOptions{ProjectUID: "project", ServingRegions: []ServingRegion{{Region: "east", Shard: "a"}, {Region: "east", Shard: "b"}, {Region: "west", Shard: "a"}}}} + for i := range 100 { + uid := types.UID(fmt.Sprintf("context-%d", i)) + for _, target := range r.selectServingTargets(uid, nil) { + if shard := r.shardForContextRegion(uid, target.Region, nil); shard != target.Shard { + t.Fatalf("binding shard %s does not match publication target %s", shard, target.Shard) + } + } + } +} + func TestNamespaceReconciliationCannotWithdrawAnotherNamespace(t *testing.T) { for _, labeled := range []bool{false, true} { t.Run(fmt.Sprintf("namespace-label-%t", labeled), func(t *testing.T) { diff --git a/internal/internaldns/controlplane/managed_domain_test.go b/internal/internaldns/controlplane/managed_domain_test.go index 9e4760b9..2855cfb3 100644 --- a/internal/internaldns/controlplane/managed_domain_test.go +++ b/internal/internaldns/controlplane/managed_domain_test.go @@ -18,7 +18,7 @@ import ( func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } domains := map[string]string{"vpc-a": "datum.internal", "vpc-b": "datum.internal", "custom": "private.example.net"} @@ -50,14 +50,12 @@ func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { if err := c.List(ctx, &associations); err != nil { t.Fatal(err) } - for i := range associations.Items { - a := &associations.Items[i] - a.Status.ResolvedDNSZoneRef = a.Spec.DNSZoneRef - a.Status.ResolvedResolverContextRef = a.Spec.ResolverContextRef - setCondition(&a.Status.Conditions, "Accepted", metav1.ConditionTrue, "Ready", "", a.Generation, now) - if err := c.Status().Update(ctx, a); err != nil { - t.Fatal(err) - } + zoneByName := map[string]*dnsv1alpha1.DNSZone{} + for i := range zones.Items { + zoneByName[zones.Items[i].Name] = &zones.Items[i] + } + if err := r.reconcileAssociations(ctx, associations.Items, zoneByName, now); err != nil { + t.Fatal(err) } if err := r.reconcileResolverContexts(ctx, "project", now); err != nil { @@ -68,7 +66,7 @@ func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { t.Fatal(err) } for _, resolver := range contexts.Items { - if resolver.Status.ManagedNamespace.Suffix != domains[resolver.Name] || resolver.Status.ManagedNamespace.DNSZoneRef.UID == "" { + if resolver.Status.ManagedNamespace.Suffix != domains[resolver.Name] || resolver.Status.ManagedNamespace.DNSZoneRef.Name == "" { t.Fatalf("unallocated context: %#v", resolver.Status) } var zone dnsv1alpha1.DNSZone @@ -89,7 +87,7 @@ func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { t.Fatal("managed association must share the resolver context lifecycle") } } - if contexts.Items[0].Status.ManagedNamespace.DNSZoneRef.UID == contexts.Items[1].Status.ManagedNamespace.DNSZoneRef.UID { + if contexts.Items[0].Status.ManagedNamespace.DNSZoneRef.Name == contexts.Items[1].Status.ManagedNamespace.DNSZoneRef.Name { t.Fatal("contexts share a zone lifetime") } zone := &zones.Items[0] @@ -105,7 +103,7 @@ func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { t.Fatal(err) } for _, resolver := range contexts.Items { - if resolver.Status.ManagedNamespace.DNSZoneRef.UID == zone.UID { + if resolver.Status.ManagedNamespace.DNSZoneRef.Name == zone.Name { t.Fatal("a mismatched zone domain remained allocated") } } @@ -115,7 +113,7 @@ func TestManagedNamespaceRequiresRequestedDomain(t *testing.T) { for _, domain := range []string{"", "bad..internal", "*.internal"} { t.Run(domain, func(t *testing.T) { s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "consumer", Namespace: "project", UID: "context-uid", Generation: 1}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "opaque-consumer", ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespace{Enabled: true, DomainSuffix: domain}}} diff --git a/internal/internaldns/controlplane/named_reference_api_test.go b/internal/internaldns/controlplane/named_reference_api_test.go new file mode 100644 index 00000000..5a8e3e92 --- /dev/null +++ b/internal/internaldns/controlplane/named_reference_api_test.go @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "testing" + "time" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func verifyNamedReferencesWithAPI(t *testing.T, cl client.Client) { + t.Helper() + ctx := context.Background() + now := time.Now().UTC() + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "named-context", Namespace: "project"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "named-consumer"}} + requireAccessTestSuccess(t, cl.Create(ctx, resolver)) + setCondition(&resolver.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "", resolver.Generation, now) + requireAccessTestSuccess(t, cl.Status().Update(ctx, resolver)) + zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "named-zone", Namespace: "project"}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "named.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate, DNSZoneClassName: "private"}} + requireAccessTestSuccess(t, cl.Create(ctx, zone)) + association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: "named-association", Namespace: "project"}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}}} + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "named-policy", Namespace: "project"}, Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{Selector: &dnsv1alpha1.DNSRegistrationSelector{MatchLabels: map[string]string{"app": "storefront"}}, DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, NamePrefix: "hosts"}}}} + requireAccessTestSuccess(t, cl.Create(ctx, association)) + requireAccessTestSuccess(t, cl.Create(ctx, policy)) + r := &Reconciler{Client: cl, Options: Options{PlatformNamespace: "platform"}} + reconcile := func(want metav1.ConditionStatus) { + t.Helper() + requireAccessTestSuccess(t, cl.Get(ctx, client.ObjectKeyFromObject(association), association)) + associations := []dnsv1alpha1.DNSZoneAssociation{*association} + zones := map[string]*dnsv1alpha1.DNSZone{zone.Name: zone} + requireAccessTestSuccess(t, r.reconcileAssociations(ctx, associations, zones, now)) + requireAccessTestSuccess(t, cl.Get(ctx, client.ObjectKeyFromObject(policy), policy)) + requireAccessTestSuccess(t, r.reconcileNamingPolicies(ctx, []dnsv1alpha1.DNSNamingPolicy{*policy}, associations, zones, now)) + requireAccessTestSuccess(t, cl.Get(ctx, client.ObjectKeyFromObject(association), association)) + requireAccessTestSuccess(t, cl.Get(ctx, client.ObjectKeyFromObject(policy), policy)) + for name, conditions := range map[string][]metav1.Condition{"association": association.Status.Conditions, "policy": policy.Status.Conditions} { + if condition := apimeta.FindStatusCondition(conditions, "Accepted"); condition == nil || condition.Status != want { + t.Fatalf("%s expected %s: %v", name, want, condition) + } + } + } + reconcile(metav1.ConditionTrue) + if selector := policy.Status.ResolvedAdditionalNames[0].Selector; selector == nil || selector.MatchLabels["app"] != "storefront" { + t.Fatal("API did not preserve the registration label selector") + } + originalUID := zone.UID + requireAccessTestSuccess(t, cl.Delete(ctx, zone)) + zone = &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: zone.Name, Namespace: "project"}, Spec: zone.Spec} + requireAccessTestSuccess(t, cl.Create(ctx, zone)) + if zone.UID == originalUID { + t.Fatal("replacement did not receive a new API-assigned UID") + } + reconcile(metav1.ConditionFalse) + associationPins, err := r.readReferences(ctx, association) + requireAccessTestSuccess(t, err) + policyPins, err := r.readReferences(ctx, policy) + requireAccessTestSuccess(t, err) + if associationPins["zone"].UID != originalUID || policyPins["zone/"+zone.Name].UID != originalUID { + t.Fatal("API-backed replacement transferred lifetime pins") + } +} diff --git a/internal/internaldns/controlplane/named_reference_test.go b/internal/internaldns/controlplane/named_reference_test.go new file mode 100644 index 00000000..df665a99 --- /dev/null +++ b/internal/internaldns/controlplane/named_reference_test.go @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "testing" + "time" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + 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 TestNamedReferencesPreserveLifetimesAcrossReplacement(t *testing.T) { + for _, replace := range []string{"zone", "context"} { + t.Run(replace, func(t *testing.T) { + ctx := context.Background() + now := time.Now().UTC() + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "application", Namespace: "project", UID: "context-original", Generation: 1}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer"}} + setCondition(&resolver.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "", resolver.Generation, now) + zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "project", UID: "zone-original", Generation: 1}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "prod.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} + association := &dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "project", UID: "source-original", Generation: 1}, Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}}} + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "project", UID: "source-original", Generation: 1}, Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, NamePrefix: "hosts"}}}} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resolver, zone, association, policy).WithStatusSubresource(association, policy).Build() + r := &Reconciler{Client: cl} + references := func(source client.Object) map[string]dnsv1alpha1.DNSObjectReference { + t.Helper() + refs, err := r.readReferences(ctx, source) + if err != nil { + t.Fatal(err) + } + return refs + } + zones := map[string]*dnsv1alpha1.DNSZone{zone.Name: zone} + reconcile := func() { + t.Helper() + if err := cl.Get(ctx, client.ObjectKeyFromObject(association), association); err != nil { + t.Fatal(err) + } + associations := []dnsv1alpha1.DNSZoneAssociation{*association} + if err := r.reconcileAssociations(ctx, associations, zones, now); err != nil { + t.Fatal(err) + } + if err := cl.Get(ctx, client.ObjectKeyFromObject(policy), policy); err != nil { + t.Fatal(err) + } + if err := r.reconcileNamingPolicies(ctx, []dnsv1alpha1.DNSNamingPolicy{*policy}, associations, zones, now); err != nil { + t.Fatal(err) + } + if err := cl.Get(ctx, client.ObjectKeyFromObject(association), association); err != nil { + t.Fatal(err) + } + if err := cl.Get(ctx, client.ObjectKeyFromObject(policy), policy); err != nil { + t.Fatal(err) + } + } + assertAccepted := func(want bool) { + t.Helper() + for name, conditions := range map[string][]metav1.Condition{"association": association.Status.Conditions, "policy": policy.Status.Conditions} { + condition := apimeta.FindStatusCondition(conditions, "Accepted") + if condition == nil || (condition.Status == metav1.ConditionTrue) != want { + t.Fatalf("%s accepted=%v, want %v", name, condition, want) + } + } + } + reconcile() + assertAccepted(true) + if references(association)["zone"].UID != zone.UID || references(policy)["context"].UID != resolver.UID { + t.Fatal("name-only references did not pin lifetimes") + } + zone.Generation++ + reconcile() + assertAccepted(true) + if references(association)["zone"].Generation != zone.Generation || references(policy)["zone/"+zone.Name].Generation != zone.Generation { + t.Fatal("resolved generations were not persisted for the same zone lifetime") + } + originalZoneUID := zone.UID + originalContextUID := resolver.UID + var replaced client.Object = zone + if replace == "context" { + replaced = resolver + } + if err := cl.Delete(ctx, replaced); err != nil { + t.Fatal(err) + } + if replace == "zone" { + delete(zones, zone.Name) + } + reconcile() + assertAccepted(false) + if references(association)["zone"].UID != originalZoneUID || references(policy)["context"].UID != originalContextUID { + t.Fatal("deletion cleared lifetime pins") + } + replaced.SetResourceVersion("") + replaced.SetUID("replacement") + if err := cl.Create(ctx, replaced); err != nil { + t.Fatal(err) + } + if replace == "zone" { + zones[zone.Name] = zone + } + reconcile() + assertAccepted(false) + if references(association)["zone"].UID != originalZoneUID || references(policy)["zone/"+zone.Name].UID != originalZoneUID || references(policy)["context"].UID != originalContextUID { + t.Fatal("replacement inherited lifetime pins") + } + // Recreate both customer resources to explicitly attach the replacement. + if err := cl.Delete(ctx, association); err != nil { + t.Fatal(err) + } + association.ResourceVersion = "" + association.UID = "association-recreated" + association.Status = dnsv1alpha1.DNSZoneAssociationStatus{} + if err := cl.Create(ctx, association); err != nil { + t.Fatal(err) + } + reconcile() + if condition := apimeta.FindStatusCondition(policy.Status.Conditions, "Accepted"); condition == nil || condition.Status != metav1.ConditionFalse { + t.Fatal("reattaching an association silently rebound the policy") + } + if err := cl.Delete(ctx, policy); err != nil { + t.Fatal(err) + } + policy.ResourceVersion = "" + policy.UID = "policy-recreated" + policy.Status = dnsv1alpha1.DNSNamingPolicyStatus{} + if err := cl.Create(ctx, policy); err != nil { + t.Fatal(err) + } + reconcile() + assertAccepted(true) + }) + } +} diff --git a/internal/internaldns/controlplane/naming.go b/internal/internaldns/controlplane/naming.go index 07299f30..62ebb44c 100644 --- a/internal/internaldns/controlplane/naming.go +++ b/internal/internaldns/controlplane/naming.go @@ -26,29 +26,27 @@ type ResolvedName struct { // additive policy names after it. It never selects an arbitrary associated // customer zone. // Selectors match DNSRegistration metadata labels supplied by the publisher. -func ResolveNaming(ctx context.Context, c client.Client, namespace string, _ types.UID, contextUID types.UID, registrationLabels map[string]string, allocatedName string) ([]ResolvedName, error) { +func ResolveNaming(ctx context.Context, c client.Client, namespace string, projectUID types.UID, contextName string, registrationLabels map[string]string, allocatedName string) ([]ResolvedName, error) { label := dnsLabel(allocatedName) if label == "" { return nil, fmt.Errorf("allocated name %q has no DNS-safe label", allocatedName) } - var managed dnsv1alpha1.DNSResolverContextList - if err := c.List(ctx, &managed, client.InNamespace(namespace)); err != nil { + var resolver dnsv1alpha1.DNSResolverContext + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: contextName}, &resolver); err != nil { return nil, err } - var out []ResolvedName - for i := range managed.Items { - m := &managed.Items[i] - accepted := apimeta.FindStatusCondition(m.Status.Conditions, "Ready") - // A Ready context without a managed namespace, or whose Ready - // condition predates its spec, has no current canonical zone. - if m.UID == contextUID && m.DeletionTimestamp.IsZero() && accepted != nil && accepted.Status == metav1.ConditionTrue && accepted.ObservedGeneration == m.Generation && m.Status.ManagedNamespace.DNSZoneRef.UID != "" { - out = append(out, ResolvedName{DNSZoneRef: m.Status.ManagedNamespace.DNSZoneRef, Name: label, Canonical: true}) - break - } + ready := apimeta.FindStatusCondition(resolver.Status.Conditions, "Ready") + // A Ready context without a managed namespace, or whose Ready + // condition predates its spec, has no current canonical zone. + if !resolver.DeletionTimestamp.IsZero() || ready == nil || ready.Status != metav1.ConditionTrue || ready.ObservedGeneration != resolver.Generation || resolver.Status.ManagedNamespace.DNSZoneRef.Name == "" { + return nil, fmt.Errorf("managed DNS namespace for context %s is not ready", contextName) } - if len(out) == 0 { - return nil, fmt.Errorf("managed DNS namespace for resolver context %s is not ready", contextUID) + var canonical dnsv1alpha1.DNSZone + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: resolver.Status.ManagedNamespace.DNSZoneRef.Name}, &canonical); err != nil { + return nil, err } + out := []ResolvedName{{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: canonical.Name, UID: canonical.UID}, Name: label, Canonical: true}} + r := &Reconciler{Client: c, Options: Options{ProjectUID: projectUID}} seen := map[string]bool{string(out[0].DNSZoneRef.UID) + "\x00" + out[0].Name: true} var policies dnsv1alpha1.DNSNamingPolicyList if err := c.List(ctx, &policies, client.InNamespace(namespace)); err != nil { @@ -63,7 +61,14 @@ func ResolveNaming(ctx context.Context, c client.Client, namespace string, _ typ for i := range policies.Items { p := &policies.Items[i] accepted := apimeta.FindStatusCondition(p.Status.Conditions, dnsValueAccepted) - if p.Status.ResolvedResolverContextRef.UID != contextUID || !p.DeletionTimestamp.IsZero() || accepted == nil || accepted.Status != metav1.ConditionTrue || accepted.ObservedGeneration != p.Generation { + if p.Status.ResolvedResolverContextRef.Name != contextName || !p.DeletionTimestamp.IsZero() || accepted == nil || accepted.Status != metav1.ConditionTrue || accepted.ObservedGeneration != p.Generation { + continue + } + pins, err := r.readReferences(ctx, p) + if err != nil { + return nil, err + } + if pins["context"].UID != resolver.UID { continue } for _, rule := range p.Status.ResolvedAdditionalNames { @@ -72,9 +77,17 @@ func ResolveNaming(ctx context.Context, c client.Client, namespace string, _ typ return nil, fmt.Errorf("naming policy %s has an invalid selector: %w", p.Name, err) } name := label + "." + canonicalName(rule.NamePrefix) - key := string(rule.DNSZoneRef.UID) + "\x00" + name + zoneRef := pins["zone/"+rule.DNSZoneRef.Name] + var zone dnsv1alpha1.DNSZone + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: zoneRef.Name}, &zone); err != nil { + return nil, err + } + if !refMatches(zoneRef, zone.Name, zone.UID, zone.Generation) { + continue + } + key := string(zoneRef.UID) + "\x00" + name if selector.Matches(labels.Set(registrationLabels)) && !seen[key] { - out = append(out, ResolvedName{DNSZoneRef: rule.DNSZoneRef, Name: name}) + out = append(out, ResolvedName{DNSZoneRef: zoneRef, Name: name}) seen[key] = true } } diff --git a/internal/internaldns/controlplane/naming_selector_test.go b/internal/internaldns/controlplane/naming_selector_test.go new file mode 100644 index 00000000..b8df2b13 --- /dev/null +++ b/internal/internaldns/controlplane/naming_selector_test.go @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "reflect" + "testing" + "time" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + 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 TestNamingSelectorsMatchRegistrationLabelsWithinContext(t *testing.T) { + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "consumer-a", Namespace: "project", UID: "context-a"}, Status: dnsv1alpha1.DNSResolverContextStatus{ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "canonical"}}, Conditions: []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue}}}} + canonical := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "canonical", Namespace: "project", UID: "canonical-zone"}} + custom := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "custom", Namespace: "project", UID: "custom-zone"}} + zone := dnsv1alpha1.DNSNameReference{Name: custom.Name} + storefront := &metav1.LabelSelector{MatchLabels: map[string]string{"app": "storefront"}} + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "names", Namespace: "project", UID: "names-uid", Generation: 1}, Status: dnsv1alpha1.DNSNamingPolicyStatus{ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue, ObservedGeneration: 1}}, ResolvedAdditionalNames: []dnsv1alpha1.DNSResolvedAdditionalNameRule{ + {DNSZoneRef: zone, NamePrefix: "all"}, + {Selector: &metav1.LabelSelector{}, DNSZoneRef: zone, NamePrefix: "empty"}, + {Selector: storefront, DNSZoneRef: zone, NamePrefix: "storefront"}, + {Selector: &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn, Values: []string{"storefront"}}}}, DNSZoneRef: zone, NamePrefix: "storefront"}, + {Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "storefront"}, MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "tier", Operator: metav1.LabelSelectorOpIn, Values: []string{"backend", "worker"}}, {Key: "disabled", Operator: metav1.LabelSelectorOpDoesNotExist}}}, DNSZoneRef: zone, NamePrefix: "backend"}, + }}} + other := policy.DeepCopy() + other.Name = "other-context" + other.Status.ResolvedResolverContextRef.Name = "consumer-b" + other.Status.ResolvedAdditionalNames = []dnsv1alpha1.DNSResolvedAdditionalNameRule{{DNSZoneRef: zone, NamePrefix: "other-context"}} + stale := policy.DeepCopy() + stale.Name = "stale-policy" + stale.Generation++ + stale.Status.ResolvedAdditionalNames = []dnsv1alpha1.DNSResolvedAdditionalNameRule{{DNSZoneRef: zone, NamePrefix: "stale"}} + foreign := policy.DeepCopy() + foreign.Name = "other-namespace" + foreign.Namespace = "other-project" + foreign.Status.ResolvedAdditionalNames = []dnsv1alpha1.DNSResolvedAdditionalNameRule{{DNSZoneRef: zone, NamePrefix: "foreign"}} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resolver, canonical, custom, policy, other, stale, foreign).Build() + r := &Reconciler{Client: cl, Options: Options{ProjectUID: "project-a"}} + seedReferenceState(t, r, policy, map[string]dnsv1alpha1.DNSObjectReference{"context": {Name: resolver.Name, UID: resolver.UID}, "zone/custom": {Name: custom.Name, UID: custom.UID}}) + for _, tc := range []struct { + name string + labels map[string]string + want []string + }{ + {"unlabeled", nil, []string{"web-01", "web-01.all", "web-01.empty"}}, + {"matching publisher one", map[string]string{"app": "storefront", "publisher": "one"}, []string{"web-01", "web-01.all", "web-01.empty", "web-01.storefront"}}, + {"matching publisher two", map[string]string{"app": "storefront", "publisher": "two"}, []string{"web-01", "web-01.all", "web-01.empty", "web-01.storefront"}}, + {"different app", map[string]string{"app": "billing"}, []string{"web-01", "web-01.all", "web-01.empty"}}, + {"set expression", map[string]string{"app": "storefront", "tier": "worker"}, []string{"web-01", "web-01.all", "web-01.empty", "web-01.storefront", "web-01.backend"}}, + {"excluded label", map[string]string{"app": "storefront", "tier": "worker", "disabled": "true"}, []string{"web-01", "web-01.all", "web-01.empty", "web-01.storefront"}}, + } { + t.Run(tc.name, func(t *testing.T) { + names, err := ResolveNaming(context.Background(), cl, "project", "project-a", "consumer-a", tc.labels, "Web 01") + if err != nil { + t.Fatal(err) + } + got := make([]string, len(names)) + for i, n := range names { + got[i] = n.Name + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("got %v want %v", got, tc.want) + } + if !names[0].Canonical { + t.Fatal("default name was not retained") + } + }) + } + t.Run("malformed accepted selector", func(t *testing.T) { + if err := cl.Get(context.Background(), client.ObjectKeyFromObject(policy), policy); err != nil { + t.Fatal(err) + } + policy.Status.ResolvedAdditionalNames[0].Selector = &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{Key: "app", Operator: "Unsupported"}}} + if err := cl.Update(context.Background(), policy); err != nil { + t.Fatal(err) + } + names, err := ResolveNaming(context.Background(), cl, "project", "project-a", "consumer-a", nil, "Web 01") + if err == nil || len(names) != 0 { + t.Fatalf("malformed accepted selector produced names: %v, %v", names, err) + } + }) +} + +func TestNamingPolicyValidatesSelectorsAndDuplicateRules(t *testing.T) { + ctx := context.Background() + now := time.Now() + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "context", Namespace: "project", UID: "context-uid", Generation: 1}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer"}} + setCondition(&resolver.Status.Conditions, "Ready", metav1.ConditionTrue, "Ready", "", 1, now) + zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "zone", UID: "zone-uid", Generation: 1}, Spec: dnsv1alpha1.DNSZoneSpec{Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} + association := dnsv1alpha1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: "association", Namespace: "project", UID: "association-uid", Generation: 1}, Status: dnsv1alpha1.DNSZoneAssociationStatus{ResolvedDNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}}} + setCondition(&association.Status.Conditions, "Accepted", metav1.ConditionTrue, "Accepted", "", 1, now) + for _, tc := range []struct { + name string + selectors []*dnsv1alpha1.DNSRegistrationSelector + valid bool + }{ + {"omitted", []*dnsv1alpha1.DNSRegistrationSelector{nil}, true}, + {"empty", []*dnsv1alpha1.DNSRegistrationSelector{{}}, true}, + {"labels", []*dnsv1alpha1.DNSRegistrationSelector{{MatchLabels: map[string]string{"app": "storefront"}}}, true}, + {"invalid-key", []*dnsv1alpha1.DNSRegistrationSelector{{MatchLabels: map[string]string{"bad key": "value"}}}, false}, + {"invalid-operator", []*dnsv1alpha1.DNSRegistrationSelector{{MatchExpressions: []dnsv1alpha1.DNSRegistrationSelectorRequirement{{Key: "app", Operator: "Unsupported"}}}}, false}, + {"missing-values", []*dnsv1alpha1.DNSRegistrationSelector{{MatchExpressions: []dnsv1alpha1.DNSRegistrationSelectorRequirement{{Key: "app", Operator: metav1.LabelSelectorOpIn}}}}, false}, + {"duplicate", []*dnsv1alpha1.DNSRegistrationSelector{nil, {}}, false}, + {"overlapping", []*dnsv1alpha1.DNSRegistrationSelector{nil, {MatchLabels: map[string]string{"app": "storefront"}}}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: tc.name, Namespace: "project", Generation: 1}, Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}}} + for _, selector := range tc.selectors { + policy.Spec.AdditionalNames = append(policy.Spec.AdditionalNames, dnsv1alpha1.DNSAdditionalNameRule{Selector: selector, DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, NamePrefix: "application"}) + } + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resolver, policy).WithStatusSubresource(policy).Build() + r := &Reconciler{Client: cl} + seedReferenceState(t, r, &association, map[string]dnsv1alpha1.DNSObjectReference{"context": {Name: resolver.Name, UID: resolver.UID}, "zone": {Name: zone.Name, UID: zone.UID, Generation: zone.Generation}}) + if err := r.reconcileNamingPolicies(ctx, []dnsv1alpha1.DNSNamingPolicy{*policy}, []dnsv1alpha1.DNSZoneAssociation{association}, map[string]*dnsv1alpha1.DNSZone{zone.Name: zone}, now); err != nil { + t.Fatal(err) + } + if err := cl.Get(ctx, client.ObjectKeyFromObject(policy), policy); err != nil { + t.Fatal(err) + } + condition := apimeta.FindStatusCondition(policy.Status.Conditions, "Accepted") + if condition == nil || (condition.Status == metav1.ConditionTrue) != tc.valid { + t.Fatalf("accepted=%v want %v", condition, tc.valid) + } + pins, err := r.readReferences(ctx, policy) + if err != nil || pins["zone/"+zone.Name].UID != zone.UID { + t.Fatal("selector validation lost the zone lifetime pin") + } + }) + } +} diff --git a/internal/internaldns/controlplane/naming_test.go b/internal/internaldns/controlplane/naming_test.go index 8439cf8d..89314e1a 100644 --- a/internal/internaldns/controlplane/naming_test.go +++ b/internal/internaldns/controlplane/naming_test.go @@ -12,13 +12,17 @@ import ( func TestResolveNamingReturnsCanonicalThenAuthorizedAliases(t *testing.T) { s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } - managed := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "vpc-a"}, Status: dnsv1alpha1.DNSResolverContextStatus{ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: "canonical", UID: "zone-c"}}, Conditions: []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue}}}} - policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "custom", Namespace: "project"}, Spec: dnsv1alpha1.DNSNamingPolicySpec{Priority: 10}, Status: dnsv1alpha1.DNSNamingPolicyStatus{ResolvedResolverContextRef: dnsv1alpha1.DNSObjectReference{Name: "prod", UID: "vpc-a"}, ResolvedAdditionalNames: []dnsv1alpha1.DNSResolvedAdditionalNameRule{{Selector: nil, DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: "custom", UID: "zone-x"}, NamePrefix: "instances"}}, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}}} - c := fake.NewClientBuilder().WithScheme(s).WithObjects(managed, policy).Build() - names, err := ResolveNaming(context.Background(), c, "project", "project-a", "vpc-a", nil, "Web 01") + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "context-a"}, Status: dnsv1alpha1.DNSResolverContextStatus{ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "canonical"}}, Conditions: []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue}}}} + canonical := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "canonical", Namespace: "project", UID: "zone-c"}} + custom := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "custom", Namespace: "project", UID: "zone-x"}} + policy := &dnsv1alpha1.DNSNamingPolicy{ObjectMeta: metav1.ObjectMeta{Name: "custom", Namespace: "project", UID: "policy-a"}, Spec: dnsv1alpha1.DNSNamingPolicySpec{Priority: 10}, Status: dnsv1alpha1.DNSNamingPolicyStatus{ResolvedResolverContextRef: dnsv1alpha1.DNSNameReference{Name: resolver.Name}, ResolvedAdditionalNames: []dnsv1alpha1.DNSResolvedAdditionalNameRule{{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "custom"}, NamePrefix: "instances"}}, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}}} + c := fake.NewClientBuilder().WithScheme(s).WithObjects(resolver, canonical, custom, policy).Build() + r := &Reconciler{Client: c, Options: Options{ProjectUID: "project-a"}} + seedReferenceState(t, r, policy, map[string]dnsv1alpha1.DNSObjectReference{"context": {Name: resolver.Name, UID: resolver.UID}, "zone/custom": {Name: custom.Name, UID: custom.UID}}) + names, err := ResolveNaming(context.Background(), c, "project", "project-a", resolver.Name, nil, "Web 01") if err != nil { t.Fatal(err) } @@ -40,10 +44,10 @@ func TestResolveNamingRequiresCurrentManagedNamespace(t *testing.T) { c.Status.Conditions[0].ObservedGeneration = c.Generation - 1 }, } { - managed := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "vpc-a", Generation: 2}, Status: dnsv1alpha1.DNSResolverContextStatus{ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: "canonical", UID: "zone-c"}}, Conditions: []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue, ObservedGeneration: 2}}}} + managed := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "vpc-a", Generation: 2}, Status: dnsv1alpha1.DNSResolverContextStatus{ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: "canonical"}}, Conditions: []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue, ObservedGeneration: 2}}}} mutate(managed) c := fake.NewClientBuilder().WithScheme(s).WithObjects(managed).Build() - if names, err := ResolveNaming(context.Background(), c, "project", "project-a", "vpc-a", nil, "web"); err == nil { + if names, err := ResolveNaming(context.Background(), c, "project", "project-a", managed.Name, nil, "web"); err == nil { t.Fatalf("%s: resolved %#v", name, names) } } diff --git a/internal/internaldns/controlplane/publication_reconciler.go b/internal/internaldns/controlplane/publication_reconciler.go index 5fc96c3b..0c28397b 100644 --- a/internal/internaldns/controlplane/publication_reconciler.go +++ b/internal/internaldns/controlplane/publication_reconciler.go @@ -54,6 +54,27 @@ func (r *Reconciler) reconcileProject(ctx context.Context, ns string) (ctrl.Resu if err := r.reconcileNamingPolicies(ctx, policies.Items, assocs.Items, zoneByName, now); err != nil { return ctrl.Result{}, err } + var sources []client.Object + for i := range assocs.Items { + sources = append(sources, &assocs.Items[i]) + } + for i := range policies.Items { + sources = append(sources, &policies.Items[i]) + } + for i := range regs.Items { + registration := ®s.Items[i] + ref := dnsv1alpha1.DNSObjectReference{Name: registration.Spec.DNSZoneRef.Name} + if zone := zoneByName[ref.Name]; zone != nil && zone.DeletionTimestamp.IsZero() && zone.Spec.Visibility == dnsv1alpha1.DNSZoneVisibilityPrivate { + ref.UID, ref.Generation = zone.UID, zone.Generation + } + if _, _, err := r.pinReferences(ctx, registration, map[string]dnsv1alpha1.DNSObjectReference{"zone": ref}); err != nil { + return ctrl.Result{}, err + } + sources = append(sources, registration) + } + if err := r.collectReferenceStates(ctx, sources); err != nil { + return ctrl.Result{}, err + } if err := r.reconcileBindings(ctx, ns, assocs.Items, zoneByName, now); err != nil { return ctrl.Result{}, err } @@ -140,7 +161,7 @@ func (r *Reconciler) aggregatePublicationStatus(ctx context.Context, z *dnsv1alp } for i := range regs { reg := ®s[i] - if !refMatches(reg.Spec.DNSZoneRef, z.Name, z.UID, z.Generation) || registrationFences[reg.UID] != reg.Generation { + if reg.Spec.DNSZoneRef.Name != z.Name || registrationFences[reg.UID] != reg.Generation { continue } regUIDs[reg.UID] = true @@ -210,6 +231,18 @@ func (r *Reconciler) compileZone(ctx context.Context, z *dnsv1alpha1.DNSZone, as return false, time.Time{}, err } input := CompileInput{Zone: z, Associations: assocs, Registrations: regs, Contributions: cs, StaticRecords: records, Previous: previous, Now: now} + input.AssociationReferences, err = r.associationReferences(ctx, assocs) + if err != nil { + return false, time.Time{}, err + } + input.RegistrationZoneReferences = map[types.UID]dnsv1alpha1.DNSObjectReference{} + for i := range regs { + refs, err := r.readReferences(ctx, ®s[i]) + if err != nil { + return false, time.Time{}, err + } + input.RegistrationZoneReferences[regs[i].UID] = refs["zone"] + } tombstone := !z.DeletionTimestamp.IsZero() var payload []byte var ownPlan PublicationPlan @@ -384,7 +417,7 @@ func (r *Reconciler) persistPublication(ctx context.Context, z *dnsv1alpha1.DNSZ for _, target := range servingTargets { apiTargets = append(apiTargets, dnsv1alpha1.DNSPublicationTarget{Region: target.Region, Shard: target.Shard}) } - m := &dnsv1alpha1.DNSPublicationManifest{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.Options.PlatformNamespace, Labels: map[string]string{"internal-dns.miloapis.com/zone-uid": string(z.UID)}}, Spec: dnsv1alpha1.DNSPublicationManifestSpec{ZoneRef: dnsv1alpha1.DNSObjectReference{Name: z.Name, UID: z.UID}, ZoneApex: z.Spec.DomainName, ContextUIDs: contextUIDs, WriterEpoch: epoch, Revision: revision, PreviousRevision: previous, Tombstone: tombstone, Chunks: refs, ContentHash: hash, GeneratedAt: metav1.NewTime(now), ContributionFences: apiFences, RegistrationFences: apiRegistrations, ServingTargets: apiTargets}} + m := &dnsv1alpha1.DNSPublicationManifest{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.Options.PlatformNamespace, Labels: map[string]string{"internal-dns.miloapis.com/zone-uid": string(z.UID), "internal-dns.miloapis.com/project-uid": string(r.Options.ProjectUID)}}, Spec: dnsv1alpha1.DNSPublicationManifestSpec{ZoneRef: dnsv1alpha1.DNSObjectReference{Name: z.Name, UID: z.UID}, ZoneApex: z.Spec.DomainName, ContextUIDs: contextUIDs, WriterEpoch: epoch, Revision: revision, PreviousRevision: previous, Tombstone: tombstone, Chunks: refs, ContentHash: hash, GeneratedAt: metav1.NewTime(now), ContributionFences: apiFences, RegistrationFences: apiRegistrations, ServingTargets: apiTargets}} if err := r.createImmutable(ctx, m); err != nil { return nil, err } @@ -518,7 +551,7 @@ func (r *Reconciler) patchInputStatuses(ctx context.Context, zone *dnsv1alpha1.D relevant := map[types.UID]bool{} for i := range regs { reg := ®s[i] - if !refMatches(reg.Spec.DNSZoneRef, zone.Name, zone.UID, zone.Generation) { + if reg.Spec.DNSZoneRef.Name != zone.Name { continue } relevant[reg.UID] = true diff --git a/internal/internaldns/controlplane/publication_reconciler_test.go b/internal/internaldns/controlplane/publication_reconciler_test.go index 4b9fd7c2..d5d4e2a9 100644 --- a/internal/internaldns/controlplane/publication_reconciler_test.go +++ b/internal/internaldns/controlplane/publication_reconciler_test.go @@ -23,7 +23,7 @@ func TestExpiredOwnershipTakeoverFencesResumedWriter(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "zone", Namespace: "project", UID: "zone-a"}} @@ -73,7 +73,7 @@ func TestQuiescentSurvivorRenewsOwnershipBeyondLeaseWindow(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } network := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "vpc-a"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer-a"}} @@ -84,7 +84,7 @@ func TestQuiescentSurvivorRenewsOwnershipBeyondLeaseWindow(t *testing.T) { assoc := input.Associations[0].DeepCopy() assoc.Namespace = "project" assoc.Name = "zone-prod" - assoc.Spec.ResolverContextRef = dnsv1alpha1.DNSObjectReference{Name: "prod", UID: "vpc-a"} + assoc.Spec.ResolverContextRef = dnsv1alpha1.DNSNameReference{Name: "prod"} assoc.Status = dnsv1alpha1.DNSZoneAssociationStatus{} reg := input.Registrations[0].DeepCopy() reg.Namespace = "project" @@ -148,7 +148,7 @@ func TestOwnershipTakeoverLoadsOldEpochHighWater(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } payload, err := json.Marshal(model.PublicationPlan{ZoneUID: "zone-a", Apex: "corp.internal", ObservationFences: []model.ObservationFence{{ContributionUID: "contribution-a", Sequence: 19, Generation: 19}}, RRSets: []model.RRSet{}}) @@ -205,7 +205,7 @@ func TestOwnershipTakeoverLoadsOldEpochHighWater(t *testing.T) { func TestLoadPreviousFailsWhenActiveChunkIsMissing(t *testing.T) { ctx := context.Background() s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } manifest := &dnsv1alpha1.DNSPublicationManifest{ @@ -226,7 +226,7 @@ func TestLoadPreviousFailsWhenActiveChunkIsMissing(t *testing.T) { func TestLoadPreviousRejectsHashValidMalformedPayload(t *testing.T) { ctx := context.Background() s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } payload := []byte(`{"zoneUID":`) @@ -245,7 +245,7 @@ func TestLoadPreviousRejectsHashValidMalformedPayload(t *testing.T) { func TestEmptyPublicationUsesJSONArraysForRequiredSlices(t *testing.T) { ctx := context.Background() s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } c := fake.NewClientBuilder().WithScheme(s).Build() @@ -271,7 +271,7 @@ func TestReconcilerPersistsFencedMultiRegionOutbox(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC) s := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(s); err != nil { + if err := addReferenceStateToScheme(s); err != nil { t.Fatal(err) } network := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "prod", Namespace: "project", UID: "vpc-a"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "consumer-a"}} @@ -281,7 +281,7 @@ func TestReconcilerPersistsFencedMultiRegionOutbox(t *testing.T) { assoc := fixture(now).Associations[0].DeepCopy() assoc.Namespace = "project" assoc.Name = "zone-prod" - assoc.Spec.ResolverContextRef = dnsv1alpha1.DNSObjectReference{Name: "prod", UID: "vpc-a"} + assoc.Spec.ResolverContextRef = dnsv1alpha1.DNSNameReference{Name: "prod"} assoc.Status = dnsv1alpha1.DNSZoneAssociationStatus{} reg := fixture(now).Registrations[0].DeepCopy() reg.Namespace = "project" @@ -365,7 +365,7 @@ func TestReconcilerPersistsFencedMultiRegionOutbox(t *testing.T) { func TestProjectContinuesAfterZonePublicationFailure(t *testing.T) { ctx := context.Background() scheme := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + if err := addReferenceStateToScheme(scheme); err != nil { t.Fatal(err) } bad := fixture(time.Now()).Zone.DeepCopy() diff --git a/internal/internaldns/controlplane/publication_regression_test.go b/internal/internaldns/controlplane/publication_regression_test.go index ca094fd0..f210cb7b 100644 --- a/internal/internaldns/controlplane/publication_regression_test.go +++ b/internal/internaldns/controlplane/publication_regression_test.go @@ -53,7 +53,7 @@ func TestCommittedUntargetedTombstoneCompletes(t *testing.T) { ctx := context.Background() now := time.Date(2026, 10, 8, 12, 0, 0, 0, time.UTC) scheme := runtime.NewScheme() - if err := dnsv1alpha1.AddToScheme(scheme); err != nil { + if err := addReferenceStateToScheme(scheme); err != nil { t.Fatal(err) } cl := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&dnsv1alpha1.DNSPublicationManifest{}).Build() diff --git a/internal/internaldns/controlplane/reconciler.go b/internal/internaldns/controlplane/reconciler.go index b30e15fc..2d337087 100644 --- a/internal/internaldns/controlplane/reconciler.go +++ b/internal/internaldns/controlplane/reconciler.go @@ -49,7 +49,7 @@ type AddressAllocator interface { func (o *ReconcilerOptions) defaults() { if o.PlatformNamespace == "" { - o.PlatformNamespace = "internal-dns-system" + o.PlatformNamespace = defaultPlatformNamespace } if o.Region == "" { o.Region = "default" @@ -127,7 +127,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { controllerName := "internal-dns-control-plane-" + model.OpaqueToken(string(r.Options.ProjectUID))[:12] b := ctrl.NewControllerManagedBy(mgr).Named(controllerName).For(&dnsv1alpha1.DNSRecordContribution{}, builder.WithPredicates()). Watches(&dnsv1alpha1.DNSZone{}, enqueue).Watches(&dnsv1alpha1.DNSZoneAssociation{}, enqueue). - Watches(&dnsv1alpha1.DNSNamingPolicy{}, enqueue) + Watches(&dnsv1alpha1.DNSRecordSet{}, enqueue).Watches(&dnsv1alpha1.DNSNamingPolicy{}, enqueue) return b.Watches(&dnsv1alpha1.DNSResolverContext{}, enqueue).Watches(&dnsv1alpha1.DNSResolverAccessBinding{}, enqueue).Complete(r) diff --git a/internal/internaldns/controlplane/reference_state.go b/internal/internaldns/controlplane/reference_state.go new file mode 100644 index 00000000..d4358733 --- /dev/null +++ b/internal/internaldns/controlplane/reference_state.go @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "encoding/json" + "fmt" + "reflect" + + dnsv1 "go.miloapis.com/dns-operator/api/v1alpha1" + "go.miloapis.com/dns-operator/internal/internaldns/model" + 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/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const referenceStateLabel = "internal-dns.miloapis.com/reference-state" +const referenceProjectLabel = "internal-dns.miloapis.com/project-uid" +const referenceStateDataKey = "references.json" +const defaultPlatformNamespace = "internal-dns-system" + +type referenceState struct { + Unverified bool `json:"unverified,omitempty"` + SourceNamespace string `json:"sourceNamespace"` + SourceKind string `json:"sourceKind"` + Source dnsv1.DNSObjectReference `json:"source"` + References map[string]dnsv1.DNSObjectReference `json:"references"` +} + +func referenceKind(source client.Object) string { + switch source.(type) { + case *dnsv1.DNSZoneAssociation: + return "DNSZoneAssociation" + case *dnsv1.DNSNamingPolicy: + return "DNSNamingPolicy" + case *dnsv1.DNSRegistration: + return "DNSRegistration" + default: + return "" + } +} + +func (r *Reconciler) referenceStateObject(source client.Object) *corev1.ConfigMap { + namespace := r.Options.PlatformNamespace + if namespace == "" { + namespace = defaultPlatformNamespace + } + identity := fmt.Sprintf("%s/%s/%s/%s/%s", r.Options.ProjectUID, source.GetNamespace(), referenceKind(source), source.GetName(), source.GetUID()) + return &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, Name: "references-" + model.OpaqueToken(identity), + Labels: map[string]string{referenceStateLabel: "true", referenceProjectLabel: string(r.Options.ProjectUID)}, + }} +} + +func (r *Reconciler) readReferences(ctx context.Context, source client.Object) (map[string]dnsv1.DNSObjectReference, error) { + _, state, err := r.loadReferenceState(ctx, source) + return state.References, err +} + +func (r *Reconciler) loadReferenceState(ctx context.Context, source client.Object) (*corev1.ConfigMap, referenceState, error) { + object := r.referenceStateObject(source) + state := referenceState{SourceKind: referenceKind(source), SourceNamespace: source.GetNamespace(), Source: dnsv1.DNSObjectReference{Name: source.GetName(), UID: source.GetUID()}, References: map[string]dnsv1.DNSObjectReference{}} + if err := r.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { + if apierrors.IsNotFound(err) { + state.Unverified = hasResolvedReferenceStatus(source) + } + return object, state, client.IgnoreNotFound(err) + } + if err := json.Unmarshal([]byte(object.Data[referenceStateDataKey]), &state); err != nil { + return object, state, fmt.Errorf("decode DNS reference state: %w", err) + } + if state.SourceKind != referenceKind(source) || state.SourceNamespace != source.GetNamespace() || state.Source.Name != source.GetName() || state.Source.UID != source.GetUID() { + return object, state, fmt.Errorf("DNS reference state source does not match") + } + return object, state, nil +} + +func (r *Reconciler) pinReferences(ctx context.Context, source client.Object, selected map[string]dnsv1.DNSObjectReference) (map[string]dnsv1.DNSObjectReference, bool, error) { + object, state, err := r.loadReferenceState(ctx, source) + if err != nil { + return nil, false, err + } + if state.Unverified { + if object.ResourceVersion == "" { + payload, err := json.Marshal(state) + if err != nil { + return nil, false, err + } + object.Data = map[string]string{referenceStateDataKey: string(payload)} + if err := r.Create(ctx, object); err != nil { + return nil, false, err + } + } + return state.References, false, nil + } + current := true + resolved := map[string]dnsv1.DNSObjectReference{} + for key, ref := range selected { + pin, valid := pinNamedReference(ref.Name, state.References[key], ref) + resolved[key] = pin + current = current && valid + } + if reflect.DeepEqual(state.References, resolved) { + return resolved, current, nil + } + state.References = resolved + payload, err := json.Marshal(state) + if err != nil { + return nil, false, err + } + if object.ResourceVersion == "" { + object.Data = map[string]string{referenceStateDataKey: string(payload)} + err = r.Create(ctx, object) + } else { + base := object.DeepCopy() + object.Data[referenceStateDataKey] = string(payload) + err = r.Patch(ctx, object, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})) + } + return resolved, current, err +} + +func (r *Reconciler) collectReferenceStates(ctx context.Context, sources []client.Object) error { + live := map[string]bool{} + for _, source := range sources { + live[r.referenceStateObject(source).Name] = true + } + var states corev1.ConfigMapList + namespace := r.Options.PlatformNamespace + if namespace == "" { + namespace = defaultPlatformNamespace + } + if err := r.List(ctx, &states, client.InNamespace(namespace), client.MatchingLabels{referenceStateLabel: "true", referenceProjectLabel: string(r.Options.ProjectUID)}); err != nil { + return err + } + for i := range states.Items { + if !live[states.Items[i].Name] { + var state referenceState + if err := json.Unmarshal([]byte(states.Items[i].Data[referenceStateDataKey]), &state); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "skip malformed DNS reference state", "name", states.Items[i].Name) + continue + } + var source client.Object + switch state.SourceKind { + case "DNSZoneAssociation": + source = &dnsv1.DNSZoneAssociation{} + case "DNSNamingPolicy": + source = &dnsv1.DNSNamingPolicy{} + case "DNSRegistration": + source = &dnsv1.DNSRegistration{} + default: + ctrl.LoggerFrom(ctx).Error(fmt.Errorf("unknown source kind %q", state.SourceKind), "skip DNS reference state", "name", states.Items[i].Name) + continue + } + source.SetNamespace(state.SourceNamespace) + source.SetName(state.Source.Name) + source.SetUID(state.Source.UID) + if r.referenceStateObject(source).Name != states.Items[i].Name { + ctrl.LoggerFrom(ctx).Error(fmt.Errorf("source identity mismatch"), "skip DNS reference state", "name", states.Items[i].Name) + continue + } + err := r.Get(ctx, client.ObjectKey{Namespace: state.SourceNamespace, Name: state.Source.Name}, source) + if err != nil && !apierrors.IsNotFound(err) { + return err + } + if err == nil && source.GetUID() == state.Source.UID { + continue + } + if err := r.Delete(ctx, &states.Items[i], client.Preconditions{UID: &states.Items[i].UID}); client.IgnoreNotFound(err) != nil { + return err + } + } + } + return nil +} + +type associationReferences struct { + Zone dnsv1.DNSObjectReference + Context dnsv1.DNSObjectReference +} + +func (r *Reconciler) associationReferences(ctx context.Context, associations []dnsv1.DNSZoneAssociation) (map[types.UID]associationReferences, error) { + result := map[types.UID]associationReferences{} + for i := range associations { + refs, err := r.readReferences(ctx, &associations[i]) + if err != nil { + return nil, err + } + result[associations[i].UID] = associationReferences{Zone: refs["zone"], Context: refs["context"]} + } + return result, nil +} + +func hasResolvedReferenceStatus(source client.Object) bool { + switch object := source.(type) { + case *dnsv1.DNSZoneAssociation: + return object.Status.ResolvedDNSZoneRef.Name != "" || object.Status.ResolvedResolverContextRef.Name != "" || len(object.Status.Conditions) != 0 + case *dnsv1.DNSNamingPolicy: + return object.Status.ResolvedResolverContextRef.Name != "" || len(object.Status.Conditions) != 0 + case *dnsv1.DNSRegistration: + return object.Status.CanonicalFQDN != "" || object.Status.ObservedGeneration != 0 || len(object.Status.Conditions) != 0 + default: + return false + } +} diff --git a/internal/internaldns/controlplane/reference_state_test.go b/internal/internaldns/controlplane/reference_state_test.go new file mode 100644 index 00000000..188a7c00 --- /dev/null +++ b/internal/internaldns/controlplane/reference_state_test.go @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "testing" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/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" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func addReferenceStateToScheme(scheme *runtime.Scheme) error { + if err := corev1.AddToScheme(scheme); err != nil { + return err + } + return dnsv1alpha1.AddToScheme(scheme) +} + +func seedReferenceState(t *testing.T, r *Reconciler, source client.Object, selected map[string]dnsv1alpha1.DNSObjectReference) { + t.Helper() + source = source.DeepCopyObject().(client.Object) + switch object := source.(type) { + case *dnsv1alpha1.DNSZoneAssociation: + object.Status = dnsv1alpha1.DNSZoneAssociationStatus{} + case *dnsv1alpha1.DNSNamingPolicy: + object.Status = dnsv1alpha1.DNSNamingPolicyStatus{} + case *dnsv1alpha1.DNSRegistration: + object.Status = dnsv1alpha1.DNSRegistrationStatus{} + } + if _, valid, err := r.pinReferences(context.Background(), source, selected); err != nil || !valid { + t.Fatalf("reference state: valid=%t err=%v", valid, err) + } +} + +func TestReferenceStateIsPrivateScopedAndCollected(t *testing.T) { + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + ctx := context.Background() + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &Reconciler{Client: cl, Options: Options{ProjectUID: "project-a", PlatformNamespace: "platform"}} + other := &Reconciler{Client: cl, Options: Options{ProjectUID: "project-b", PlatformNamespace: "platform"}} + source := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration-original"}} + selected := map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: "private", UID: "zone-original", Generation: 1}} + if _, valid, err := r.pinReferences(ctx, source, selected); err != nil || !valid { + t.Fatalf("initial reference: valid=%t err=%v", valid, err) + } + if _, valid, err := other.pinReferences(ctx, source, selected); err != nil || !valid { + t.Fatalf("other project reference: valid=%t err=%v", valid, err) + } + selected["zone"] = dnsv1alpha1.DNSObjectReference{Name: "private", UID: "zone-replacement", Generation: 1} + pins, valid, err := r.pinReferences(ctx, source, selected) + if err != nil || valid || pins["zone"].UID != "zone-original" { + t.Fatalf("replacement transferred a resolved lifetime: pins=%v valid=%t err=%v", pins, valid, err) + } + var public corev1.ConfigMapList + if err := cl.List(ctx, &public, client.InNamespace(source.Namespace)); err != nil || len(public.Items) != 0 { + t.Fatalf("reference state appeared in the project: count=%d err=%v", len(public.Items), err) + } + if err := r.collectReferenceStates(ctx, nil); err != nil { + t.Fatal(err) + } + var states corev1.ConfigMapList + if err := cl.List(ctx, &states, client.InNamespace("platform")); err != nil || len(states.Items) != 1 || states.Items[0].Labels[referenceProjectLabel] != "project-b" { + t.Fatalf("cleanup removed another project's state: states=%v err=%v", states.Items, err) + } + if err := cl.Create(ctx, source); err != nil { + t.Fatal(err) + } + if err := other.collectReferenceStates(ctx, nil); err != nil { + t.Fatal(err) + } + if err := cl.List(ctx, &states, client.InNamespace("platform")); err != nil || len(states.Items) != 1 { + t.Fatalf("collector removed a live source absent from the list snapshot: count=%d err=%v", len(states.Items), err) + } + if err := cl.Delete(ctx, source); err != nil { + t.Fatal(err) + } + if err := other.collectReferenceStates(ctx, nil); err != nil { + t.Fatal(err) + } + if err := cl.List(ctx, &states, client.InNamespace("platform")); err != nil || len(states.Items) != 0 { + t.Fatalf("deleted source retained private state: count=%d err=%v", len(states.Items), err) + } +} + +func TestMissingReferenceStateDoesNotReattachObservedResource(t *testing.T) { + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + ctx := context.Background() + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &Reconciler{Client: cl, Options: Options{ProjectUID: "project", PlatformNamespace: "platform"}} + source := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration"}, Status: dnsv1alpha1.DNSRegistrationStatus{CanonicalFQDN: "web.datum.internal."}} + selected := map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: "private", UID: "replacement-zone"}} + for range 2 { + pins, valid, err := r.pinReferences(ctx, source, selected) + if err != nil || valid || pins["zone"].UID != "" { + t.Fatalf("unverified reference reattached: pins=%v valid=%t err=%v", pins, valid, err) + } + source.Status = dnsv1alpha1.DNSRegistrationStatus{} + } + source.UID = "recreated-registration" + if _, valid, err := r.pinReferences(ctx, source, selected); err != nil || !valid { + t.Fatalf("new registration could not establish references: valid=%t err=%v", valid, err) + } +} + +func TestReferenceCollectionSkipsMalformedOrForeignObjects(t *testing.T) { + scheme := runtime.NewScheme() + if err := addReferenceStateToScheme(scheme); err != nil { + t.Fatal(err) + } + ctx := context.Background() + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &Reconciler{Client: cl, Options: Options{ProjectUID: "project", PlatformNamespace: "platform"}} + source := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "project", UID: "registration"}} + seedReferenceState(t, r, source, map[string]dnsv1alpha1.DNSObjectReference{"zone": {Name: "private", UID: "zone"}}) + for name, data := range map[string]string{"broken": "not-json", "foreign": `{"sourceKind":"DNSRegistration","sourceNamespace":"project","source":{"name":"web","uid":"foreign"}}`, "unknown": `{"sourceKind":"Other"}`} { + object := r.referenceStateObject(source) + object.Name = name + object.Data = map[string]string{referenceStateDataKey: data} + if err := cl.Create(ctx, object); err != nil { + t.Fatal(err) + } + } + if err := r.collectReferenceStates(ctx, nil); err != nil { + t.Fatalf("unrelated reference state blocked collection: %v", err) + } + var states corev1.ConfigMapList + if err := cl.List(ctx, &states, client.InNamespace("platform")); err != nil || len(states.Items) != 3 { + t.Fatalf("valid retired state not collected, or foreign state deleted: count=%d err=%v", len(states.Items), err) + } +} diff --git a/internal/internaldns/platform/acks.go b/internal/internaldns/platform/acks.go index d185bab0..99f31d60 100644 --- a/internal/internaldns/platform/acks.go +++ b/internal/internaldns/platform/acks.go @@ -17,6 +17,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -26,6 +27,7 @@ const dnsValueNode = "node" // to its own AckSubject. It also verifies membership, region, resource lifetime, // revision and freshness before changing project-facing publication status. type ProjectScope struct { + ProjectUID string Client client.Client Namespace string RequestTimeout time.Duration @@ -34,7 +36,6 @@ type ProjectScope struct { type AckSink struct { Client client.Client Namespace string - ProjectNamespaces []string Projects []ProjectScope ProjectScopes func() []ProjectScope PublicationMembers []model.Member @@ -46,6 +47,15 @@ type AckSink struct { } func (s *AckSink) Apply(ctx context.Context, subject string, env model.Envelope) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := ctx.Err(); err != nil { + return err + } + return s.apply(ctx, subject, env) + }) +} + +func (s *AckSink) apply(ctx context.Context, subject string, env model.Envelope) error { if env.Kind != model.KindMemberAck || env.Region != s.Region || env.Shard != s.Shard { return fmt.Errorf("ack belongs to another kind, region or shard") } @@ -127,6 +137,9 @@ func (s *AckSink) publication(ctx context.Context, a model.MemberAck, row dnsv1a m.Status.ReplicaAcknowledgements = upsertAck(m.Status.ReplicaAcknowledgements, row) complete := s.publicationVerified(&m, now) condition(&m.Status.Conditions, "Published", complete, "ReplicaVerification", m.Generation, now) + if jsonEqualStatus(base.Status, m.Status) { + return nil + } if err := s.Client.Status().Patch(ctx, &m, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})); err != nil { return err } @@ -173,6 +186,9 @@ func (s *AckSink) serving(ctx context.Context, a model.MemberAck, row dnsv1alpha binding.Status.Phase = dnsValuePending } s.bindingConditions(binding, e, snapshot, now, complete) + if jsonEqualStatus(base.Status, binding.Status) { + continue + } if err := s.Client.Status().Patch(ctx, binding, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})); err != nil { return err } @@ -186,11 +202,18 @@ func (s *AckSink) projectPublication(ctx context.Context, m *dnsv1alpha1.DNSPubl if s.ProjectScopes != nil { projects = append(projects, s.ProjectScopes()...) } - for _, ns := range s.ProjectNamespaces { - projects = append(projects, ProjectScope{Client: s.Client, Namespace: ns}) - } results := make(chan error, len(projects)) + started := 0 + var projectErrors error for _, project := range projects { + if m.Labels["internal-dns.miloapis.com/project-uid"] != "" && project.ProjectUID == "" { + projectErrors = errors.Join(projectErrors, fmt.Errorf("publication projection scope %q requires a project identity", project.Namespace)) + continue + } + if owner := m.Labels["internal-dns.miloapis.com/project-uid"]; owner != "" && project.ProjectUID != owner { + continue + } + started++ go func() { projectCtx := ctx @@ -202,8 +225,7 @@ func (s *AckSink) projectPublication(ctx context.Context, m *dnsv1alpha1.DNSPubl results <- s.projectPublicationForScope(projectCtx, project, m, complete, now) }() } - var projectErrors error - for range projects { + for i := 0; i < started; i++ { projectErrors = errors.Join(projectErrors, <-results) } return projectErrors diff --git a/internal/internaldns/platform/platform_test.go b/internal/internaldns/platform/platform_test.go index 64407f73..119f9fc3 100644 --- a/internal/internaldns/platform/platform_test.go +++ b/internal/internaldns/platform/platform_test.go @@ -52,7 +52,7 @@ func TestProjectPublicationContinuesAfterSourceFailure(t *testing.T) { t.Fatal(err) } zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: "healthy", Namespace: "healthy-project", UID: "zone-healthy", Generation: 4}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: "healthy.internal", Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} - registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "api", Namespace: zone.Namespace, UID: "registration-healthy", Generation: 2}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name, UID: zone.UID}, Name: "api"}} + registration := &dnsv1alpha1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: "api", Namespace: zone.Namespace, UID: "registration-healthy", Generation: 2}, Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Name: "api"}} healthy := fake.NewClientBuilder().WithScheme(scheme).WithObjects(zone, registration).WithStatusSubresource(&dnsv1alpha1.DNSZone{}, &dnsv1alpha1.DNSRegistration{}).Build() blocked := &blockingListClient{Client: healthy, started: make(chan struct{})} sink := AckSink{Client: testClient(t, chunk), Namespace: "platform", Projects: []ProjectScope{{Client: blocked, Namespace: "offline-project", RequestTimeout: 200 * time.Millisecond}, {Client: healthy, Namespace: zone.Namespace}}} @@ -555,3 +555,102 @@ func TestServingAvailabilityDoesNotRequireEveryReplica(t *testing.T) { t.Fatal("stale observations claimed a serving outage") } } + +func TestPublicationProjectionOnlyReadsOwningProject(t *testing.T) { + cl := testClient(t) + unrelated := &blockingListClient{Client: cl, started: make(chan struct{})} + manifest := &dnsv1alpha1.DNSPublicationManifest{ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"internal-dns.miloapis.com/project-uid": "owner"}, + }} + sink := AckSink{Client: cl, Projects: []ProjectScope{ + {ProjectUID: "unrelated", Client: unrelated, RequestTimeout: 20 * time.Millisecond}, + {ProjectUID: "owner", Client: cl}, + }} + if err := sink.projectPublication(context.Background(), manifest, false, time.Now()); err != nil { + t.Fatal(err) + } + select { + case <-unrelated.started: + t.Fatal("publication contacted an unrelated project") + default: + } +} + +type heldManifestRead struct { + client.Client + once sync.Once + read chan struct{} + release chan struct{} +} + +func (c *heldManifestRead) Get(ctx context.Context, key client.ObjectKey, object client.Object, opts ...client.GetOption) error { + if err := c.Client.Get(ctx, key, object, opts...); err != nil { + return err + } + if _, ok := object.(*dnsv1alpha1.DNSPublicationManifest); ok { + c.once.Do(func() { close(c.read) }) + select { + case <-c.release: + case <-ctx.Done(): + return ctx.Err() + } + } + return nil +} + +func TestConcurrentAckProjectionPreservesNewerObservation(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + now := time.Now().UTC() + manifest := &dnsv1alpha1.DNSPublicationManifest{ObjectMeta: metav1.ObjectMeta{Name: "manifest", Namespace: "platform"}, Spec: dnsv1alpha1.DNSPublicationManifestSpec{ZoneRef: dnsv1alpha1.DNSObjectReference{UID: "zone"}, WriterEpoch: 1, Revision: 2}} + owner := &dnsv1alpha1.DNSPublicationOwnership{ObjectMeta: metav1.ObjectMeta{Name: "owner-" + model.OpaqueToken("zone")[:20], Namespace: "platform"}, Spec: dnsv1alpha1.DNSPublicationOwnershipSpec{ZoneUID: "zone", WriterEpoch: 1, ActiveManifestName: manifest.Name}} + cl := testClient(t, owner, manifest) + held := &heldManifestRead{Client: cl, read: make(chan struct{}), release: make(chan struct{})} + sink := AckSink{Client: cl, Namespace: "platform", Region: "east", Shard: "shared", Members: []model.Member{{MemberID: "auth", ReplicaID: "replica", Role: "regional"}}, Now: func() time.Time { return now }} + olderSink := sink + olderSink.Client = held + ack := model.MemberAck{MemberID: "auth", ReplicaID: "replica", ResourceUID: "zone", Kind: model.KindPublicationManifest, Epoch: 1, Revision: 2, Phase: model.AckVerified, ObservedAt: now.Add(-time.Second), ValidUntil: now.Add(time.Minute)} + older, err := model.NewEnvelope(model.KindMemberAck, "older", "east", "shared", "zone", 1, 2, now, ack) + if err != nil { + t.Fatal(err) + } + ack.Phase = model.AckRejected + ack.ObservedAt = now + newer, err := model.NewEnvelope(model.KindMemberAck, "newer", "east", "shared", "zone", 1, 2, now, ack) + if err != nil { + t.Fatal(err) + } + subject := model.AckSubject("east", "shared", "auth") + done := make(chan error, 1) + go func() { done <- olderSink.Apply(ctx, subject, older) }() + select { + case <-held.read: + case <-ctx.Done(): + t.Fatal("older acknowledgement did not reach manifest") + } + if err := sink.Apply(ctx, subject, newer); err != nil { + close(held.release) + t.Fatal(err) + } + close(held.release) + if err := <-done; err != nil { + t.Fatalf("racing older projection did not retry the optimistic conflict: %v", err) + } + if err := sink.Apply(ctx, subject, older); err != nil { + t.Fatalf("older acknowledgement redelivery failed: %v", err) + } + if err := cl.Get(ctx, client.ObjectKeyFromObject(manifest), manifest); err != nil { + t.Fatal(err) + } + if len(manifest.Status.ReplicaAcknowledgements) != 1 || manifest.Status.ReplicaAcknowledgements[0].Phase != string(model.AckRejected) || manifest.Status.ReplicaAcknowledgements[0].ObservedUnixNano != now.UnixNano() { + t.Fatalf("late verified acknowledgement overwrote newer rejection: %+v", manifest.Status.ReplicaAcknowledgements) + } +} + +func TestPublicationProjectionReportsMissingProjectIdentity(t *testing.T) { + manifest := &dnsv1alpha1.DNSPublicationManifest{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"internal-dns.miloapis.com/project-uid": "project-owner"}}} + sink := AckSink{Projects: []ProjectScope{{Namespace: "default"}}} + if err := sink.projectPublication(context.Background(), manifest, true, time.Now()); err == nil { + t.Fatal("missing projection identity was silently skipped") + } +} diff --git a/internal/internaldns/runtime/config.go b/internal/internaldns/runtime/config.go index d4ea47b3..e429e592 100644 --- a/internal/internaldns/runtime/config.go +++ b/internal/internaldns/runtime/config.go @@ -62,6 +62,7 @@ type RegionalTransportConfig struct { } type Config struct { + MetricsBindAddress string `json:"metricsBindAddress,omitempty"` Region string `json:"region"` Shard string `json:"shard"` PlatformNamespace string `json:"platformNamespace"` diff --git a/internal/internaldns/runtime/deployment_permissions_test.go b/internal/internaldns/runtime/deployment_permissions_test.go index 1702e8ad..6ec1ceae 100644 --- a/internal/internaldns/runtime/deployment_permissions_test.go +++ b/internal/internaldns/runtime/deployment_permissions_test.go @@ -266,13 +266,13 @@ func verifyProjectReferenceSchemas(t *testing.T, ctx context.Context, cl client. object client.Object valid bool }{ - {"registration-valid", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: ref, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 30}}, true}, - {"registration-unpinned", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 30}}, false}, - {"registration-ttl31", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: ref, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 31}}, false}, - {"association-valid", &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: ref, ResolverContextRef: contextRef}}, true}, - {"association-unpinned", &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name}, ResolverContextRef: contextRef}}, false}, - {"naming-valid", &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: contextRef, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{Selector: nil, DNSZoneRef: ref, NamePrefix: "services"}}}}, true}, - {"naming-unpinned", &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: contextRef, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{Selector: nil, DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: zone.Name}, NamePrefix: "services"}}}}, false}, + {"registration-name-only", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zone.Name}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 30}}, true}, + {"registration-empty-zone", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 30}}, false}, + {"registration-ttl31", &dnsv1alpha1.DNSRegistration{Spec: dnsv1alpha1.DNSRegistrationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: ref.Name}, Name: "api", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 31}}, false}, + {"association-valid", &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: ref.Name}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: contextRef.Name}}}, true}, + {"association-empty-zone", &dnsv1alpha1.DNSZoneAssociation{Spec: dnsv1alpha1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1alpha1.DNSNameReference{}, ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: contextRef.Name}}}, false}, + {"naming-valid", &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: contextRef.Name}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: ref.Name}, NamePrefix: "services"}}}}, true}, + {"naming-empty-zone", &dnsv1alpha1.DNSNamingPolicy{Spec: dnsv1alpha1.DNSNamingPolicySpec{ResolverContextRef: dnsv1alpha1.DNSNameReference{Name: contextRef.Name}, AdditionalNames: []dnsv1alpha1.DNSAdditionalNameRule{{DNSZoneRef: dnsv1alpha1.DNSNameReference{}, NamePrefix: "services"}}}}, false}, } for _, tc := range cases { tc.object.SetName(tc.name) diff --git a/internal/internaldns/runtime/metrics.go b/internal/internaldns/runtime/metrics.go new file mode 100644 index 00000000..d1c6e079 --- /dev/null +++ b/internal/internaldns/runtime/metrics.go @@ -0,0 +1,33 @@ +package runtime + +import ( + "time" + + "github.com/prometheus/client_golang/prometheus" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +var operationResults = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "internal_dns_operations_total", + Help: "Completed internal DNS operations by configured loop and result.", +}, []string{"operation", "result"}) + +var operationLastSuccess = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Name: "internal_dns_operation_last_success_timestamp_seconds", + Help: "Unix timestamp of the latest successful internal DNS operation.", +}, []string{"operation"}) + +func init() { + metrics.Registry.MustRegister(operationResults, operationLastSuccess) +} + +func observeOperation(operation string, err error) { + lastSuccess := operationLastSuccess.WithLabelValues(operation) + result := "success" + if err != nil { + result = "error" + } else { + lastSuccess.Set(float64(time.Now().Unix())) + } + operationResults.WithLabelValues(operation, result).Inc() +} diff --git a/internal/internaldns/runtime/metrics_test.go b/internal/internaldns/runtime/metrics_test.go new file mode 100644 index 00000000..08da5071 --- /dev/null +++ b/internal/internaldns/runtime/metrics_test.go @@ -0,0 +1,54 @@ +package runtime + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus/testutil" +) + +func TestRetryLoopReportsFailureAndRecovery(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + operation := t.Name() + defer operationResults.DeleteLabelValues(operation, "error") + defer operationResults.DeleteLabelValues(operation, "success") + defer operationLastSuccess.DeleteLabelValues(operation) + done := make(chan error, 1) + calls := 0 + go retryLoop(ctx, time.Millisecond, operation, func(context.Context) error { + calls++ + if calls == 1 { + return errors.New("source unavailable") + } + cancel() + return nil + }, done) + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("loop did not stop") + } + if got := testutil.ToFloat64(operationResults.WithLabelValues(operation, "error")); got != 1 { + t.Fatalf("error count = %v", got) + } + if got := testutil.ToFloat64(operationResults.WithLabelValues(operation, "success")); got != 1 { + t.Fatalf("success count = %v", got) + } + if got := testutil.ToFloat64(operationLastSuccess.WithLabelValues(operation)); got == 0 { + t.Fatal("successful recovery did not update freshness") + } +} + +func TestFailedOperationDoesNotRenewSuccess(t *testing.T) { + operation := t.Name() + defer operationResults.DeleteLabelValues(operation, "error") + defer operationLastSuccess.DeleteLabelValues(operation) + operationLastSuccess.WithLabelValues(operation).Set(123) + observeOperation(operation, errors.New("failed")) + if got := testutil.ToFloat64(operationLastSuccess.WithLabelValues(operation)); got != 123 { + t.Fatalf("failure renewed successful operation timestamp: %v", got) + } +} diff --git a/internal/internaldns/runtime/project_discovery.go b/internal/internaldns/runtime/project_discovery.go index 62cb1349..940adb0e 100644 --- a/internal/internaldns/runtime/project_discovery.go +++ b/internal/internaldns/runtime/project_discovery.go @@ -49,7 +49,7 @@ func (d *projectDirectory) scopes() []platform.ProjectScope { sort.Strings(names) for _, name := range names { p := d.projects[name] - scopes = append(scopes, platform.ProjectScope{Client: p.direct, Namespace: p.config.Namespace, RequestTimeout: p.config.KubeAPI.requestTimeout()}) + scopes = append(scopes, platform.ProjectScope{ProjectUID: string(p.config.ProjectUID), Client: p.direct, Namespace: p.config.Namespace, RequestTimeout: p.config.KubeAPI.requestTimeout()}) } return scopes } diff --git a/internal/internaldns/runtime/run.go b/internal/internaldns/runtime/run.go index f9f2be09..4b83d46a 100644 --- a/internal/internaldns/runtime/run.go +++ b/internal/internaldns/runtime/run.go @@ -190,6 +190,13 @@ func runControlPlane(ctx context.Context, cfg Config, kubeconfig string) error { } errCh := make(chan error, len(projects)+len(planners)*3+8) + if cfg.MetricsBindAddress != "" && cfg.MetricsBindAddress != "0" { + server, err := metricsserver.NewServer(metricsserver.Options{BindAddress: cfg.MetricsBindAddress}, platformREST, nil) + if err != nil { + return fmt.Errorf("create metrics server: %w", err) + } + go func() { errCh <- server.Start(ctx) }() + } startProjectSources(ctx, projects) if discovery != nil { go retryLoop(ctx, time.Duration(cfg.ProjectDiscovery.IntervalSeconds)*time.Second, "project discovery", discovery.step, errCh) @@ -316,11 +323,14 @@ func runControlPlaneRegion(ctx context.Context, cfg Config, region string, plann continue } workerCtx, cancel := context.WithCancel(ctx) - localDone := make(chan error, len(consumers)+1) + const ackWorkers = 4 + localDone := make(chan error, len(consumers)*ackWorkers+1) outbox := &platform.Outbox{Client: platformClient, Namespace: cfg.PlatformNamespace, Region: region, Publisher: route.publisher, Bootstrap: route.bootstrap, PendingOnly: true} go retryLoop(workerCtx, 500*time.Millisecond, "outbox "+region, outbox.Step, localDone) for key, consumer := range consumers { - go consumeAcks(workerCtx, consumer, sinks[key], localDone) + for range ackWorkers { + go consumeAcks(workerCtx, consumer, sinks[key], localDone) + } } <-ctx.Done() cancel() @@ -474,7 +484,11 @@ func retryLoop(ctx context.Context, interval time.Duration, name string, step fu ticker := time.NewTicker(interval) defer ticker.Stop() for { - if err := step(ctx); err != nil && !errors.Is(err, context.Canceled) { + err := step(ctx) + if !errors.Is(err, context.Canceled) { + observeOperation(name, err) + } + if err != nil && !errors.Is(err, context.Canceled) { log.Error(err, "operation failed; retrying") } select { @@ -495,16 +509,22 @@ func consumeAcks(ctx context.Context, source *transport.Consumer, sink *platform terminal <- nil return } + observeOperation("ack read "+sink.Region+"/"+sink.Shard, err) log.Error(err, "read acknowledgement failed; retrying") time.Sleep(time.Second) continue } - if err := sink.Apply(ctx, message.Subject(), message.Envelope()); err != nil { + observeOperation("ack read "+sink.Region+"/"+sink.Shard, nil) + err = sink.Apply(ctx, message.Subject(), message.Envelope()) + observeOperation("ack apply "+sink.Region+"/"+sink.Shard, err) + if err != nil { log.Error(err, "apply acknowledgement failed; retrying") _ = message.Retry(time.Second) continue } - if err := message.Ack(ctx); err != nil { + err = message.Ack(ctx) + observeOperation("ack commit "+sink.Region+"/"+sink.Shard, err) + if err != nil { log.Error(err, "acknowledgement commit was ambiguous; awaiting redelivery") } } diff --git a/internal/internaldns/serving/continuity.go b/internal/internaldns/serving/continuity.go index 74e70f90..19acc5eb 100644 --- a/internal/internaldns/serving/continuity.go +++ b/internal/internaldns/serving/continuity.go @@ -106,6 +106,9 @@ func (a *Agent) restoreActive(ctx context.Context) error { if !a.canRetainActive() { return a.failClosed(ctx) } + if err := a.refreshActiveDNSDist(); err != nil { + return err + } files := a.state.Active.Files if !activationPending(a.cfg.StateDir) && installedFilesMatch(files) { if err := a.probeLocalDaemons(ctx); err == nil { @@ -133,3 +136,29 @@ func (a *Agent) restoreActive(ctx context.Context) error { a.needsActivation = false return nil } + +func (a *Agent) refreshActiveDNSDist() error { + role := a.cfg.Render.Role + var routers []DNSDistRenderConfig + if role == "" || role == RenderBoth || role == RenderNode { + routers = append(routers, a.cfg.Render.NodeDNSDist) + } + if role == "" || role == RenderBoth || role == RenderCluster || role == RenderRegionalDNSDist { + routers = append(routers, a.cfg.Render.ClusterDNSDist) + } + for _, router := range routers { + if len(router.Backends) == 0 { + continue + } + retained, ok := a.state.Active.Files[router.Path] + if !ok { + continue + } + current, err := refreshDNSDistConfiguration(router, retained) + if err != nil { + return err + } + a.state.Active.Files[router.Path] = current + } + return nil +} diff --git a/internal/internaldns/serving/dnsdist.go b/internal/internaldns/serving/dnsdist.go index 071ff5f9..e7fc5711 100644 --- a/internal/internaldns/serving/dnsdist.go +++ b/internal/internaldns/serving/dnsdist.go @@ -3,6 +3,7 @@ package serving import ( + "bytes" "crypto/rand" "encoding/base64" "errors" @@ -55,13 +56,18 @@ func prepareDNSDistRouter(router DNSDistRenderConfig) error { if err := os.MkdirAll(filepath.Dir(router.Path), 0750); err != nil { return err } - if _, err := os.Stat(router.Path); err == nil { - return nil + startup := renderDNSDist(router, router.Backends, nil, nil, false, nil, nil) + retained, err := os.ReadFile(router.Path) + if err == nil { + startup, err = refreshDNSDistConfiguration(router, retained) + if err != nil { + return err + } } else if !errors.Is(err, os.ErrNotExist) { return err } temporary := router.Path + ".bootstrap" - if err := writeDurable(temporary, renderDNSDist(router, router.Backends, nil, nil, false, nil, nil), 0644); err != nil { + if err := writeDurable(temporary, startup, 0644); err != nil { return err } if err := os.Rename(temporary, router.Path); err != nil { @@ -73,6 +79,18 @@ func prepareDNSDistRouter(router DNSDistRenderConfig) error { return nil } +// Refresh immutable process settings while retaining accepted tenant rules. +func refreshDNSDistConfiguration(router DNSDistRenderConfig, retained []byte) ([]byte, error) { + boundary := []byte(dnsdistRulesBoundary) + _, rules, ok := bytes.Cut(retained, boundary) + if !ok || !bytes.HasSuffix(rules, []byte("setRules(rules)\nend\n")) { + return nil, errors.New("retained dnsdist configuration has no complete rules section") + } + startup := renderDNSDist(router, router.Backends, nil, nil, false, nil, nil) + header, _, _ := bytes.Cut(startup, boundary) + return append(append(header, boundary...), rules...), nil +} + func prepareDNSDistKey(path string) error { if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil { return err diff --git a/internal/internaldns/serving/dnsdist_test.go b/internal/internaldns/serving/dnsdist_test.go index 485311ba..01d41ad9 100644 --- a/internal/internaldns/serving/dnsdist_test.go +++ b/internal/internaldns/serving/dnsdist_test.go @@ -3,6 +3,8 @@ package serving import ( + "bytes" + "context" "encoding/base64" "os" "path/filepath" @@ -65,14 +67,68 @@ func TestDNSDistBootstrapPublishesOnlyCompleteConfiguration(t *testing.T) { if err != nil || !strings.Contains(string(data), "DNSAction.Refused") || !strings.Contains(string(data), "setRules(rules)") { t.Fatal("bootstrap must refuse unknown contexts", err) } - if err := os.WriteFile(router.Path, []byte("accepted snapshot"), 0644); err != nil { + accepted := strings.Replace(string(data), "local bindings = {", "local bindings = {\n [\"fd53::a\"] = {context=\"tenant-a\", ready=true},", 1) + accepted = strings.ReplaceAll(accepted, "tcpOnly=true,checkTCP=true,", "") + if err := os.WriteFile(router.Path, []byte(accepted), 0644); err != nil { t.Fatal(err) } if err := prepareDNSDistRouter(router); err != nil { t.Fatal(err) } retained, err := os.ReadFile(router.Path) - if err != nil || string(retained) != "accepted snapshot" { - t.Fatal("bootstrap replaced accepted routing", err) + if err != nil { + t.Fatal(err) + } + _, acceptedRules, _ := strings.Cut(accepted, dnsdistRulesBoundary) + _, retainedRules, _ := strings.Cut(string(retained), dnsdistRulesBoundary) + if retainedRules != acceptedRules || !strings.Contains(string(retained), "tcpOnly=true,checkTCP=true") { + t.Fatal("startup must refresh transport and preserve accepted tenant rules") + } + if err := os.WriteFile(router.Path, []byte("incomplete configuration"), 0644); err != nil { + t.Fatal(err) + } + if err := prepareDNSDistRouter(router); err == nil { + t.Fatal("incomplete retained rules must block startup") + } +} + +func TestRestartRetainsRoutingAcrossProcessConfigurationRefresh(t *testing.T) { + cfg := regionalTestConfig(t) + cfg.Mode = ModeResolver + cfg.Render.Role = RenderNode + cfg.Render.NodeDNSDist.Path = filepath.Join(t.TempDir(), "node-dnsdist.conf") + _, snapshot := renderFixture(t) + data := renderDNSDist(cfg.Render.NodeDNSDist, cfg.Render.NodeDNSDist.Backends, snapshot.Bindings, map[string]bool{"bind-a": true}, false, nil, nil) + retained := []byte(strings.ReplaceAll(string(data), "tcpOnly=true,checkTCP=true,", "")) + if err := os.WriteFile(cfg.Render.NodeDNSDist.Path, retained, 0644); err != nil { + t.Fatal(err) + } + agent, err := NewAgent(cfg, Dependencies{Runner: &recordingRunner{}}) + if err != nil { + t.Fatal(err) + } + agent.state.Snapshot = &snapshot + agent.state.BindingFences, _ = advanceBindingFences(nil, snapshot) + agent.state.Active = &activeConfiguration{Snapshot: snapshot, Files: map[string][]byte{cfg.Render.NodeDNSDist.Path: retained}} + agent.state.RenderedHash = hashFiles(agent.state.Active.Files) + if err := agent.store.Save(agent.state); err != nil { + t.Fatal(err) + } + if err := prepareDNSDistRouter(cfg.Render.NodeDNSDist); err != nil { + t.Fatal(err) + } + runner := &recordingRunner{} + restarted, err := NewAgent(cfg, Dependencies{Runner: runner}) + if err != nil { + t.Fatal(err) + } + if err := restarted.restoreActive(context.Background()); err != nil { + t.Fatal(err) + } + if len(runner.calls) != 0 { + t.Fatalf("restoration reloaded a running daemon with matching accepted routing: %v", runner.calls) + } + if !installedFilesMatch(restarted.state.Active.Files) || !bytes.Equal(restarted.state.Active.Files[cfg.Render.NodeDNSDist.Path], data) { + t.Fatal("accepted checkpoint and current process configuration differ") } } diff --git a/internal/internaldns/serving/render.go b/internal/internaldns/serving/render.go index 6756d9a2..491e3bc2 100644 --- a/internal/internaldns/serving/render.go +++ b/internal/internaldns/serving/render.go @@ -199,16 +199,20 @@ func collectBackends(bindings []model.Binding, get func(model.Binding) []model.B return out } +const dnsdistRulesBoundary = "local rules = {}\n" + func renderDNSDist(c DNSDistRenderConfig, backends []model.Backend, bindings []model.Binding, active map[string]bool, cluster bool, eligible map[string]map[string]bool, zones map[string]map[string]map[string]bool) []byte { var out bytes.Buffer endpoint := dnsdistConsole(c.Path) fmt.Fprint(&out, "local client = inClientStartup()\nif client or not internalDNSInitialized then\n") fmt.Fprintf(&out, " local key\n if os.getenv(\"INTERNAL_DNS_VALIDATE\") == \"1\" then key = string.rep(\"A\",43)..\"=\" else\n local handle = assert(io.open(%q,\"r\"))\n key = handle:read(\"*l\"); handle:close()\n end\n setKey(key)\n controlSocket(%q)\n setConsoleACL({\"127.0.0.1/32\"})\nend\nif not client then\n", DNSDistKeyPath, endpoint) topology, _ := json.Marshal(struct { - Listen string - ACLs []string - Backends []model.Backend - }{c.ListenAddress, c.ACLs, backends}) + Listen string + ACLs []string + Backends []model.Backend + Pool string + Transport string + }{c.ListenAddress, c.ACLs, backends, c.PoolName, "proxy-v2-tcp"}) fingerprint := model.OpaqueToken(string(topology)) fmt.Fprint(&out, "if not internalDNSInitialized then\n") fmt.Fprintf(&out, "setLocal(%q)\nsetACL({", c.ListenAddress) @@ -223,7 +227,7 @@ func renderDNSDist(c DNSDistRenderConfig, backends []model.Backend, bindings []m fmt.Fprintf(&out, "internalDNSServers[%q] = newServer({address=%q,name=%q,pool=%q,useProxyProtocol=true,tcpOnly=true,checkTCP=true,checkName=\".\",checkType=\"SOA\",mustResolve=false})\n", backend.MemberID, net.JoinHostPort(backend.Address, strconv.Itoa(int(backend.Port))), "b_"+model.OpaqueToken(backend.MemberID), c.PoolName) } fmt.Fprintf(&out, "internalDNSTopology = %q\ninternalDNSInitialized = true\nend\nassert(internalDNSTopology == %q,\"dnsdist topology requires a replacement member\")\n", fingerprint, fingerprint) - fmt.Fprint(&out, "local rules = {}\nlocal function appendRule(rule,action) rules[#rules+1] = newRuleAction(rule,action) end\n") + fmt.Fprint(&out, dnsdistRulesBoundary+"local function appendRule(rule,action) rules[#rules+1] = newRuleAction(rule,action) end\n") if cluster { pools := map[string]map[string]bool{} for _, binding := range bindings { diff --git a/internal/internaldns/transport/jetstream.go b/internal/internaldns/transport/jetstream.go index e62d0f9d..b1dad3eb 100644 --- a/internal/internaldns/transport/jetstream.go +++ b/internal/internaldns/transport/jetstream.go @@ -285,9 +285,20 @@ func (j *JetStream) Consumer(ctx context.Context, c ConsumerConfig) (*Consumer, // Next returns one event. Malformed envelopes are terminated so a poison event // cannot block every later revision on an ordered durable. func (c *Consumer) Next(ctx context.Context) (Message, error) { - m, err := c.c.Next(jetstream.FetchContext(ctx)) - if err != nil { - return nil, err + var m jetstream.Msg + for { + if err := ctx.Err(); err != nil { + return nil, err + } + var err error + m, err = c.c.Next(jetstream.FetchContext(ctx)) + if errors.Is(err, nats.ErrTimeout) { + continue + } + if err != nil { + return nil, err + } + break } var env model.Envelope if err := json.Unmarshal(m.Data(), &env); err != nil { diff --git a/internal/internaldns/transport/jetstream_test.go b/internal/internaldns/transport/jetstream_test.go index aa2e9e42..19632da4 100644 --- a/internal/internaldns/transport/jetstream_test.go +++ b/internal/internaldns/transport/jetstream_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" "go.miloapis.com/dns-operator/internal/internaldns/model" ) @@ -63,6 +64,49 @@ func TestBrokerMessageIDIsNamespacedBySubject(t *testing.T) { } } +type nextResultConsumer struct { + jetstream.Consumer + next func() (jetstream.Msg, error) +} + +func (c nextResultConsumer) Next(...jetstream.FetchOpt) (jetstream.Msg, error) { + return c.next() +} + +func TestConsumerWaitsThroughIdlePulls(t *testing.T) { + for _, finalError := range []error{jetstream.ErrNoHeartbeat, nats.ErrConnectionClosed} { + t.Run(finalError.Error(), func(t *testing.T) { + calls := 0 + c := Consumer{c: nextResultConsumer{next: func() (jetstream.Msg, error) { + calls++ + if calls == 1 { + return nil, nats.ErrTimeout + } + return nil, finalError + }}} + _, err := c.Next(context.Background()) + if !errors.Is(err, finalError) || calls != 2 { + t.Fatalf("got %v after %d pulls; want %v after idle pull", err, calls, finalError) + } + }) + } +} + +func TestConsumerIdlePullHonorsCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + calls := 0 + c := Consumer{c: nextResultConsumer{next: func() (jetstream.Msg, error) { + calls++ + cancel() + return nil, nats.ErrTimeout + }}} + _, err := c.Next(ctx) + if !errors.Is(err, context.Canceled) || calls != 1 { + t.Fatalf("got %v after %d pulls; want cancellation", err, calls) + } +} + type envelopeMessage struct { jetstream.Msg data []byte diff --git a/internal/webhook/internaldns.go b/internal/webhook/internaldns.go index 438b69cc..8ad206ad 100644 --- a/internal/webhook/internaldns.go +++ b/internal/webhook/internaldns.go @@ -41,10 +41,10 @@ func SetupInternalDNSWebhook(mgr mcmanager.Manager) { } // liveProjectClient selects the project control plane named by the request's -// cluster context and reads it without a cache, since the checks compare UIDs -// and generations a cache could serve stale. A request without project context -// reads the local cluster. A named project that cannot be found fails the -// request instead of falling back to another cluster. +// cluster context and reads it without a cache, since the checks compare +// lifetimes, UIDs, and generations a cache could serve stale. A request +// without project context reads the local cluster. A named project that cannot +// be found fails the request instead of falling back to another cluster. func liveProjectClient(mgr mcmanager.Manager) func(context.Context, admission.Request) (client.Client, error) { return func(ctx context.Context, _ admission.Request) (client.Client, error) { clusterName, ok := mccontext.ClusterFrom(ctx) diff --git a/internal/webhook/server_test.go b/internal/webhook/server_test.go index d6e60005..7a7d73e7 100644 --- a/internal/webhook/server_test.go +++ b/internal/webhook/server_test.go @@ -177,11 +177,11 @@ func TestSetupWebhooks_AreServed(t *testing.T) { if err := k8sClient.Create(ctx, privateZone); err != nil { t.Fatalf("creating private DNSZone: %v", err) } - registrationReview := func(zoneUID types.UID, extra map[string]authv1.ExtraValue) admissionv1.AdmissionReview { + registrationReview := func(zoneName string, extra map[string]authv1.ExtraValue) admissionv1.AdmissionReview { registration := &dnsv1alpha1.DNSRegistration{ ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "default"}, Spec: dnsv1alpha1.DNSRegistrationSpec{ - DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: privateZone.Name, UID: zoneUID}, + DNSZoneRef: dnsv1alpha1.DNSNameReference{Name: zoneName}, Name: "web", RecordTypes: []dnsv1alpha1.RRType{dnsv1alpha1.RRTypeA}, TTLSeconds: 30, @@ -207,22 +207,22 @@ func TestSetupWebhooks_AreServed(t *testing.T) { } t.Run("internal DNS path admits a live private zone reference", func(t *testing.T) { - resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview(privateZone.UID, nil)) + resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview(privateZone.Name, nil)) if !resp.Allowed { t.Fatalf("internal DNS webhook refused a live private zone reference: %v", resp.Result) } }) - t.Run("internal DNS path refuses a replaced zone", func(t *testing.T) { - resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview("replaced-zone-uid", nil)) + t.Run("internal DNS path refuses a missing zone", func(t *testing.T) { + resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview("missing-zone", nil)) if resp.Allowed { - t.Fatal("internal DNS webhook admitted a reference to a replaced zone") + t.Fatal("internal DNS webhook admitted a reference to a missing zone") } }) t.Run("internal DNS path fails closed for an unknown project", func(t *testing.T) { extra := map[string]authv1.ExtraValue{ParentTypeExtraKey: {"Project"}, ParentNameExtraKey: {"missing-project"}} - resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview(privateZone.UID, extra)) + resp := sendAdmissionReview(ctx, t, httpClient, endpoint(internalDNSPath), registrationReview(privateZone.Name, extra)) if resp.Allowed { t.Fatal("internal DNS webhook checked an unknown project's write against the local cluster") } diff --git a/test/internaldns/kubernetes/qualify/main.go b/test/internaldns/kubernetes/qualify/main.go index 00ea92f4..50095d2e 100644 --- a/test/internaldns/kubernetes/qualify/main.go +++ b/test/internaldns/kubernetes/qualify/main.go @@ -290,9 +290,9 @@ func (s *suite) run(path string) error { if a.resolverContext.UID == b.resolverContext.UID || a.zone.UID == b.zone.UID || a.resolverContext.Status.ManagedNamespace.DNSZoneRef.Name == b.resolverContext.Status.ManagedNamespace.DNSZoneRef.Name { return errors.New("source projects lost their distinct context, zone, or managed namespace identity") } - for _, p := range s.env.Projects { - if p.resolverContext.Status.ManagedNamespace.Suffix != "datum.internal" { - return errors.New("managed namespace did not use the default domain") + for _, p := range []*project{a, b} { + if p.resolverContext.Status.ManagedNamespace.Suffix != p.resolverContext.Spec.ManagedNamespace.DomainSuffix { + return fmt.Errorf("%s managed suffix does not match its requested domain", p.Name) } } s.pass("independent projects retain distinct context, zone, and managed namespace identities", map[string]string{"a": string(a.resolverContext.UID), "b": string(b.resolverContext.UID)}) @@ -330,7 +330,7 @@ func (s *suite) prepare(p *project) error { } if err := s.wait(p.Name+" managed namespace", 120*time.Second, func() (bool, error) { err := p.product.Get(s.ctx, client.ObjectKeyFromObject(&p.resolverContext), &p.resolverContext) - return condition(p.resolverContext.Status.Conditions, "Ready", p.resolverContext.Generation) && p.resolverContext.Status.ManagedNamespace.DNSZoneRef.UID != "", err + return condition(p.resolverContext.Status.Conditions, "Ready", p.resolverContext.Generation) && p.resolverContext.Status.ManagedNamespace.DNSZoneRef.Name != "", err }); err != nil { return err } @@ -354,7 +354,7 @@ func (s *suite) prepare(p *project) error { } func (s *suite) associate(p *project, zone *dnsv1.DNSZone, name string) error { - association := &dnsv1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: p.Namespace}, Spec: dnsv1.DNSZoneAssociationSpec{DNSZoneRef: objectRef(zone), ResolverContextRef: objectRef(&p.resolverContext)}} + association := &dnsv1.DNSZoneAssociation{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: p.Namespace}, Spec: dnsv1.DNSZoneAssociationSpec{DNSZoneRef: dnsv1.DNSNameReference{Name: zone.Name}, ResolverContextRef: dnsv1.DNSNameReference{Name: p.resolverContext.Name}}} if err := p.customer.Create(s.ctx, association); err != nil { return err } @@ -385,7 +385,7 @@ func recordSets(name, address, ipv6 string) []dnsv1.DNSContributionRecordSet { return sets } -func (s *suite) publish(p *project, zone dnsv1.DNSObjectReference, prefix, name, address, ipv6 string) (*publication, error) { +func (s *suite) publish(p *project, zone dnsv1.DNSNameReference, prefix, name, address, ipv6 string) (*publication, error) { pub := &publication{registration: dnsv1.DNSRegistration{ObjectMeta: metav1.ObjectMeta{Name: prefix + "-registration", Namespace: p.Namespace}, Spec: dnsv1.DNSRegistrationSpec{DNSZoneRef: zone, Name: name, RecordTypes: []dnsv1.RRType{dnsv1.RRTypeA}, TTLSeconds: 2}}} if ipv6 != "" { pub.registration.Spec.RecordTypes = append(pub.registration.Spec.RecordTypes, dnsv1.RRTypeAAAA) diff --git a/test/internaldns/kubernetes/qualify/scenarios.go b/test/internaldns/kubernetes/qualify/scenarios.go index 1ed1d550..cf485cb8 100644 --- a/test/internaldns/kubernetes/qualify/scenarios.go +++ b/test/internaldns/kubernetes/qualify/scenarios.go @@ -40,11 +40,11 @@ func (s *suite) scenarios(a, b *project) error { return err } } - pa, err := s.publish(a, objectRef(&a.zone), "common", "common", "10.42.0.10", "fd42::10") + pa, err := s.publish(a, dnsv1.DNSNameReference{Name: a.zone.Name}, "common", "common", "10.42.0.10", "fd42::10") if err != nil { return err } - pb, err := s.publish(b, objectRef(&b.zone), "common", "common", "10.42.0.20", "fd42::20") + pb, err := s.publish(b, dnsv1.DNSNameReference{Name: b.zone.Name}, "common", "common", "10.42.0.20", "fd42::20") if err != nil { return err } @@ -91,7 +91,7 @@ func (s *suite) scenarios(a, b *project) error { return err } s.pass("an unbound destination cannot resolve either tenant's private records", nil) - if _, err := s.publish(a, objectRef(&a.zone), "only-a", "only-a", "10.42.0.20", ""); err != nil { + if _, err := s.publish(a, dnsv1.DNSNameReference{Name: a.zone.Name}, "only-a", "only-a", "10.42.0.20", ""); err != nil { return err } if err := s.expect(a, "only-a.prod.internal", "A", false, dns.RcodeSuccess, "10.42.0.20"); err != nil { @@ -131,7 +131,7 @@ func (s *suite) scenarios(a, b *project) error { if err := s.associate(a, apps, "apps"); err != nil { return err } - if _, err := s.publish(a, objectRef(apps), "apps", "api", "10.42.0.12", ""); err != nil { + if _, err := s.publish(a, dnsv1.DNSNameReference{Name: apps.Name}, "apps", "api", "10.42.0.12", ""); err != nil { return err } if err := s.expect(a, "api.apps.internal", "A", true, dns.RcodeSuccess, "10.42.0.12"); err != nil { @@ -179,7 +179,7 @@ func (s *suite) scenarios(a, b *project) error { if err := s.observe(b, pb, true); err != nil { return err } - deleted, err := s.publish(a, objectRef(&a.zone), "deleted", "deleted", "10.42.0.14", "") + deleted, err := s.publish(a, dnsv1.DNSNameReference{Name: a.zone.Name}, "deleted", "deleted", "10.42.0.14", "") if err != nil { return err }