From 4bc22ff2b7ed53adbe790f413862e3d82e56f476 Mon Sep 17 00:00:00 2001 From: tcas Date: Fri, 1 Aug 2025 14:28:48 +0200 Subject: [PATCH] Support custom list of services to be added to /etc/hosts in cluster DNS operator - RFE-4145 --- manifests/0000_70_dns-operator_00.crd.yaml | 29 ++++ .../controller_dns_node_resolver_daemonset.go | 32 +++- ...roller_dns_node_resolver_daemonset_test.go | 138 ++++++++++++++++++ .../openshift/api/operator/v1/types_dns.go | 30 ++++ .../0000_70_dns_00_dnses.crd.yaml | 29 ++++ 5 files changed, 252 insertions(+), 6 deletions(-) diff --git a/manifests/0000_70_dns-operator_00.crd.yaml b/manifests/0000_70_dns-operator_00.crd.yaml index 8810d71bb..7ee5979ee 100644 --- a/manifests/0000_70_dns-operator_00.crd.yaml +++ b/manifests/0000_70_dns-operator_00.crd.yaml @@ -177,6 +177,35 @@ 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. + For each service reference, entries will be created using the format "..svc" + and an alias with the CLUSTER_DOMAIN suffix will also be added. + items: + description: DNSNodeService represents a Kubernetes service by name + and namespace for node services. + properties: + name: + description: name is the name of the service. + maxLength: 63 + minLength: 1 + pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + type: string + namespace: + description: namespace is the namespace of the service. + maxLength: 63 + minLength: 1 + pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - name + - namespace + type: object + maxItems: 20 + type: array operatorLogLevel: default: Normal description: 'operatorLogLevel controls the logging level of the DNS diff --git a/pkg/operator/controller/controller_dns_node_resolver_daemonset.go b/pkg/operator/controller/controller_dns_node_resolver_daemonset.go index b8efe9829..c77db95a0 100644 --- a/pkg/operator/controller/controller_dns_node_resolver_daemonset.go +++ b/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,27 @@ 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 { + // Build service name in format: name.namespace.svc + if service.Name != "" && service.Namespace != "" { + serviceName := service.Name + "." + service.Namespace + ".svc" + services = append(services, serviceName) + } + } + } + + // 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 +102,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{ diff --git a/pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go b/pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go index 5926cc937..c53a10ee4 100644 --- a/pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go +++ b/pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go @@ -12,6 +12,71 @@ 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 []operatorv1.DNSNodeService + expected string + }{ + { + name: "no additional services", + nodeServices: nil, + expected: "image-registry.openshift-image-registry.svc", + }, + { + name: "empty additional services slice", + nodeServices: []operatorv1.DNSNodeService{}, + expected: "image-registry.openshift-image-registry.svc", + }, + { + name: "single additional service", + nodeServices: []operatorv1.DNSNodeService{{Name: "my-service", Namespace: "my-namespace"}}, + expected: "image-registry.openshift-image-registry.svc,my-service.my-namespace.svc", + }, + { + name: "multiple additional services", + nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "namespace1"}, {Name: "service2", Namespace: "namespace2"}, {Name: "service3", Namespace: "namespace3"}}, + expected: "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc,service3.namespace3.svc", + }, + { + name: "service with empty name is filtered out", + nodeServices: []operatorv1.DNSNodeService{{Name: "", Namespace: "my-namespace"}}, + expected: "image-registry.openshift-image-registry.svc", + }, + { + name: "service with empty namespace is filtered out", + nodeServices: []operatorv1.DNSNodeService{{Name: "my-service", Namespace: ""}}, + expected: "image-registry.openshift-image-registry.svc", + }, + { + name: "mixed valid and invalid services", + nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "namespace1"}, {Name: "", Namespace: "namespace2"}, {Name: "service3", Namespace: "namespace3"}}, + expected: "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service3.namespace3.svc", + }, + { + name: "all invalid services filtered out", + nodeServices: []operatorv1.DNSNodeService{{Name: "", Namespace: "namespace1"}, {Name: "service2", Namespace: ""}}, + expected: "image-registry.openshift-image-registry.svc", + }, + } + + 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 +118,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 []operatorv1.DNSNodeService + expectedServices string + }{ + { + name: "with single node service", + nodeServices: []operatorv1.DNSNodeService{{Name: "my-api", Namespace: "my-namespace"}}, + expectedServices: "image-registry.openshift-image-registry.svc,my-api.my-namespace.svc", + }, + { + name: "with multiple node services", + nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "ns1"}, {Name: "service2", Namespace: "ns2"}}, + expectedServices: "image-registry.openshift-image-registry.svc,service1.ns1.svc,service2.ns2.svc", + }, + { + name: "with valid services only", + nodeServices: []operatorv1.DNSNodeService{{Name: "clean-service", Namespace: "namespace"}, {Name: "another-service", Namespace: "ns"}}, + 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) + } + } + }) } } diff --git a/vendor/github.com/openshift/api/operator/v1/types_dns.go b/vendor/github.com/openshift/api/operator/v1/types_dns.go index 3d7cbb6c0..e7b81a740 100644 --- a/vendor/github.com/openshift/api/operator/v1/types_dns.go +++ b/vendor/github.com/openshift/api/operator/v1/types_dns.go @@ -95,6 +95,16 @@ type DNSSpec struct { // +kubebuilder:default=Normal OperatorLogLevel DNSLogLevel `json:"operatorLogLevel,omitempty"` + // 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. + // For each service reference, entries will be created using the format "..svc" + // and an alias with the CLUSTER_DOMAIN suffix will also be added. + // + // +optional + // +kubebuilder:validation:MaxItems=20 + NodeServices []DNSNodeService `json:"nodeServices,omitempty"` + // logLevel describes the desired logging verbosity for CoreDNS. // Any one of the following values may be specified: // * Normal logs errors from upstream resolvers. @@ -163,6 +173,26 @@ var ( DNSLogLevelTrace DNSLogLevel = "Trace" ) + +// DNSNodeService represents a Kubernetes service by name and namespace for node services. +type DNSNodeService struct { + // name is the name of the service. + // +required + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=`^[a-z]([-a-z0-9]*[a-z0-9])?$` + Name string `json:"name"` + + // namespace is the namespace of the service. + // +required + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=`^[a-z]([-a-z0-9]*[a-z0-9])?$` + Namespace string `json:"namespace"` +} + // Server defines the schema for a server that runs per instance of CoreDNS. type Server struct { // name is required and specifies a unique name for the server. Name must comply diff --git a/vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses.crd.yaml b/vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses.crd.yaml index 8810d71bb..7ee5979ee 100644 --- a/vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses.crd.yaml +++ b/vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses.crd.yaml @@ -177,6 +177,35 @@ 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. + For each service reference, entries will be created using the format "..svc" + and an alias with the CLUSTER_DOMAIN suffix will also be added. + items: + description: DNSNodeService represents a Kubernetes service by name + and namespace for node services. + properties: + name: + description: name is the name of the service. + maxLength: 63 + minLength: 1 + pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + type: string + namespace: + description: namespace is the namespace of the service. + maxLength: 63 + minLength: 1 + pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - name + - namespace + type: object + maxItems: 20 + type: array operatorLogLevel: default: Normal description: 'operatorLogLevel controls the logging level of the DNS