From 55257eb7d7bf60b83eab3b08a1ec2b48f04aab11 Mon Sep 17 00:00:00 2001 From: Scot Schuchert-Wells Date: Tue, 3 Nov 2026 13:44:32 -0600 Subject: [PATCH] feat(internal-dns): label and describe resolver contexts by their network Backport of #851 to release/v0.34. --- internal/controller/internaldns_controller.go | 80 ++++++++++++++++++- .../controller/internaldns_controller_test.go | 62 ++++++++++++++ 2 files changed, 138 insertions(+), 4 deletions(-) diff --git a/internal/controller/internaldns_controller.go b/internal/controller/internaldns_controller.go index fbd0c28c..72e4dff5 100644 --- a/internal/controller/internaldns_controller.go +++ b/internal/controller/internaldns_controller.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/cluster" @@ -52,6 +53,11 @@ const ( internalDNSOwnerNetworkUIDAnnotation = "networking.datumapis.com/internal-dns-network-uid" internalDNSOwnerProjectUIDAnnotation = "networking.datumapis.com/internal-dns-project-uid" + // internalDNSManagedLabelsAnnotation names the context labels DNS copies to + // the context's managed zone. + internalDNSManagedLabelsAnnotation = "dns.networking.miloapis.com/managed-object-labels" + internalDNSDescriptionAnnotation = "kubernetes.io/description" + internalDNSDisplayNameAnnotation = "kubernetes.io/display-name" internalDNSRegionTopologyKey = "topology.datum.net/region" internalDNSRetryInterval = 10 * time.Second @@ -63,6 +69,8 @@ const ( ) var ( + internalDNSManagedLabels = networkingv1alpha.NetworkLabel + "," + networkingv1alpha.NetworkUIDLabel + internalDNSContextGVK = schema.GroupVersionKind{ Group: "dns.networking.miloapis.com", Version: "v1alpha1", Kind: "DNSResolverContext", } @@ -296,14 +304,20 @@ func (r *InternalDNSProjectReconciler) ensureContext( if domain != internalDNSDefaultDomain || !enabled { return nil, fmt.Errorf("DNSResolverContext %s/%s does not request the network default domain", network.Namespace, name) } + // The labels only tell people which network a zone is for, so a failed + // write never holds up DNS access. + if err := labelInternalDNSContext(ctx, cl, current, network); err != nil { + log.FromContext(ctx).Error(err, "label DNSResolverContext with its network", "name", name) + } return current, nil } desired := newInternalDNSObject(internalDNSContextGVK, network.Namespace, name) - desired.SetAnnotations(map[string]string{ - internalDNSOwnerNetworkUIDAnnotation: string(network.UID), - internalDNSOwnerProjectUIDAnnotation: string(projectUID), - }) + desired.SetLabels(internalDNSNetworkLabels(network)) + annotations := internalDNSContextAnnotations(network) + annotations[internalDNSOwnerNetworkUIDAnnotation] = string(network.UID) + annotations[internalDNSOwnerProjectUIDAnnotation] = string(projectUID) + desired.SetAnnotations(annotations) desired.SetOwnerReferences([]metav1.OwnerReference{{ APIVersion: networkingv1alpha.GroupVersion.String(), Kind: "Network", Name: network.Name, UID: network.UID, Controller: boolPtr(true), BlockOwnerDeletion: boolPtr(true), @@ -318,6 +332,64 @@ func (r *InternalDNSProjectReconciler) ensureContext( return desired, nil } +// internalDNSNetworkLabels names the network a resolver context serves. DNS +// copies a context's labels to its managed zone, so the zone says which +// network it is for. +func internalDNSNetworkLabels(network *networkingv1alpha.Network) map[string]string { + labels := map[string]string{networkingv1alpha.NetworkUIDLabel: string(network.UID)} + if len(validation.IsValidLabelValue(network.Name)) == 0 { + labels[networkingv1alpha.NetworkLabel] = network.Name + } + return labels +} + +// internalDNSContextAnnotations ask DNS to copy the network labels to the +// managed zone, and describe the zone by the network it serves, which DNS also +// copies to the zone. +func internalDNSContextAnnotations(network *networkingv1alpha.Network) map[string]string { + name := network.Name + if display := network.Annotations[internalDNSDisplayNameAnnotation]; display != "" { + name = display + } + return map[string]string{ + internalDNSManagedLabelsAnnotation: internalDNSManagedLabels, + internalDNSDescriptionAnnotation: "Private DNS for the " + name + " VPC", + } +} + +// labelInternalDNSContext adds the network labels and annotations to a context +// created before it carried them, and keeps the description current. +func labelInternalDNSContext(ctx context.Context, cl client.Client, current *unstructured.Unstructured, network *networkingv1alpha.Network) error { + base := current.DeepCopy() + labels := current.GetLabels() + if labels == nil { + labels = map[string]string{} + } + changed := false + for key, value := range internalDNSNetworkLabels(network) { + if labels[key] != value { + labels[key] = value + changed = true + } + } + annotations := current.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + for key, value := range internalDNSContextAnnotations(network) { + if annotations[key] != value { + annotations[key] = value + changed = true + } + } + if !changed { + return nil + } + current.SetLabels(labels) + current.SetAnnotations(annotations) + return cl.Patch(ctx, current, client.MergeFrom(base)) +} + func resolverContextReady(obj *unstructured.Unstructured) bool { if !obj.GetDeletionTimestamp().IsZero() || obj.GetGeneration() < 1 { return false diff --git a/internal/controller/internaldns_controller_test.go b/internal/controller/internaldns_controller_test.go index 47cf1bef..aa29ca8a 100644 --- a/internal/controller/internaldns_controller_test.go +++ b/internal/controller/internaldns_controller_test.go @@ -4,7 +4,9 @@ package controller import ( "context" + "maps" "slices" + "strings" "testing" "time" @@ -810,3 +812,63 @@ func (f *dnsOffFixture) assertDNSOff(t *testing.T, ctx context.Context, key clie t.Errorf("the regional access the context used survived DNS turning off: %v", err) } } + +func TestInternalDNSContextNamesItsNetwork(t *testing.T) { + t.Parallel() + ctx := context.Background() + scheme := runtime.NewScheme() + scheme.AddKnownTypeWithName(internalDNSContextGVK, &unstructured.Unstructured{}) + network := &networkingv1alpha.Network{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "blue", UID: "network-uid"}} + want := map[string]string{networkingv1alpha.NetworkLabel: "blue", networkingv1alpha.NetworkUIDLabel: "network-uid"} + r := &InternalDNSProjectReconciler{} + + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + created, err := r.ensureContext(ctx, cl, network, "project-uid", "project-uid/network-uid") + if err != nil { + t.Fatal(err) + } + if got := created.GetLabels(); !maps.Equal(got, want) { + t.Fatalf("new context labels = %v, want %v", got, want) + } + if got := created.GetAnnotations()[internalDNSManagedLabelsAnnotation]; got != internalDNSManagedLabels { + t.Fatalf("new context managed labels annotation = %q", got) + } + + // A context created before the labels existed gets them on the next pass. + existing := newInternalDNSObject(internalDNSContextGVK, "tenant", internalDNSResourceName("blue", network.UID)) + existing.SetLabels(map[string]string{"other": "kept"}) + existing.SetAnnotations(map[string]string{internalDNSOwnerNetworkUIDAnnotation: "network-uid"}) + existing.Object["spec"] = map[string]any{ + "consumerID": "project-uid/network-uid", + "managedNamespace": map[string]any{"enabled": true, "domainSuffix": internalDNSDefaultDomain}, + } + cl = fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build() + if _, err := r.ensureContext(ctx, cl, network, "project-uid", "project-uid/network-uid"); err != nil { + t.Fatal(err) + } + stored := newInternalDNSObject(internalDNSContextGVK, "tenant", existing.GetName()) + if err := cl.Get(ctx, client.ObjectKeyFromObject(stored), stored); err != nil { + t.Fatal(err) + } + want["other"] = "kept" + if got := stored.GetLabels(); !maps.Equal(got, want) { + t.Fatalf("existing context labels = %v, want %v", got, want) + } + if got := stored.GetAnnotations()[internalDNSManagedLabelsAnnotation]; got != internalDNSManagedLabels { + t.Fatalf("existing context managed labels annotation = %q", got) + } + if got := stored.GetAnnotations()[internalDNSDescriptionAnnotation]; got != "Private DNS for the blue VPC" { + t.Fatalf("existing context description = %q", got) + } + network.Annotations = map[string]string{internalDNSDisplayNameAnnotation: "Blue network"} + if got := internalDNSContextAnnotations(network)[internalDNSDescriptionAnnotation]; got != "Private DNS for the Blue network VPC" { + t.Fatalf("display name description = %q", got) + } + + // A name too long for a label value still gets the UID label. + long := network.DeepCopy() + long.Name = strings.Repeat("a", 64) + if got := internalDNSNetworkLabels(long); got[networkingv1alpha.NetworkLabel] != "" || got[networkingv1alpha.NetworkUIDLabel] != "network-uid" { + t.Fatalf("long name labels = %v", got) + } +}