fix: namespace filtering

Author: zachsmith1

internal/provider/provider.go

@@ -6,7 +6,6 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/external-dns/endpoint"
 	"sigs.k8s.io/external-dns/plan"
@@ -49,7 +48,8 @@ func (p *Provider) Start(ctx context.Context) error {
 }
 
 // Records returns the current DNS records from DNSRecordSet resources across
-// all zone sources.
+// all zone sources. Only records in namespaces with discovered zones are
+// returned, ensuring namespace-level filtering is respected.
 func (p *Provider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
 	p.logger.Debug("Fetching DNS records from all zone sources")
 
@@ -60,14 +60,22 @@ func (p *Provider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
 	var allEndpoints []*endpoint.Endpoint
 
 	for _, src := range p.registry.Sources() {
+		zoneNamespaces := src.GetZoneNamespaces()
+
 		recordSets, err := src.RecordSets().List(ctx, p.ownerID)
 		if err != nil {
 			p.logger.WithError(err).Errorf("Failed to list records from source %q", src.Name())
 			continue
 		}
 
 		for _, rs := range recordSets {
-			zone := p.resolveZoneForRecordSet(ctx, rs, src)
+			if !zoneNamespaces[rs.Namespace] {
+				p.logger.Debugf("Skipping record %s/%s: namespace not in discovered zone set",
+					rs.Namespace, rs.Name)
+				continue
+			}
+
+			zone := p.resolveZoneForRecordSet(rs, src)
 			if zone == nil {
 				continue
 			}
@@ -87,25 +95,17 @@ func (p *Provider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
 	return allEndpoints, nil
 }
 
-// resolveZoneForRecordSet attempts to find the zone that owns a record set,
-// first via the registry cache, then via a direct API lookup.
-func (p *Provider) resolveZoneForRecordSet(ctx context.Context, rs *dnsv1alpha1.DNSRecordSet, src *ZoneSource) *dnsv1alpha1.DNSZone {
-	match, err := p.registry.GetZoneForDomain(rs.Spec.DNSZoneRef.Name)
-	if err == nil {
-		return match.Zone
-	}
-
-	var zoneObj dnsv1alpha1.DNSZone
-	if err := src.Client().Get(ctx, client.ObjectKey{
-		Name:      rs.Spec.DNSZoneRef.Name,
-		Namespace: rs.Namespace,
-	}, &zoneObj); err != nil {
-		p.logger.Warnf("Failed to get zone %s for record set %s/%s: %v",
-			rs.Spec.DNSZoneRef.Name, rs.Namespace, rs.Name, err)
+// resolveZoneForRecordSet looks up the zone for a record set from the source's
+// zone cache using the DNSZoneRef object name and namespace. No API fallback is
+// performed — if the zone isn't in the cache, the record is skipped.
+func (p *Provider) resolveZoneForRecordSet(rs *dnsv1alpha1.DNSRecordSet, src *ZoneSource) *dnsv1alpha1.DNSZone {
+	zone := src.GetZoneByRef(rs.Spec.DNSZoneRef.Name, rs.Namespace)
+	if zone == nil {
+		p.logger.Warnf("Zone %s not found in cache for record %s/%s",
+			rs.Spec.DNSZoneRef.Name, rs.Namespace, rs.Name)
 		RecordTranslationError("zone_not_found")
-		return nil
 	}
-	return &zoneObj
+	return zone
 }
 
 // ApplyChanges applies the given changes to DNS records, routing each operation

internal/provider/provider_test.go

@@ -372,6 +372,159 @@ func TestProvider_NewProvider(t *testing.T) {
 	})
 }
 
+func TestProvider_Records_NamespaceLabelSelector_FiltersCorrectly(t *testing.T) {
+	ctx := context.Background()
+
+	objects := []runtime.Object{
+		&corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "managed-ns",
+				Labels: map[string]string{"datum.net/managed-dns": "true"},
+			},
+		},
+		&corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{Name: "unmanaged-ns"},
+		},
+		// Zone in labeled namespace
+		&dnsv1alpha1.DNSZone{
+			ObjectMeta: metav1.ObjectMeta{Name: "managed-com", Namespace: "managed-ns"},
+			Spec:       dnsv1alpha1.DNSZoneSpec{DomainName: "managed.com"},
+		},
+		// Zone in unlabeled namespace — should NOT be discovered
+		&dnsv1alpha1.DNSZone{
+			ObjectMeta: metav1.ObjectMeta{Name: "unmanaged-com", Namespace: "unmanaged-ns"},
+			Spec:       dnsv1alpha1.DNSZoneSpec{DomainName: "unmanaged.com"},
+		},
+		// Record in labeled namespace (owned by us)
+		&dnsv1alpha1.DNSRecordSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "app-a-managed", Namespace: "managed-ns",
+				Labels: map[string]string{LabelOwner: "test-owner", LabelManagedBy: ManagedByValue},
+			},
+			Spec: dnsv1alpha1.DNSRecordSetSpec{
+				DNSZoneRef: corev1.LocalObjectReference{Name: "managed-com"},
+				RecordType: dnsv1alpha1.RRTypeA,
+				Records:    []dnsv1alpha1.RecordEntry{{Name: "app", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.1"}}},
+			},
+		},
+		// Record in unlabeled namespace (owned by us) — should NOT be returned
+		&dnsv1alpha1.DNSRecordSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "app-a-unmanaged", Namespace: "unmanaged-ns",
+				Labels: map[string]string{LabelOwner: "test-owner", LabelManagedBy: ManagedByValue},
+			},
+			Spec: dnsv1alpha1.DNSRecordSetSpec{
+				DNSZoneRef: corev1.LocalObjectReference{Name: "unmanaged-com"},
+				RecordType: dnsv1alpha1.RRTypeA,
+				Records:    []dnsv1alpha1.RecordEntry{{Name: "app", A: &dnsv1alpha1.ARecordSpec{Content: "10.0.0.1"}}},
+			},
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	require.NoError(t, dnsv1alpha1.AddToScheme(scheme))
+	require.NoError(t, corev1.AddToScheme(scheme))
+
+	fakeClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithRuntimeObjects(objects...).
+		Build()
+
+	config := &Config{DryRun: false}
+	logger := logrus.New()
+	logger.SetLevel(logrus.ErrorLevel)
+
+	src := NewZoneSource("test", fakeClient, ZoneSourceConfig{
+		NamespaceLabelSelector: "datum.net/managed-dns=true",
+	}, config, logger)
+
+	p, err := NewProvider(config, "test-owner", []*ZoneSource{src})
+	require.NoError(t, err)
+	require.NoError(t, p.registry.Refresh(ctx))
+
+	// Zone discovery should only find managed.com
+	zones := p.registry.ListZones()
+	assert.Len(t, zones, 1, "only zones from labeled namespaces should be discovered")
+	assert.Equal(t, "managed.com", zones[0].Spec.DomainName)
+
+	// Domain filter should only include managed.com
+	filter := p.GetDomainFilter()
+	assert.True(t, filter.Match("app.managed.com"))
+	assert.False(t, filter.Match("app.unmanaged.com"))
+
+	// Records() must only return records from labeled namespaces
+	endpoints, err := p.Records(ctx)
+	require.NoError(t, err)
+	require.Len(t, endpoints, 1, "Records() should only return records from labeled namespaces")
+	assert.Equal(t, "app.managed.com", endpoints[0].DNSName)
+	assert.Equal(t, endpoint.Targets{"192.0.2.1"}, endpoints[0].Targets)
+}
+
+func TestProvider_Records_NamespaceFlag_FiltersCorrectly(t *testing.T) {
+	ctx := context.Background()
+
+	objects := []runtime.Object{
+		&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "watched-ns"}},
+		&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "other-ns"}},
+		&dnsv1alpha1.DNSZone{
+			ObjectMeta: metav1.ObjectMeta{Name: "watched-com", Namespace: "watched-ns"},
+			Spec:       dnsv1alpha1.DNSZoneSpec{DomainName: "watched.com"},
+		},
+		&dnsv1alpha1.DNSZone{
+			ObjectMeta: metav1.ObjectMeta{Name: "other-com", Namespace: "other-ns"},
+			Spec:       dnsv1alpha1.DNSZoneSpec{DomainName: "other.com"},
+		},
+		&dnsv1alpha1.DNSRecordSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "app-a-watched", Namespace: "watched-ns",
+				Labels: map[string]string{LabelOwner: "test-owner", LabelManagedBy: ManagedByValue},
+			},
+			Spec: dnsv1alpha1.DNSRecordSetSpec{
+				DNSZoneRef: corev1.LocalObjectReference{Name: "watched-com"},
+				RecordType: dnsv1alpha1.RRTypeA,
+				Records:    []dnsv1alpha1.RecordEntry{{Name: "app", A: &dnsv1alpha1.ARecordSpec{Content: "192.0.2.1"}}},
+			},
+		},
+		&dnsv1alpha1.DNSRecordSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "app-a-other", Namespace: "other-ns",
+				Labels: map[string]string{LabelOwner: "test-owner", LabelManagedBy: ManagedByValue},
+			},
+			Spec: dnsv1alpha1.DNSRecordSetSpec{
+				DNSZoneRef: corev1.LocalObjectReference{Name: "other-com"},
+				RecordType: dnsv1alpha1.RRTypeA,
+				Records:    []dnsv1alpha1.RecordEntry{{Name: "app", A: &dnsv1alpha1.ARecordSpec{Content: "10.0.0.1"}}},
+			},
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	require.NoError(t, dnsv1alpha1.AddToScheme(scheme))
+	require.NoError(t, corev1.AddToScheme(scheme))
+
+	fakeClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithRuntimeObjects(objects...).
+		Build()
+
+	config := &Config{DryRun: false}
+	logger := logrus.New()
+	logger.SetLevel(logrus.ErrorLevel)
+
+	src := NewZoneSource("test", fakeClient, ZoneSourceConfig{
+		Namespace: "watched-ns",
+	}, config, logger)
+
+	p, err := NewProvider(config, "test-owner", []*ZoneSource{src})
+	require.NoError(t, err)
+	require.NoError(t, p.registry.Refresh(ctx))
+
+	endpoints, err := p.Records(ctx)
+	require.NoError(t, err)
+	require.Len(t, endpoints, 1, "Records() should only return records from the watched namespace")
+	assert.Equal(t, "app.watched.com", endpoints[0].DNSName)
+}
+
 func TestProvider_OwnershipConflict(t *testing.T) {
 	objects := []runtime.Object{
 		&dnsv1alpha1.DNSZone{

internal/provider/zonesource.go

@@ -182,6 +182,33 @@ func (s *ZoneSource) GetZones() map[string]*dnsv1alpha1.DNSZone {
 	return out
 }
 
+// GetZoneByRef looks up a cached zone by its Kubernetes object name and
+// namespace. Returns nil if no matching zone is found in the cache.
+func (s *ZoneSource) GetZoneByRef(name, namespace string) *dnsv1alpha1.DNSZone {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	for _, zone := range s.cache {
+		if zone.Name == name && zone.Namespace == namespace {
+			return zone
+		}
+	}
+	return nil
+}
+
+// GetZoneNamespaces returns the set of namespaces that contain discovered
+// zones. Only records in these namespaces should be considered managed.
+func (s *ZoneSource) GetZoneNamespaces() map[string]bool {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	ns := make(map[string]bool)
+	for _, zone := range s.cache {
+		ns[zone.Namespace] = true
+	}
+	return ns
+}
+
 // Refresh forces an immediate zone cache refresh.
 func (s *ZoneSource) Refresh(ctx context.Context) error {
 	return s.refresh(ctx)