diff --git a/api/v1alpha1/internaldns_project_types.go b/api/v1alpha1/internaldns_project_types.go index bd49de5b..d815ab56 100644 --- a/api/v1alpha1/internaldns_project_types.go +++ b/api/v1alpha1/internaldns_project_types.go @@ -38,6 +38,7 @@ type DNSZoneAssociationStatus struct { // DNSResolverContext is the DNS-owned serving and namespace boundary created by // a trusted network integration. consumerID is opaque to DNS and immutable. // +kubebuilder:validation:XValidation:rule="oldSelf.consumerID == self.consumerID",message="consumerID is immutable" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.managedNamespace) || !has(oldSelf.managedNamespace.domainSuffix) || (has(self.managedNamespace) && has(self.managedNamespace.domainSuffix) && self.managedNamespace.domainSuffix == oldSelf.managedNamespace.domainSuffix)",message="managed namespace domainSuffix is immutable" type DNSResolverContextSpec struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=512 @@ -45,8 +46,14 @@ type DNSResolverContextSpec struct { ManagedNamespace DNSResolverContextManagedNamespace `json:"managedNamespace,omitempty"` } +// +kubebuilder:validation:XValidation:rule="!has(self.enabled) || !self.enabled || (has(self.domainSuffix) && self.domainSuffix.size() > 0)",message="domainSuffix is required when the managed namespace is enabled" type DNSResolverContextManagedNamespace struct { Enabled bool `json:"enabled,omitempty"` + // DomainSuffix is the zone apex requested by the trusted consumer integration. + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$` + DomainSuffix string `json:"domainSuffix,omitempty"` } type DNSResolverContextManagedNamespaceStatus struct { diff --git a/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml b/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml index 64cc3207..46dee0ed 100644 --- a/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml +++ b/config/crd/bases/dns.networking.miloapis.com_dnsresolvercontexts.yaml @@ -47,15 +47,31 @@ spec: type: string managedNamespace: properties: + domainSuffix: + description: DomainSuffix is the zone apex requested by the trusted + consumer integration. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$ + type: string enabled: type: boolean type: object + x-kubernetes-validations: + - message: domainSuffix is required when the managed namespace is + enabled + rule: '!has(self.enabled) || !self.enabled || (has(self.domainSuffix) + && self.domainSuffix.size() > 0)' required: - consumerID type: object x-kubernetes-validations: - message: consumerID is immutable rule: oldSelf.consumerID == self.consumerID + - message: managed namespace domainSuffix is immutable + rule: '!has(oldSelf.managedNamespace) || !has(oldSelf.managedNamespace.domainSuffix) + || (has(self.managedNamespace) && has(self.managedNamespace.domainSuffix) + && self.managedNamespace.domainSuffix == oldSelf.managedNamespace.domainSuffix)' status: properties: conditions: diff --git a/config/internal-dns/examples/context.yaml b/config/internal-dns/examples/context.yaml index 337d38c2..56364b7d 100644 --- a/config/internal-dns/examples/context.yaml +++ b/config/internal-dns/examples/context.yaml @@ -9,3 +9,5 @@ spec: # Products discover this namespace instead of asking users to select a zone. managedNamespace: enabled: true + # Network services selects the default domain for this consumer. + domainSuffix: datum.internal diff --git a/internal/internaldns/controlplane/access_recovery_envtest_test.go b/internal/internaldns/controlplane/access_recovery_envtest_test.go index ba61169e..f8057d0d 100644 --- a/internal/internaldns/controlplane/access_recovery_envtest_test.go +++ b/internal/internaldns/controlplane/access_recovery_envtest_test.go @@ -13,6 +13,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/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -46,6 +47,30 @@ func TestAccessRecoveryPreservesSourceFencesWithCEL(t *testing.T) { for _, ns := range []string{"project", "platform"} { requireAccessTestSuccess(t, cl.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})) } + t.Run("requested managed domain admission", func(t *testing.T) { + for name, domain := range map[string]string{"missing": "", "invalid": "bad..internal", "wildcard": "*.internal"} { + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "project"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: name, ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespace{Enabled: true, DomainSuffix: domain}}} + if err := cl.Create(ctx, resolver); !apierrors.IsInvalid(err) { + t.Fatalf("invalid managed domain %q was not rejected: %v", domain, err) + } + } + resolver := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "custom-domain", Namespace: "project"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "custom", ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespace{Enabled: true, DomainSuffix: "private.example.net"}}} + requireAccessTestSuccess(t, cl.Create(ctx, resolver)) + changed := resolver.DeepCopy() + changed.Spec.ManagedNamespace.DomainSuffix = "different.internal" + if err := cl.Update(ctx, changed); !apierrors.IsInvalid(err) { + t.Fatalf("domain mutation was not rejected: %v", err) + } + raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resolver) + requireAccessTestSuccess(t, err) + unstructured.RemoveNestedField(raw, "spec", "managedNamespace") + removed := &unstructured.Unstructured{Object: raw} + removed.SetAPIVersion(dnsv1alpha1.GroupVersion.String()) + removed.SetKind("DNSResolverContext") + if err := cl.Update(ctx, removed); !apierrors.IsInvalid(err) { + t.Fatalf("domain removal was not rejected: %v", err) + } + }) now := time.Now().UTC().Truncate(time.Second) resolverContext := &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: "context", Namespace: "project"}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "opaque-consumer"}} readyContext := func(c *dnsv1alpha1.DNSResolverContext) { diff --git a/internal/internaldns/controlplane/context_reconciler.go b/internal/internaldns/controlplane/context_reconciler.go index 826b2319..47bbf8a6 100644 --- a/internal/internaldns/controlplane/context_reconciler.go +++ b/internal/internaldns/controlplane/context_reconciler.go @@ -122,7 +122,7 @@ func (r *Reconciler) reconcileResolverContexts(ctx context.Context, ns string, n consumerOwners[c.Spec.ConsumerID] = c.UID } valid = valid && !duplicate - invalidDomain := c.Spec.ManagedNamespace.Enabled && len(validation.IsDNS1123Subdomain(r.Options.ManagedDomainSuffix)) > 0 + invalidDomain := c.Spec.ManagedNamespace.Enabled && len(validation.IsDNS1123Subdomain(c.Spec.ManagedNamespace.DomainSuffix)) > 0 valid = valid && !invalidDomain c.Status.ServingTargets = nil if valid { @@ -131,6 +131,7 @@ func (r *Reconciler) reconcileResolverContexts(ctx context.Context, ns string, n } } ready := valid && !c.Spec.ManagedNamespace.Enabled + c.Status.ManagedNamespace = dnsv1alpha1.DNSResolverContextManagedNamespaceStatus{} if c.Spec.ManagedNamespace.Enabled && valid { var err error ready, err = r.reconcileManagedZone(ctx, c) @@ -144,6 +145,9 @@ func (r *Reconciler) reconcileResolverContexts(ctx context.Context, ns string, n if duplicate { reason, msg = "DuplicateConsumerID", "another live resolver context owns this consumerID" } + if invalidDomain { + reason, msg = "InvalidManagedDomain", "an enabled managed namespace requires a valid domainSuffix" + } } if valid && !ready { status, reason, msg = metav1.ConditionFalse, "Preparing", "managed namespace is not ready" @@ -603,7 +607,7 @@ func resolverContextReady(contextObject *dnsv1alpha1.DNSResolverContext) bool { } func (r *Reconciler) reconcileManagedZone(ctx context.Context, c *dnsv1alpha1.DNSResolverContext) (bool, error) { - suffix := canonicalName(model.OpaqueToken(string(c.UID)) + "." + r.Options.ManagedDomainSuffix) + suffix := canonicalName(c.Spec.ManagedNamespace.DomainSuffix) zoneName := "managed-" + model.OpaqueToken(string(c.UID))[:12] zone := &dnsv1alpha1.DNSZone{ObjectMeta: metav1.ObjectMeta{Name: zoneName, Namespace: c.Namespace, Labels: map[string]string{"dns.networking.miloapis.com/managed": "true"}}, Spec: dnsv1alpha1.DNSZoneSpec{DomainName: suffix, DNSZoneClassName: r.Options.PrivateZoneClassName, Visibility: dnsv1alpha1.DNSZoneVisibilityPrivate}} if r.Scheme != nil { diff --git a/internal/internaldns/controlplane/managed_domain_test.go b/internal/internaldns/controlplane/managed_domain_test.go new file mode 100644 index 00000000..9e4760b9 --- /dev/null +++ b/internal/internaldns/controlplane/managed_domain_test.go @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controlplane + +import ( + "context" + "testing" + "time" + + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +func TestManagedDomainIsSharedWithSeparateContextZones(t *testing.T) { + s := runtime.NewScheme() + if err := dnsv1alpha1.AddToScheme(s); err != nil { + t.Fatal(err) + } + domains := map[string]string{"vpc-a": "datum.internal", "vpc-b": "datum.internal", "custom": "private.example.net"} + objects := make([]client.Object, 0, len(domains)) + for name, domain := range domains { + objects = append(objects, &dnsv1alpha1.DNSResolverContext{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "project", UID: types.UID(name + "-context"), Generation: 1}, Spec: dnsv1alpha1.DNSResolverContextSpec{ConsumerID: "project-uid/" + name, ManagedNamespace: dnsv1alpha1.DNSResolverContextManagedNamespace{Enabled: true, DomainSuffix: domain}}}) + } + c := fake.NewClientBuilder().WithScheme(s).WithObjects(objects...).WithStatusSubresource(&dnsv1alpha1.DNSResolverContext{}, &dnsv1alpha1.DNSZoneAssociation{}).WithInterceptorFuncs(interceptor.Funcs{Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + obj.SetUID(types.UID(obj.GetName() + "-uid")) + obj.SetGeneration(1) + return cl.Create(ctx, obj, opts...) + }}).Build() + r := &Reconciler{Client: c, Scheme: s, Options: ReconcilerOptions{ProjectUID: "project-uid"}} + r.Options.defaults() + ctx := context.Background() + now := time.Now() + if err := r.reconcileResolverContexts(ctx, "project", now); err != nil { + t.Fatal(err) + } + + var zones dnsv1alpha1.DNSZoneList + if err := c.List(ctx, &zones); err != nil { + t.Fatal(err) + } + if len(zones.Items) != len(domains) || zones.Items[0].UID == zones.Items[1].UID { + t.Fatalf("expected separate zones: %#v", zones.Items) + } + var associations dnsv1alpha1.DNSZoneAssociationList + 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) + } + } + + if err := r.reconcileResolverContexts(ctx, "project", now); err != nil { + t.Fatal(err) + } + var contexts dnsv1alpha1.DNSResolverContextList + if err := c.List(ctx, &contexts); err != nil { + t.Fatal(err) + } + for _, resolver := range contexts.Items { + if resolver.Status.ManagedNamespace.Suffix != domains[resolver.Name] || resolver.Status.ManagedNamespace.DNSZoneRef.UID == "" { + t.Fatalf("unallocated context: %#v", resolver.Status) + } + var zone dnsv1alpha1.DNSZone + if err := c.Get(ctx, client.ObjectKey{Namespace: "project", Name: resolver.Status.ManagedNamespace.DNSZoneRef.Name}, &zone); err != nil { + t.Fatal(err) + } + if zone.Spec.DomainName != domains[resolver.Name] { + t.Fatalf("requested %q, allocated %q", domains[resolver.Name], zone.Spec.DomainName) + } + if !metav1.IsControlledBy(&zone, &resolver) { + t.Fatal("managed zone must share the resolver context lifecycle") + } + var association dnsv1alpha1.DNSZoneAssociation + if err := c.Get(ctx, client.ObjectKey{Namespace: "project", Name: zone.Name + "-context"}, &association); err != nil { + t.Fatal(err) + } + if !metav1.IsControlledBy(&association, &resolver) { + 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 { + t.Fatal("contexts share a zone lifetime") + } + zone := &zones.Items[0] + zone.Spec.DomainName = "different.internal" + if err := c.Update(ctx, zone); err != nil { + t.Fatal(err) + } + + if err := r.reconcileResolverContexts(ctx, "project", now); err != nil { + t.Fatal(err) + } + if err := c.List(ctx, &contexts); err != nil { + t.Fatal(err) + } + for _, resolver := range contexts.Items { + if resolver.Status.ManagedNamespace.DNSZoneRef.UID == zone.UID { + t.Fatal("a mismatched zone domain remained allocated") + } + } +} + +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 { + 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}}} + c := fake.NewClientBuilder().WithScheme(s).WithObjects(resolver).WithStatusSubresource(resolver).Build() + r := &Reconciler{Client: c, Scheme: s} + if err := r.reconcileResolverContexts(context.Background(), "project", time.Now()); err != nil { + t.Fatal(err) + } + if err := c.Get(context.Background(), client.ObjectKeyFromObject(resolver), resolver); err != nil { + t.Fatal(err) + } + if len(resolver.Status.Conditions) != 1 || resolver.Status.Conditions[0].Reason != "InvalidManagedDomain" { + t.Fatalf("invalid domain was accepted: %#v", resolver.Status) + } + var namespaces dnsv1alpha1.DNSZoneList + if err := c.List(context.Background(), &namespaces); err != nil || len(namespaces.Items) != 0 { + t.Fatalf("invalid domain allocated a namespace: %#v, %v", namespaces.Items, err) + } + }) + } +} diff --git a/internal/internaldns/controlplane/reconciler.go b/internal/internaldns/controlplane/reconciler.go index 5c5f2e97..b30e15fc 100644 --- a/internal/internaldns/controlplane/reconciler.go +++ b/internal/internaldns/controlplane/reconciler.go @@ -30,7 +30,6 @@ type ReconcilerOptions struct { LeaseDuration time.Duration Now func() time.Time AddressAllocator AddressAllocator - ManagedDomainSuffix string PrivateZoneClassName string ServingRegions []ServingRegion } diff --git a/internal/internaldns/runtime/config.go b/internal/internaldns/runtime/config.go index 7083b548..d4ea47b3 100644 --- a/internal/internaldns/runtime/config.go +++ b/internal/internaldns/runtime/config.go @@ -78,7 +78,6 @@ type Config struct { RegionalTransports []RegionalTransportConfig `json:"regionalTransports,omitempty"` Admission AdmissionConfig `json:"admission,omitempty"` OwnershipLeaseSeconds int64 `json:"ownershipLeaseSeconds,omitempty"` - ManagedDomainSuffix string `json:"managedDomainSuffix,omitempty"` PrivateZoneClassName string `json:"privateZoneClassName,omitempty"` Agent serving.Config `json:"agent,omitempty"` Shards []platform.PlannerConfig `json:"shards,omitempty"` diff --git a/internal/internaldns/runtime/run.go b/internal/internaldns/runtime/run.go index 803c01b2..7b4643c3 100644 --- a/internal/internaldns/runtime/run.go +++ b/internal/internaldns/runtime/run.go @@ -131,8 +131,8 @@ func runControlPlane(ctx context.Context, cfg Config, kubeconfig string) error { }, ProjectNamespace: project.Namespace, PlatformNamespace: cfg.PlatformNamespace, Region: cfg.Region, Shard: cfg.Shard, Identity: identity, - LeaseDuration: cfg.ownershipLease(), - AddressAllocator: allocator, ManagedDomainSuffix: cfg.ManagedDomainSuffix, + LeaseDuration: cfg.ownershipLease(), + AddressAllocator: allocator, PrivateZoneClassName: cfg.PrivateZoneClassName, ServingRegions: regions, }} if err := reconciler.SetupWithManager(mgr); err != nil { diff --git a/test/internaldns/kubernetes/qualify/main.go b/test/internaldns/kubernetes/qualify/main.go index e3e2beff..bfeb22b1 100644 --- a/test/internaldns/kubernetes/qualify/main.go +++ b/test/internaldns/kubernetes/qualify/main.go @@ -287,9 +287,14 @@ func (s *suite) run(path string) error { } } a, b := &s.env.Projects[0], &s.env.Projects[1] - if a.resolverContext.UID == b.resolverContext.UID || a.zone.UID == b.zone.UID || a.resolverContext.Status.ManagedNamespace.Suffix == b.resolverContext.Status.ManagedNamespace.Suffix { + 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") + } + } 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)}) if err := s.scenarios(a, b); err != nil { return err