From 374bb2714a87848eb9346b0bd32e91f65ac00c30 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 27 Apr 2026 15:03:45 +0200 Subject: [PATCH 1/6] USHIFT-6902: Add dns.configFile config field and validation Add a new ConfigFile field to the DNS config struct that allows users to specify a custom CoreDNS Corefile path. Validation ensures mutual exclusivity with dns.hosts, and checks that the file is absolute, exists, is readable, non-empty, and within the 1MiB ConfigMap limit. --- .../config/config-openapi-spec.json | 5 ++ docs/user/howto_config.md | 2 + packaging/microshift/config.yaml | 8 +++ pkg/config/config.go | 3 ++ pkg/config/dns.go | 54 ++++++++++++++++++- 5 files changed, 71 insertions(+), 1 deletion(-) diff --git a/cmd/generate-config/config/config-openapi-spec.json b/cmd/generate-config/config/config-openapi-spec.json index 901548610d..4d5d03a2db 100755 --- a/cmd/generate-config/config/config-openapi-spec.json +++ b/cmd/generate-config/config/config-openapi-spec.json @@ -245,6 +245,11 @@ "default": "example.com", "example": "microshift.example.com" }, + "configFile": { + "description": "configFile is the path to a custom CoreDNS Corefile on the host filesystem.\nWhen set, MicroShift uses this file as the Corefile in the dns-default ConfigMap,\nfully replacing the default template-rendered configuration.\nChanges to this file are detected at runtime and applied without restarting MicroShift.\nMutually exclusive with dns.hosts: setting both causes a startup error.", + "type": "string", + "example": "/etc/microshift/dns/Corefile" + }, "hosts": { "description": "Hosts contains configuration for the hosts file.", "type": "object", diff --git a/docs/user/howto_config.md b/docs/user/howto_config.md index cbed9baa28..72cd84df6e 100644 --- a/docs/user/howto_config.md +++ b/docs/user/howto_config.md @@ -40,6 +40,7 @@ debugging: logLevel: "" dns: baseDomain: "" + configFile: "" hosts: file: "" status: "" @@ -198,6 +199,7 @@ debugging: logLevel: Normal dns: baseDomain: example.com + configFile: "" hosts: file: /etc/hosts status: Disabled diff --git a/packaging/microshift/config.yaml b/packaging/microshift/config.yaml index fd45f1a37a..7d54331024 100644 --- a/packaging/microshift/config.yaml +++ b/packaging/microshift/config.yaml @@ -72,6 +72,14 @@ dns: # example: # microshift.example.com baseDomain: example.com + # configFile is the path to a custom CoreDNS Corefile on the host filesystem. + # When set, MicroShift uses this file as the Corefile in the dns-default ConfigMap, + # fully replacing the default template-rendered configuration. + # Changes to this file are detected at runtime and applied without restarting MicroShift. + # Mutually exclusive with dns.hosts: setting both causes a startup error. + # example: + # /etc/microshift/dns/Corefile + configFile: "" # Hosts contains configuration for the hosts file. hosts: # File is the path to the hosts file to monitor. diff --git a/pkg/config/config.go b/pkg/config/config.go index cffb723f2c..192ee26d73 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -212,6 +212,9 @@ func (c *Config) incorporateUserSettings(u *Config) { if u.DNS.BaseDomain != "" { c.DNS.BaseDomain = u.DNS.BaseDomain } + if u.DNS.ConfigFile != "" { + c.DNS.ConfigFile = u.DNS.ConfigFile + } if u.Network.CNIPlugin != "" { c.Network.CNIPlugin = u.Network.CNIPlugin diff --git a/pkg/config/dns.go b/pkg/config/dns.go index d8948449c9..3c42a10aba 100644 --- a/pkg/config/dns.go +++ b/pkg/config/dns.go @@ -27,6 +27,15 @@ type DNS struct { // +kubebuilder:example=microshift.example.com BaseDomain string `json:"baseDomain"` + // configFile is the path to a custom CoreDNS Corefile on the host filesystem. + // When set, MicroShift uses this file as the Corefile in the dns-default ConfigMap, + // fully replacing the default template-rendered configuration. + // Changes to this file are detected at runtime and applied without restarting MicroShift. + // Mutually exclusive with dns.hosts: setting both causes a startup error. + // +optional + // +kubebuilder:example="/etc/microshift/dns/Corefile" + ConfigFile string `json:"configFile,omitempty"` + // Hosts contains configuration for the hosts file. Hosts HostsConfig `json:"hosts,omitempty"` } @@ -59,6 +68,50 @@ func dnsDefaults() DNS { } func (t *DNS) validate() error { + if t.ConfigFile != "" && t.Hosts.Status == HostsStatusEnabled { + return fmt.Errorf("dns.configFile and dns.hosts are mutually exclusive") + } + + if err := t.validateConfigFile(); err != nil { + return err + } + + return t.validateHosts() +} + +func (t *DNS) validateConfigFile() error { + if t.ConfigFile == "" { + return nil + } + + cleanPath := filepath.Clean(t.ConfigFile) + if !filepath.IsAbs(cleanPath) { + return fmt.Errorf("dns config file path must be absolute: got %s", t.ConfigFile) + } + + fi, err := os.Stat(cleanPath) + if os.IsNotExist(err) { + return fmt.Errorf("dns config file %s does not exist", t.ConfigFile) + } else if err != nil { + return fmt.Errorf("error checking dns config file %s: %v", t.ConfigFile, err) + } + + if fi.Size() == 0 { + return fmt.Errorf("dns config file %s is empty", t.ConfigFile) + } + + if fi.Size() > 1048576 { + return fmt.Errorf("dns config file %s exceeds 1MiB ConfigMap size limit (got %d bytes)", t.ConfigFile, fi.Size()) + } + + file, err := os.Open(cleanPath) + if err != nil { + return fmt.Errorf("dns config file %s is not readable: %v", t.ConfigFile, err) + } + return file.Close() +} + +func (t *DNS) validateHosts() error { switch t.Hosts.Status { case HostsStatusEnabled: if t.Hosts.File == "" { @@ -68,7 +121,6 @@ func (t *DNS) validate() error { cleanPath := filepath.Clean(t.Hosts.File) fi, err := os.Stat(cleanPath) - // Enforce ConfigMap requirement: the file must not exceed 1MiB, as it will be mounted into a ConfigMap. if err == nil && fi.Size() > 1048576 { return fmt.Errorf("hosts file %s exceeds 1MiB ConfigMap (and internal buffer) size limit (got %d bytes)", t.Hosts.File, fi.Size()) } From 77239f6fa82a3564dabad23793a9f713801bd48e Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 27 Apr 2026 15:05:30 +0200 Subject: [PATCH 2/6] USHIFT-6902: Use custom Corefile in DNS controller rendering When dns.configFile is set, read the file and apply its contents directly as the Corefile key in the dns-default ConfigMap, bypassing the default template rendering. When unset, existing behavior is preserved. --- pkg/components/controllers.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/components/controllers.go b/pkg/components/controllers.go index e9bdf7af50..ded367dfca 100644 --- a/pkg/components/controllers.go +++ b/pkg/components/controllers.go @@ -291,10 +291,9 @@ func startDNSController(ctx context.Context, cfg *config.Config, kubeconfigPath "components/openshift-dns/dns/service-account.yaml", "components/openshift-dns/node-resolver/service-account.yaml", } - cm = []string{ - "components/openshift-dns/dns/configmap.yaml", - } - svc = []string{ + cm = "components/openshift-dns/dns/configmap.yaml" + cmList = []string{cm} + svc = []string{ "components/openshift-dns/dns/service.yaml", } ) @@ -303,9 +302,10 @@ func startDNSController(ctx context.Context, cfg *config.Config, kubeconfigPath return err } + hostsEnabled := cfg.DNS.Hosts.Status == config.HostsStatusEnabled extraParams := assets.RenderParams{ "ClusterIP": cfg.Network.DNS, - "HostsEnabled": cfg.DNS.Hosts.Status == config.HostsStatusEnabled, + "HostsEnabled": hostsEnabled, } if err := assets.ApplyServices(ctx, svc, renderTemplate, renderParamsFromConfig(cfg, extraParams), kubeconfigPath); err != nil { @@ -333,10 +333,24 @@ func startDNSController(ctx context.Context, cfg *config.Config, kubeconfigPath klog.Warningf("Failed to apply serviceAccount %v %v", sa, err) return err } - if err := assets.ApplyConfigMaps(ctx, cm, renderTemplate, renderParamsFromConfig(cfg, extraParams), kubeconfigPath); err != nil { - klog.Warningf("Failed to apply configMap %v %v", cm, err) - return err + + if cfg.DNS.ConfigFile != "" { + corefileContent, err := os.ReadFile(cfg.DNS.ConfigFile) + if err != nil { + klog.Warningf("Failed to read custom DNS config file %s: %v", cfg.DNS.ConfigFile, err) + return err + } + if err := assets.ApplyConfigMapWithData(ctx, cm, map[string]string{"Corefile": string(corefileContent)}, kubeconfigPath); err != nil { + klog.Warningf("Failed to apply custom DNS configMap: %v", err) + return err + } + } else { + if err := assets.ApplyConfigMaps(ctx, cmList, renderTemplate, renderParamsFromConfig(cfg, extraParams), kubeconfigPath); err != nil { + klog.Warningf("Failed to apply configMap %v %v", cmList, err) + return err + } } + if err := assets.ApplyDaemonSets(ctx, apps, renderTemplate, renderParamsFromConfig(cfg, extraParams), kubeconfigPath); err != nil { klog.Warningf("Failed to apply apps %v %v", apps, err) return err From 83575142a217ce2cea866d6c23368716c46325a3 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 27 Apr 2026 15:11:17 +0200 Subject: [PATCH 3/6] USHIFT-6902: Add DNS configuration watcher for runtime reload Add DNSConfigurationWatcherManager that watches the custom Corefile specified by dns.configFile for changes at runtime, updating the dns-default ConfigMap when the file is modified. Follows the same fsnotify + SHA256 hashing pattern as the existing hosts watcher. --- pkg/cmd/run.go | 1 + pkg/controllers/dnsconfigurationwatcher.go | 231 +++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 pkg/controllers/dnsconfigurationwatcher.go diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index bd32bb8782..d8dff2a4f4 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -236,6 +236,7 @@ func RunMicroshift(cfg *config.Config) error { util.Must(m.AddService(controllers.NewClusterID(cfg))) util.Must(m.AddService(controllers.NewTelemetryManager(cfg))) util.Must(m.AddService(controllers.NewHostsWatcherManager(cfg))) + util.Must(m.AddService(controllers.NewDNSConfigurationWatcherManager(cfg))) util.Must(m.AddService(gdp.NewGenericDevicePlugin(cfg))) // Storing and clearing the env, so other components don't send the READY=1 until MicroShift is fully ready diff --git a/pkg/controllers/dnsconfigurationwatcher.go b/pkg/controllers/dnsconfigurationwatcher.go new file mode 100644 index 0000000000..85523174a1 --- /dev/null +++ b/pkg/controllers/dnsconfigurationwatcher.go @@ -0,0 +1,231 @@ +package controllers + +import ( + "context" + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/fsnotify/fsnotify" + "github.com/openshift/microshift/pkg/config" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog/v2" +) + +const ( + dnsConfigMapNamespace = "openshift-dns" + dnsConfigMapName = "dns-default" + dnsConfigMapKey = "Corefile" +) + +type DNSConfigurationWatcherManager struct { + file string + kubeconfig string +} + +func NewDNSConfigurationWatcherManager(cfg *config.Config) *DNSConfigurationWatcherManager { + return &DNSConfigurationWatcherManager{ + file: cfg.DNS.ConfigFile, + kubeconfig: cfg.KubeConfigPath(config.KubeAdmin), + } +} + +func (s *DNSConfigurationWatcherManager) Name() string { return "dns-configuration-watcher-manager" } +func (s *DNSConfigurationWatcherManager) Dependencies() []string { return []string{"kube-apiserver"} } + +func (s *DNSConfigurationWatcherManager) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error { + defer close(stopped) + + if s.file == "" { + klog.Infof("%s is disabled (not configured)", s.Name()) + close(ready) + return ctx.Err() + } + + kubeClient, err := s.createKubeClient() + if err != nil { + klog.Errorf("%s failed to create Kubernetes client: %v", s.Name(), err) + return err + } + + if err := s.updateConfigMap(ctx, kubeClient); err != nil { + klog.Errorf("%s failed to create initial ConfigMap: %v", s.Name(), err) + return err + } + + watcher, err := fsnotify.NewWatcher() + if err != nil { + klog.Errorf("%s failed to create file watcher: %v", s.Name(), err) + return err + } + defer func() { + if cerr := watcher.Close(); cerr != nil { + klog.Errorf("%s failed to close file watcher: %v", s.Name(), cerr) + } + }() + + if err := s.setupWatches(watcher); err != nil { + return err + } + close(ready) + + lastHash, err := s.getFileHash() + if err != nil { + klog.Warningf("%s failed to get initial file hash: %v", s.Name(), err) + } + + klog.Infof("%s is ready and watching %s", s.Name(), s.file) + + return s.eventLoop(ctx, watcher, kubeClient, lastHash) +} + +func (s *DNSConfigurationWatcherManager) setupWatches(watcher *fsnotify.Watcher) error { + filesToWatch := []string{ + s.file, + filepath.Dir(s.file), + } + for i, file := range filesToWatch { + if err := watcher.Add(file); err != nil { + if i == 0 { + klog.Errorf("%s failed to watch DNS config file %s: %v", s.Name(), s.file, err) + return err + } + klog.Warningf("%s failed to watch DNS config directory %s: %v", s.Name(), file, err) + } + } + return nil +} + +func (s *DNSConfigurationWatcherManager) eventLoop(ctx context.Context, watcher *fsnotify.Watcher, kubeClient kubernetes.Interface, initHash string) error { + lastHash := initHash + + for { + select { + case <-ctx.Done(): + klog.Infof("%s stopping", s.Name()) + return watcher.Close() + + case event, ok := <-watcher.Events: + if !ok { + return fmt.Errorf("%s watcher channel closed", s.Name()) + } + if s.isRelevantEvent(event) { + updated, newHash, updateErr := s.handleChange(ctx, kubeClient, lastHash) + if updateErr != nil { + klog.Errorf("%s failed to process DNS config file change: %v", s.Name(), updateErr) + continue + } + if updated { + lastHash = newHash + } + } + + case err, ok := <-watcher.Errors: + if !ok { + return fmt.Errorf("%s watcher error channel closed", s.Name()) + } + klog.Errorf("%s watcher error: %v", s.Name(), err) + } + } +} + +func (s *DNSConfigurationWatcherManager) isRelevantEvent(event fsnotify.Event) bool { + if event.Name != s.file { + return false + } + return event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create +} + +func (s *DNSConfigurationWatcherManager) handleChange(ctx context.Context, kubeClient kubernetes.Interface, lastHash string) (bool, string, error) { + klog.Infof("%s detected change in DNS config file: %s", s.Name(), s.file) + currentHash, err := s.getFileHash() + if err != nil { + klog.Warningf("%s failed to get file hash after change: %v", s.Name(), err) + return false, lastHash, err + } + if currentHash == lastHash { + klog.V(2).Infof("%s file hash unchanged, skipping update", s.Name()) + return false, lastHash, nil + } + if err := s.updateConfigMap(ctx, kubeClient); err != nil { + klog.Errorf("%s failed to update ConfigMap: %v", s.Name(), err) + return false, currentHash, err + } + klog.Infof("%s successfully updated ConfigMap %s/%s", s.Name(), dnsConfigMapNamespace, dnsConfigMapName) + return true, currentHash, nil +} + +func (s *DNSConfigurationWatcherManager) createKubeClient() (*kubernetes.Clientset, error) { + cfg, err := clientcmd.BuildConfigFromFlags("", s.kubeconfig) + if err != nil { + return nil, fmt.Errorf("failed to build kubeconfig: %w", err) + } + client, err := kubernetes.NewForConfig(cfg) + if err != nil { + return nil, fmt.Errorf("failed to create Kubernetes client: %w", err) + } + return client, nil +} + +func (s *DNSConfigurationWatcherManager) updateConfigMap(ctx context.Context, client kubernetes.Interface) error { + content, err := os.ReadFile(s.file) + if err != nil { + return fmt.Errorf("failed to read DNS config file %s: %w", s.file, err) + } + + return s.createOrUpdateConfigMap(ctx, client, string(content)) +} + +func (s *DNSConfigurationWatcherManager) createOrUpdateConfigMap(ctx context.Context, client kubernetes.Interface, corefileContent string) error { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: dnsConfigMapName, + Namespace: dnsConfigMapNamespace, + Labels: map[string]string{ + "dns.operator.openshift.io/owning-dns": "default", + }, + Annotations: map[string]string{ + "microshift.io/dns-config-file": s.file, + "microshift.io/last-updated": time.Now().Format(time.RFC3339), + }, + }, + Data: map[string]string{ + dnsConfigMapKey: corefileContent, + }, + } + + configMapsClient := client.CoreV1().ConfigMaps(dnsConfigMapNamespace) + + existing, err := configMapsClient.Get(ctx, dnsConfigMapName, metav1.GetOptions{}) + if err != nil { + _, err = configMapsClient.Create(ctx, configMap, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("failed to create ConfigMap: %w", err) + } + klog.Infof("%s created ConfigMap %s/%s", s.Name(), dnsConfigMapNamespace, dnsConfigMapName) + } else { + existing.Data = configMap.Data + existing.Annotations = configMap.Annotations + _, err = configMapsClient.Update(ctx, existing, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update ConfigMap: %w", err) + } + klog.V(2).Infof("%s updated ConfigMap %s/%s", s.Name(), dnsConfigMapNamespace, dnsConfigMapName) + } + + return nil +} + +func (s *DNSConfigurationWatcherManager) getFileHash() (string, error) { + content, err := os.ReadFile(s.file) + if err != nil { + return "", err + } + hash := sha256.Sum256(content) + return fmt.Sprintf("%x", hash), nil +} From 9e0a8d3e258d9288d4285a7c09441230aed0523e Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 27 Apr 2026 17:40:59 +0200 Subject: [PATCH 4/6] USHIFT-6902: Add unit tests for dns.configFile validation Cover mutual exclusivity with dns.hosts, non-absolute path, missing file, empty file, file exceeding 1MiB limit, valid config, and default behavior when configFile is unset. --- pkg/config/dns_test.go | 128 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 pkg/config/dns_test.go diff --git a/pkg/config/dns_test.go b/pkg/config/dns_test.go new file mode 100644 index 0000000000..ad1766a878 --- /dev/null +++ b/pkg/config/dns_test.go @@ -0,0 +1,128 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDNS_ValidateConfigFile_MutualExclusivity(t *testing.T) { + tmpFile := createTempCorefile(t, ".:5353 { whoami }") + + dns := DNS{ + ConfigFile: tmpFile, + Hosts: HostsConfig{ + Status: HostsStatusEnabled, + File: "/etc/hosts", + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "dns.configFile and dns.hosts are mutually exclusive") +} + +func TestDNS_ValidateConfigFile_WithHostsDisabled(t *testing.T) { + tmpFile := createTempCorefile(t, ".:5353 { whoami }") + + dns := DNS{ + ConfigFile: tmpFile, + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + assert.NoError(t, dns.validate()) +} + +func TestDNS_ValidateConfigFile_EmptyConfigFilePreservesDefault(t *testing.T) { + dns := DNS{ + ConfigFile: "", + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + assert.NoError(t, dns.validate()) +} + +func TestDNS_ValidateConfigFile_NonAbsolutePath(t *testing.T) { + dns := DNS{ + ConfigFile: "relative/path/Corefile", + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "dns config file path must be absolute") +} + +func TestDNS_ValidateConfigFile_NonExistentFile(t *testing.T) { + dns := DNS{ + ConfigFile: "/tmp/nonexistent-corefile-test-12345", + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "does not exist") +} + +func TestDNS_ValidateConfigFile_EmptyFile(t *testing.T) { + tmpFile := createTempCorefile(t, "") + + dns := DNS{ + ConfigFile: tmpFile, + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "is empty") +} + +func TestDNS_ValidateConfigFile_ExceedsSizeLimit(t *testing.T) { + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "Corefile") + // 1 MiB + 1 byte + data := make([]byte, 1048576+1) + require.NoError(t, os.WriteFile(tmpFile, data, 0644)) + + dns := DNS{ + ConfigFile: tmpFile, + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "exceeds 1MiB ConfigMap size limit") +} + +func TestDNS_ValidateConfigFile_ValidFile(t *testing.T) { + tmpFile := createTempCorefile(t, ".:5353 {\n whoami\n reload\n}\n") + + dns := DNS{ + ConfigFile: tmpFile, + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + assert.NoError(t, dns.validate()) +} + +func TestDNS_ConfigFile_IncorporatedFromDropIn(t *testing.T) { + tmpFile := createTempCorefile(t, ".:5353 {\n whoami\n reload\n}\n") + + yamlConfig := fmt.Sprintf("dns:\n configFile: %s\n", tmpFile) + config, err := getActiveConfigFromYAMLDropins([][]byte{[]byte(yamlConfig)}) + require.NoError(t, err) + assert.Equal(t, tmpFile, config.DNS.ConfigFile) +} + +func createTempCorefile(t *testing.T, content string) string { + t.Helper() + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "Corefile") + require.NoError(t, os.WriteFile(tmpFile, []byte(content), 0644)) + return tmpFile +} From 390a845e84366af236a5ed744b3a624244f39e34 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 27 Apr 2026 17:42:07 +0200 Subject: [PATCH 5/6] USHIFT-6903: Add e2e tests for custom DNS configuration RobotFramework tests covering: - Custom Corefile is used by CoreDNS instead of the default - Runtime reload propagates changes without MicroShift restart - Cluster-local resolution works with custom Corefile --- test/assets/dns/Corefile.template | 24 +++ test/suites/standard1/dns-custom-config.robot | 137 ++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 test/assets/dns/Corefile.template create mode 100644 test/suites/standard1/dns-custom-config.robot diff --git a/test/assets/dns/Corefile.template b/test/assets/dns/Corefile.template new file mode 100644 index 0000000000..4039e7cc8b --- /dev/null +++ b/test/assets/dns/Corefile.template @@ -0,0 +1,24 @@ +.:5353 { + bufsize 1232 + errors + health { + lameduck 20s + } + ready + kubernetes cluster.local in-addr.arpa ip6.arpa { + pods insecure + fallthrough in-addr.arpa ip6.arpa + } + prometheus 127.0.0.1:9153 + forward . /etc/resolv.conf { + policy sequential + } + cache 900 { + denial 9984 30 + } + hosts { + ${COREFILE_HOST_IP} ${COREFILE_HOSTNAME} + fallthrough + } + reload +} diff --git a/test/suites/standard1/dns-custom-config.robot b/test/suites/standard1/dns-custom-config.robot new file mode 100644 index 0000000000..79d671cba7 --- /dev/null +++ b/test/suites/standard1/dns-custom-config.robot @@ -0,0 +1,137 @@ +*** Settings *** +Documentation Tests for custom DNS configuration (dns.configFile) + +Resource ../../resources/common.resource +Resource ../../resources/oc.resource +Resource ../../resources/microshift-config.resource +Resource ../../resources/microshift-process.resource +Resource ../../resources/microshift-network.resource +Resource ../../resources/hosts.resource + +Suite Setup Setup Suite With Namespace +Suite Teardown Teardown Suite With Namespace + +Test Tags slow + + +*** Variables *** +${FAKE_LISTEN_IP} 99.99.99.98 +${CUSTOM_COREFILE_PATH} /etc/microshift/dns/Corefile +${COREFILE_TEMPLATE} ./assets/dns/Corefile.template +${HOSTNAME} ${EMPTY} + +${CUSTOM_DNS_CONFIG} SEPARATOR=\n +... --- +... dns: +... \ \ configFile: /etc/microshift/dns/Corefile + + +*** Test Cases *** +Custom DNS Config File Is Used By CoreDNS + [Documentation] Provide a valid custom Corefile and verify CoreDNS uses it + ... instead of the default template-rendered configuration. + [Setup] Setup Custom Corefile With Hosts Entry + Verify ConfigMap Uses Custom Corefile + Resolve Host From Pod ${HOSTNAME} + [Teardown] Teardown Custom Corefile + +Runtime Reload Without MicroShift Restart + [Documentation] Modify the custom Corefile while MicroShift is running + ... and verify CoreDNS picks up the change without restart. + [Setup] Setup Custom Corefile With Hosts Entry + Resolve Host From Pod ${HOSTNAME} + ${updated_hostname}= Generate Random HostName + Update Corefile With New Host ${updated_hostname} + Resolve Host From Pod ${updated_hostname} + [Teardown] Teardown Custom Corefile + +Cluster Local Resolution With Custom Corefile + [Documentation] Verify that a custom Corefile with the kubernetes plugin + ... still resolves cluster-local services correctly. + [Setup] Setup Custom Corefile With Hosts Entry + Resolve Host From Pod kubernetes.default.svc.cluster.local + [Teardown] Teardown Custom Corefile + + +*** Keywords *** +Build Custom Corefile + [Documentation] Build a Corefile from the template file, substituting + ... the hostname and IP placeholders. + [Arguments] ${hostname} ${ip} + ${template}= OperatingSystem.Get File ${COREFILE_TEMPLATE} + VAR ${COREFILE_HOSTNAME}= ${hostname} + VAR ${COREFILE_HOST_IP}= ${ip} + ${corefile}= Replace Variables ${template} + RETURN ${corefile} + +Setup Custom Corefile With Hosts Entry + [Documentation] Create a custom Corefile with a fake hostname entry, + ... upload it, configure MicroShift to use it, and restart. + ${HOSTNAME}= Generate Random HostName + VAR ${HOSTNAME}= ${HOSTNAME} scope=SUITE + Add Fake IP To NIC ${FAKE_LISTEN_IP} + ${corefile}= Build Custom Corefile ${HOSTNAME} ${FAKE_LISTEN_IP} + Create Remote Dir For Path ${CUSTOM_COREFILE_PATH} + Upload String To File ${corefile} ${CUSTOM_COREFILE_PATH} + Drop In MicroShift Config ${CUSTOM_DNS_CONFIG} 20-dns-custom + Restart MicroShift + +Update Corefile With New Host + [Documentation] Add a new hosts entry to the custom Corefile without + ... restarting MicroShift. The file watcher and CoreDNS reload plugin + ... should pick up the change. + [Arguments] ${hostname} + ${corefile}= Build Custom Corefile ${hostname} ${FAKE_LISTEN_IP} + Upload String To File ${corefile} ${CUSTOM_COREFILE_PATH} + +Verify ConfigMap Uses Custom Corefile + [Documentation] Verify the dns-default ConfigMap contains custom content + ... rather than the default template-rendered Corefile. + ${corefile_data}= Oc Get JsonPath + ... configmap openshift-dns dns-default .data.Corefile + Should Contain ${corefile_data} ${HOSTNAME} + +Resolve Host From Pod + [Documentation] Verify DNS resolution from a pod using nslookup + [Arguments] ${hostname} + Wait Until Keyword Succeeds 40x 5s + ... Router Should Resolve Hostname ${hostname} + +Router Should Resolve Hostname + [Documentation] Check if the router pod resolves the given hostname + [Arguments] ${hostname} + ${output}= Oc Exec router-default nslookup ${hostname} openshift-ingress deployment + Should Contain ${output} Name: ${hostname} + +Teardown Custom Corefile + [Documentation] Remove custom Corefile, drop-in config, fake IP, and restart + Run Keywords + ... Remove Drop In MicroShift Config 20-dns-custom + ... AND Remove Custom Corefile + ... AND Remove Fake IP From NIC ${FAKE_LISTEN_IP} + ... AND Restart MicroShift + +Remove Custom Corefile + [Documentation] Remove the custom Corefile from the host + ${stdout} ${stderr} ${rc}= Execute Command + ... rm -f ${CUSTOM_COREFILE_PATH} + ... sudo=True return_rc=True return_stdout=True return_stderr=True + Should Be Equal As Integers 0 ${rc} + +Add Fake IP To NIC + [Documentation] Add the given IP to br-ex temporarily. + [Arguments] ${ip_address}=${FAKE_LISTEN_IP} ${nic_name}=br-ex + ${stdout} ${stderr} ${rc}= SSHLibrary.Execute Command + ... ip address add ${ip_address}/32 dev ${nic_name} + ... sudo=True return_rc=True return_stderr=True return_stdout=True + Log Many ${stdout} ${stderr} + Should Be Equal As Integers 0 ${rc} + +Remove Fake IP From NIC + [Documentation] Remove the given IP from br-ex. + [Arguments] ${ip_address}=${FAKE_LISTEN_IP} ${nic_name}=br-ex + ${stdout} ${stderr} ${rc}= SSHLibrary.Execute Command + ... ip address delete ${ip_address}/32 dev ${nic_name} + ... sudo=True return_rc=True return_stderr=True return_stdout=True + Log Many ${stdout} ${stderr} + Should Be Equal As Integers 0 ${rc} From 1bb1468936d794e0ba8eac1c8a0cb848c43712df Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Tue, 28 Apr 2026 14:32:06 +0200 Subject: [PATCH 6/6] USHIFT-6902: Address review feedback for custom DNS configuration - Use apierrors.IsNotFound for ConfigMap Get errors instead of treating any error as not-found - Merge annotations into existing ConfigMap instead of replacing them to avoid clobbering annotations set by the asset apply - Handle atomic-rename file updates (Rename/Remove events) and re-add the file watch on Create so the watcher survives vim/sed -i edits - Reject non-regular files (directories, FIFOs) in validateConfigFile - Fix Should Contain assertion in robot test that treated the hostname as a failure message instead of a second assertion - Add retry wrapper to ConfigMap verification after restart --- pkg/config/dns.go | 3 +++ pkg/config/dns_test.go | 13 +++++++++ pkg/controllers/dnsconfigurationwatcher.go | 27 +++++++++++++------ test/suites/standard1/dns-custom-config.robot | 11 ++++++-- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/pkg/config/dns.go b/pkg/config/dns.go index 3c42a10aba..101866a05d 100644 --- a/pkg/config/dns.go +++ b/pkg/config/dns.go @@ -95,6 +95,9 @@ func (t *DNS) validateConfigFile() error { } else if err != nil { return fmt.Errorf("error checking dns config file %s: %v", t.ConfigFile, err) } + if !fi.Mode().IsRegular() { + return fmt.Errorf("dns config file %s must be a regular file", t.ConfigFile) + } if fi.Size() == 0 { return fmt.Errorf("dns config file %s is empty", t.ConfigFile) diff --git a/pkg/config/dns_test.go b/pkg/config/dns_test.go index ad1766a878..b04b1bc715 100644 --- a/pkg/config/dns_test.go +++ b/pkg/config/dns_test.go @@ -68,6 +68,19 @@ func TestDNS_ValidateConfigFile_NonExistentFile(t *testing.T) { assert.ErrorContains(t, err, "does not exist") } +func TestDNS_ValidateConfigFile_Directory(t *testing.T) { + tmpDir := t.TempDir() + + dns := DNS{ + ConfigFile: tmpDir, + Hosts: HostsConfig{ + Status: HostsStatusDisabled, + }, + } + err := dns.validate() + assert.ErrorContains(t, err, "must be a regular file") +} + func TestDNS_ValidateConfigFile_EmptyFile(t *testing.T) { tmpFile := createTempCorefile(t, "") diff --git a/pkg/controllers/dnsconfigurationwatcher.go b/pkg/controllers/dnsconfigurationwatcher.go index 85523174a1..7a8e663e55 100644 --- a/pkg/controllers/dnsconfigurationwatcher.go +++ b/pkg/controllers/dnsconfigurationwatcher.go @@ -11,6 +11,7 @@ import ( "github.com/fsnotify/fsnotify" "github.com/openshift/microshift/pkg/config" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" @@ -115,6 +116,9 @@ func (s *DNSConfigurationWatcherManager) eventLoop(ctx context.Context, watcher return fmt.Errorf("%s watcher channel closed", s.Name()) } if s.isRelevantEvent(event) { + if event.Op&fsnotify.Create == fsnotify.Create { + _ = watcher.Add(s.file) + } updated, newHash, updateErr := s.handleChange(ctx, kubeClient, lastHash) if updateErr != nil { klog.Errorf("%s failed to process DNS config file change: %v", s.Name(), updateErr) @@ -138,7 +142,8 @@ func (s *DNSConfigurationWatcherManager) isRelevantEvent(event fsnotify.Event) b if event.Name != s.file { return false } - return event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create + const mask = fsnotify.Write | fsnotify.Create | fsnotify.Rename | fsnotify.Remove + return event.Op&mask != 0 } func (s *DNSConfigurationWatcherManager) handleChange(ctx context.Context, kubeClient kubernetes.Interface, lastHash string) (bool, string, error) { @@ -202,17 +207,23 @@ func (s *DNSConfigurationWatcherManager) createOrUpdateConfigMap(ctx context.Con configMapsClient := client.CoreV1().ConfigMaps(dnsConfigMapNamespace) existing, err := configMapsClient.Get(ctx, dnsConfigMapName, metav1.GetOptions{}) - if err != nil { - _, err = configMapsClient.Create(ctx, configMap, metav1.CreateOptions{}) - if err != nil { + switch { + case apierrors.IsNotFound(err): + if _, err = configMapsClient.Create(ctx, configMap, metav1.CreateOptions{}); err != nil { return fmt.Errorf("failed to create ConfigMap: %w", err) } klog.Infof("%s created ConfigMap %s/%s", s.Name(), dnsConfigMapNamespace, dnsConfigMapName) - } else { + case err != nil: + return fmt.Errorf("failed to get ConfigMap: %w", err) + default: existing.Data = configMap.Data - existing.Annotations = configMap.Annotations - _, err = configMapsClient.Update(ctx, existing, metav1.UpdateOptions{}) - if err != nil { + if existing.Annotations == nil { + existing.Annotations = map[string]string{} + } + for k, v := range configMap.Annotations { + existing.Annotations[k] = v + } + if _, err = configMapsClient.Update(ctx, existing, metav1.UpdateOptions{}); err != nil { return fmt.Errorf("failed to update ConfigMap: %w", err) } klog.V(2).Infof("%s updated ConfigMap %s/%s", s.Name(), dnsConfigMapNamespace, dnsConfigMapName) diff --git a/test/suites/standard1/dns-custom-config.robot b/test/suites/standard1/dns-custom-config.robot index 79d671cba7..b04d38de2d 100644 --- a/test/suites/standard1/dns-custom-config.robot +++ b/test/suites/standard1/dns-custom-config.robot @@ -87,9 +87,15 @@ Update Corefile With New Host Verify ConfigMap Uses Custom Corefile [Documentation] Verify the dns-default ConfigMap contains custom content ... rather than the default template-rendered Corefile. + Wait Until Keyword Succeeds 20x 5s + ... ConfigMap Should Contain Hostname ${HOSTNAME} + +ConfigMap Should Contain Hostname + [Documentation] Check the dns-default ConfigMap Corefile data for the hostname + [Arguments] ${hostname} ${corefile_data}= Oc Get JsonPath ... configmap openshift-dns dns-default .data.Corefile - Should Contain ${corefile_data} ${HOSTNAME} + Should Contain ${corefile_data} ${hostname} Resolve Host From Pod [Documentation] Verify DNS resolution from a pod using nslookup @@ -101,7 +107,8 @@ Router Should Resolve Hostname [Documentation] Check if the router pod resolves the given hostname [Arguments] ${hostname} ${output}= Oc Exec router-default nslookup ${hostname} openshift-ingress deployment - Should Contain ${output} Name: ${hostname} + Should Contain ${output} Name: + Should Contain ${output} ${hostname} Teardown Custom Corefile [Documentation] Remove custom Corefile, drop-in config, fake IP, and restart