Support custom list of services to be added to /etc/hosts in cluster DNS operator - RFE-4145

Author: t-cas

manifests/0000_70_dns-operator_00.crd.yaml

@@ -177,6 +177,18 @@ spec:
                       type: object
                     type: array
                 type: object
+              nodeServices:
+                description: |-
+                  nodeServices specifies K8s services for which entries should be added
+                  to /etc/hosts by the node resolver. These services will be added in addition to
+                  the default "image-registry.openshift-image-registry.svc" service.
+                  Each service should be a relative name following the format "<service>.<namespace>.svc";
+                  for each relative name, an alias with the CLUSTER_DOMAIN suffix will also be added.
+                items:
+                  pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.svc)?$
+                  type: string
+                maxItems: 20
+                type: array
               operatorLogLevel:
                 default: Normal
                 description: 'operatorLogLevel controls the logging level of the DNS

pkg/operator/controller/controller_dns_node_resolver_daemonset.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -23,11 +24,9 @@ import (
 )
 
 const (
-	// services is a comma- or space-delimited list of services for which
-	// entries should be added to /etc/hosts.  NOTE: For now, ensure these
-	// are relative names; for each relative name, an alias with the
-	// CLUSTER_DOMAIN suffix will also be added.
-	services = "image-registry.openshift-image-registry.svc"
+	// defaultService is the default service that should always be included
+	// in the services list for the node resolver
+	defaultService = "image-registry.openshift-image-registry.svc"
 
 	// workloadPartitioningManagement contains the management workload annotation
 	workloadPartitioningManagement = "target.workload.openshift.io/management"
@@ -38,6 +37,26 @@ var (
 	nodeResolverScript = manifests.NodeResolverScript()
 )
 
+// buildServicesList combines the default service with additional services from the DNS spec.
+// The default service is always included first, followed by any additional services.
+func buildServicesList(dns *operatorv1.DNS) string {
+	// Start with the default service
+	services := []string{defaultService}
+
+	// Add any additional services from the spec
+	if len(dns.Spec.NodeServices) > 0 {
+		for _, service := range dns.Spec.NodeServices {
+			// Trim whitespace and add non-empty services
+			if trimmed := strings.TrimSpace(service); trimmed != "" {
+				services = append(services, trimmed)
+			}
+		}
+	}
+
+	// Join all services with commas for the environment variable
+	return strings.Join(services, ",")
+}
+
 // ensureNodeResolverDaemonset ensures the node resolver daemonset exists if it
 // should or does not exist if it should not exist.  Returns a Boolean
 // indicating whether the daemonset exists, the daemonset if it does exist, and
@@ -82,7 +101,7 @@ func desiredNodeResolverDaemonSet(dns *operatorv1.DNS, clusterIP, clusterDomain,
 	maxUnavailable := intstr.FromString("33%")
 	envs := []corev1.EnvVar{{
 		Name:  "SERVICES",
-		Value: services,
+		Value: buildServicesList(dns),
 	}}
 	if len(clusterIP) > 0 {
 		envs = append(envs, corev1.EnvVar{

pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go

@@ -12,6 +12,81 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
+// TestBuildServicesList tests the buildServicesList function with various DNS configurations
+func TestBuildServicesList(t *testing.T) {
+	testCases := []struct {
+		name               string
+		nodeServices       []string
+		expected           string
+	}{
+		{
+			name:               "no additional services",
+			nodeServices:       nil,
+			expected:           "image-registry.openshift-image-registry.svc",
+		},
+		{
+			name:               "empty additional services slice",
+			nodeServices:       []string{},
+			expected:           "image-registry.openshift-image-registry.svc",
+		},
+		{
+			name:               "single additional service",
+			nodeServices:       []string{"my-service.my-namespace.svc"},
+			expected:           "image-registry.openshift-image-registry.svc,my-service.my-namespace.svc",
+		},
+		{
+			name:               "multiple additional services",
+			nodeServices:       []string{"service1.namespace1.svc", "service2.namespace2.svc", "service3.namespace3.svc"},
+			expected:           "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc,service3.namespace3.svc",
+		},
+		{
+			name:               "service with whitespace gets trimmed",
+			nodeServices:       []string{"  my-service.my-namespace.svc  "},
+			expected:           "image-registry.openshift-image-registry.svc,my-service.my-namespace.svc",
+		},
+		{
+			name:               "mixed clean and whitespace services",
+			nodeServices:       []string{"service1.namespace1.svc", "  service2.namespace2.svc  ", "service3.namespace3.svc"},
+			expected:           "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc,service3.namespace3.svc",
+		},
+		{
+			name:               "empty string services are filtered out",
+			nodeServices:       []string{"service1.namespace1.svc", "", "service2.namespace2.svc"},
+			expected:           "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc",
+		},
+		{
+			name:               "whitespace-only services are filtered out",
+			nodeServices:       []string{"service1.namespace1.svc", "   ", "service2.namespace2.svc"},
+			expected:           "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc",
+		},
+		{
+			name:               "all whitespace/empty services filtered out",
+			nodeServices:       []string{"", "   ", "\t"},
+			expected:           "image-registry.openshift-image-registry.svc",
+		},
+		{
+			name:               "service without .svc suffix",
+			nodeServices:       []string{"my-service.my-namespace"},
+			expected:           "image-registry.openshift-image-registry.svc,my-service.my-namespace",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			dns := &operatorv1.DNS{
+				Spec: operatorv1.DNSSpec{
+					NodeServices: tc.nodeServices,
+				},
+			}
+
+			result := buildServicesList(dns)
+			if result != tc.expected {
+				t.Errorf("expected %q, got %q", tc.expected, result)
+			}
+		})
+	}
+}
+
 // TestDesiredNodeResolverDaemonset verifies that desiredNodeResolverDaemonSet
 // returns the expected daemonset.
 func TestDesiredNodeResolverDaemonset(t *testing.T) {
@@ -53,6 +128,79 @@ func TestDesiredNodeResolverDaemonset(t *testing.T) {
 		} else if clusterDomain != domain {
 			t.Errorf("expected CLUSTER_DOMAIN env for dns node resolver image %q, got %q", clusterDomain, domain)
 		}
+		services, ok := envs["SERVICES"]
+		if !ok {
+			t.Errorf("SERVICES env for dns node resolver image not found")
+		} else if services != "image-registry.openshift-image-registry.svc" {
+			t.Errorf("expected SERVICES env for dns node resolver image %q, got %q", "image-registry.openshift-image-registry.svc", services)
+		}
+	}
+}
+
+// TestDesiredNodeResolverDaemonsetWithNodeServices verifies that desiredNodeResolverDaemonSet
+// correctly handles node services in the SERVICES environment variable.
+func TestDesiredNodeResolverDaemonsetWithNodeServices(t *testing.T) {
+	clusterDomain := "cluster.local"
+	clusterIP := "172.30.77.10"
+	openshiftCLIImage := "openshift/origin-cli:test"
+
+	testCases := []struct {
+		name               string
+		nodeServices       []string
+		expectedServices   string
+	}{
+		{
+			name:               "with single node service",
+			nodeServices:       []string{"my-api.my-namespace.svc"},
+			expectedServices:   "image-registry.openshift-image-registry.svc,my-api.my-namespace.svc",
+		},
+		{
+			name:               "with multiple node services",
+			nodeServices:       []string{"service1.ns1.svc", "service2.ns2.svc"},
+			expectedServices:   "image-registry.openshift-image-registry.svc,service1.ns1.svc,service2.ns2.svc",
+		},
+		{
+			name:               "with services containing whitespace",
+			nodeServices:       []string{"  clean-service.namespace.svc  ", "another-service.ns.svc"},
+			expectedServices:   "image-registry.openshift-image-registry.svc,clean-service.namespace.svc,another-service.ns.svc",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			dns := &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: DefaultDNSController,
+				},
+				Spec: operatorv1.DNSSpec{
+					NodeServices: tc.nodeServices,
+				},
+			}
+
+			if want, ds, err := desiredNodeResolverDaemonSet(dns, clusterIP, clusterDomain, openshiftCLIImage); err != nil {
+				t.Errorf("invalid node resolver daemonset: %v", err)
+			} else if !want {
+				t.Error("expected the node resolver daemonset desired to be true, got false")
+			} else if len(ds.Spec.Template.Spec.Containers) != 1 {
+				t.Errorf("expected number of daemonset containers 1, got %d", len(ds.Spec.Template.Spec.Containers))
+			} else {
+				c := ds.Spec.Template.Spec.Containers[0]
+
+				// Check environment variables
+				envs := map[string]string{}
+				for _, e := range c.Env {
+					envs[e.Name] = e.Value
+				}
+
+				// Verify SERVICES environment variable contains expected services
+				services, ok := envs["SERVICES"]
+				if !ok {
+					t.Errorf("SERVICES env for dns node resolver image not found")
+				} else if services != tc.expectedServices {
+					t.Errorf("expected SERVICES env for dns node resolver image %q, got %q", tc.expectedServices, services)
+				}
+			}
+		})
 	}
 }