diff --git a/internal/cmd/cell/cell.go b/internal/cmd/cell/cell.go index a9518985..c9c09bce 100644 --- a/internal/cmd/cell/cell.go +++ b/internal/cmd/cell/cell.go @@ -419,6 +419,7 @@ func controllerRegistrations( return false, nil } return true, (&controller.InternalDNSCellReconciler{ + Location: serverConfig.Location, Config: serverConfig.InternalDNS, HubCluster: hubCluster, }).SetupWithManager(mgr) diff --git a/internal/controller/internaldns_cell_controller.go b/internal/controller/internaldns_cell_controller.go index 27d2426d..3c56c65e 100644 --- a/internal/controller/internaldns_cell_controller.go +++ b/internal/controller/internaldns_cell_controller.go @@ -26,6 +26,7 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" "go.datum.net/network-services-operator/internal/dnsaccess" + "go.miloapis.com/locations/pkg/locationidentity" ) const ( @@ -42,9 +43,11 @@ var internalDNSServiceProtocols = []networkv1alpha1.NetworkServiceProtocol{ } type InternalDNSCellReconciler struct { - Config config.InternalDNSConfig - HubCluster cluster.Cluster - Now func() time.Time + Location config.LocationConfig + localReader client.Reader + Config config.InternalDNSConfig + HubCluster cluster.Cluster + Now func() time.Time mgr mcmanager.Manager } @@ -73,6 +76,19 @@ func (r *InternalDNSCellReconciler) reconcile( cl client.Client, networkContext *networkingv1alpha.NetworkContext, ) (ctrl.Result, error) { + // Legacy broad policies can deliver another location's context. Do not + // program it or overwrite the owning cell's readiness with NoAttachments. + identity, err := resolveLocationIdentity(ctx, r.localReader, r.Location) + if err != nil { + var unresolved *locationidentity.LocationUnresolved + if errors.As(err, &unresolved) { + return ctrl.Result{RequeueAfter: internalDNSRetryInterval}, nil + } + return ctrl.Result{}, err + } + if identity.Reference.Name != networkContext.Spec.Location.Name { + return ctrl.Result{RequeueAfter: internalDNSRetryInterval}, nil + } settings, err := internalDNSSettingsFromAnnotations(networkContext.Annotations, r.Config.Nameservers()) serviceDestination := networkContext.Annotations[InternalDNSServiceDestinationAnnotation] region := networkContext.Annotations[InternalDNSRegionAnnotation] @@ -498,6 +514,7 @@ func (r *InternalDNSCellReconciler) SetupWithManager(mgr mcmanager.Manager) erro return errors.New("a federation hub cluster is required") } r.mgr = mgr + r.localReader = mgr.GetLocalManager().GetClient() return mcbuilder.ControllerManagedBy(mgr). For(&networkingv1alpha.NetworkContext{}, mcbuilder.WithEngageWithLocalCluster(false)). Named("internaldns-cell"). diff --git a/internal/controller/internaldns_cell_controller_test.go b/internal/controller/internaldns_cell_controller_test.go index 4b5beb42..c66cf7ce 100644 --- a/internal/controller/internaldns_cell_controller_test.go +++ b/internal/controller/internaldns_cell_controller_test.go @@ -211,6 +211,8 @@ func TestInternalDNSCellProgramsOnePathPerResolver(t *testing.T) { primary := map[string]string{"service.datum.net/internal-dns": "true", "internal-dns.datum.net/tier": "node"} secondary := map[string]string{"service.datum.net/internal-dns": "true", "internal-dns.datum.net/tier": "node-b"} r := &InternalDNSCellReconciler{ + Location: config.LocationConfig{Name: "ord-1"}, + localReader: noServingLocations{cl}, Config: config.InternalDNSConfig{Resolvers: []config.InternalDNSResolver{ {Frontend: "fd53::53", ProducerAttachmentSelector: primary}, {Frontend: "fd53::54", ProducerAttachmentSelector: secondary}, @@ -336,8 +338,10 @@ func TestInternalDNSCellReportsNoAttachments(t *testing.T) { WithStatusSubresource(&networkingv1alpha.NetworkContext{}). WithObjects(networkContext.DeepCopy()).Build() r := &InternalDNSCellReconciler{ - Config: config.InternalDNSConfig{Frontend: "fd53::53"}, - HubCluster: &fakeCluster{cl: hub}, + Location: config.LocationConfig{Name: "ord-1"}, + localReader: noServingLocations{cl}, + Config: config.InternalDNSConfig{Frontend: "fd53::53"}, + HubCluster: &fakeCluster{cl: hub}, } if _, err := r.reconcile(ctx, cl, networkContext); err != nil { t.Fatal(err) diff --git a/internal/controller/internaldns_location_guard_test.go b/internal/controller/internaldns_location_guard_test.go new file mode 100644 index 00000000..c794be98 --- /dev/null +++ b/internal/controller/internaldns_location_guard_test.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "reflect" + "testing" + + networking "go.datum.net/network-services-operator/api/v1alpha" + "go.datum.net/network-services-operator/internal/config" + 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 TestInternalDNSForeignOrUnknownLocationDoesNotPublish(t *testing.T) { + for _, location := range []string{"other-location", ""} { + t.Run(location, func(t *testing.T) { + scheme := runtime.NewScheme() + if err := networking.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + nc := testNetworkContext("context", "context-uid") + nc.Annotations = testDNSIntent(metav1.Now().Time) + nc.Status.Conditions = []metav1.Condition{{Type: networking.NetworkContextDNSReady, Status: metav1.ConditionTrue, Reason: "Ready", LastTransitionTime: metav1.Now()}} + hub := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(nc).WithObjects(nc.DeepCopy()).Build() + before := &networking.NetworkContext{} + if err := hub.Get(context.Background(), client.ObjectKeyFromObject(nc), before); err != nil { + t.Fatal(err) + } + cell := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &InternalDNSCellReconciler{Location: config.LocationConfig{Name: location}, localReader: noServingLocations{cell}, HubCluster: &fakeCluster{cl: hub}} + result, err := r.reconcile(context.Background(), cell, nc) + if err != nil { + t.Fatal(err) + } + if result.RequeueAfter == 0 { + t.Fatal("must retry location resolution") + } + got := &networking.NetworkContext{} + if err := hub.Get(context.Background(), client.ObjectKeyFromObject(nc), got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(before, got) { + t.Fatal("unrelated cell overwrote readiness") + } + }) + } +}