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/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/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 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..b567cbbc08 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,36 +68,31 @@ func dnsDefaults() DNS { } func (t *DNS) validate() error { - switch t.Hosts.Status { - case HostsStatusEnabled: - if t.Hosts.File == "" { - break - } + if t.ConfigFile != "" && t.Hosts.Status == HostsStatusEnabled { + return fmt.Errorf("dns.configFile and dns.hosts are mutually exclusive") + } - cleanPath := filepath.Clean(t.Hosts.File) + if err := t.validateConfigFile(); err != nil { + return err + } - 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()) - } - if !filepath.IsAbs(cleanPath) { - return fmt.Errorf("hosts file path must be absolute: got %s", t.Hosts.File) - } + return t.validateHosts() +} - _, err = os.Stat(cleanPath) - if os.IsNotExist(err) { - return fmt.Errorf("hosts file %s does not exist", t.Hosts.File) - } else if err != nil { - return fmt.Errorf("error checking hosts file %s: %v", t.Hosts.File, err) - } +func (t *DNS) validateConfigFile() error { + if t.ConfigFile == "" { + return nil + } + return validateFilePath(t.ConfigFile, "dns config file") +} - file, err := os.Open(t.Hosts.File) - if err != nil { - return fmt.Errorf("hosts file %s is not readable: %v", t.Hosts.File, err) +func (t *DNS) validateHosts() error { + switch t.Hosts.Status { + case HostsStatusEnabled: + if t.Hosts.File == "" { + break } - return file.Close() - + return validateFilePath(t.Hosts.File, "hosts file") case HostsStatusDisabled: return nil default: @@ -96,3 +100,34 @@ func (t *DNS) validate() error { } return nil } + +func validateFilePath(path, label string) error { + cleanPath := filepath.Clean(path) + if !filepath.IsAbs(cleanPath) { + return fmt.Errorf("%s path must be absolute: got %s", label, path) + } + + fi, err := os.Stat(cleanPath) + if os.IsNotExist(err) { + return fmt.Errorf("%s %s does not exist", label, path) + } else if err != nil { + return fmt.Errorf("error checking %s %s: %v", label, path, err) + } + if !fi.Mode().IsRegular() { + return fmt.Errorf("%s %s must be a regular file", label, path) + } + + if fi.Size() == 0 { + return fmt.Errorf("%s %s is empty", label, path) + } + + if fi.Size() > 1048576 { + return fmt.Errorf("%s %s exceeds 1MiB size limit (got %d bytes)", label, path, fi.Size()) + } + + file, err := os.Open(cleanPath) + if err != nil { + return fmt.Errorf("%s %s is not readable: %v", label, path, err) + } + return file.Close() +} diff --git a/pkg/config/dns_test.go b/pkg/config/dns_test.go new file mode 100644 index 0000000000..b3c21e63e1 --- /dev/null +++ b/pkg/config/dns_test.go @@ -0,0 +1,141 @@ +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_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, "") + + 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 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 +} diff --git a/pkg/controllers/dnsconfigurationwatcher.go b/pkg/controllers/dnsconfigurationwatcher.go new file mode 100644 index 0000000000..66908fd588 --- /dev/null +++ b/pkg/controllers/dnsconfigurationwatcher.go @@ -0,0 +1,33 @@ +package controllers + +import ( + "github.com/fsnotify/fsnotify" + "github.com/openshift/microshift/pkg/config" +) + +type DNSConfigurationWatcherManager struct { + fileWatcher +} + +func NewDNSConfigurationWatcherManager(cfg *config.Config) *DNSConfigurationWatcherManager { + return &DNSConfigurationWatcherManager{fileWatcher{cfg: fileWatcherConfig{ + serviceName: "dns-configuration-watcher-manager", + dependencies: []string{"infrastructure-services-manager"}, + file: cfg.DNS.ConfigFile, + kubeconfig: cfg.KubeConfigPath(config.KubeAdmin), + enabled: cfg.DNS.ConfigFile != "", + configMapNamespace: "openshift-dns", + configMapName: "dns-default", + configMapDataKey: "Corefile", + labels: map[string]string{ + "dns.operator.openshift.io/owning-dns": "default", + }, + annotations: map[string]string{ + "microshift.io/dns-config-file": cfg.DNS.ConfigFile, + }, + eventMask: fsnotify.Write | fsnotify.Create | fsnotify.Rename | fsnotify.Remove, + reAddOnCreate: true, + mergeAnnotations: true, + deleteOnDisable: false, + }}} +} diff --git a/pkg/controllers/filewatcher.go b/pkg/controllers/filewatcher.go new file mode 100644 index 0000000000..fe132eed94 --- /dev/null +++ b/pkg/controllers/filewatcher.go @@ -0,0 +1,284 @@ +package controllers + +import ( + "context" + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/fsnotify/fsnotify" + 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" + "k8s.io/klog/v2" +) + +type fileWatcherConfig struct { + serviceName string + dependencies []string + file string + kubeconfig string + enabled bool + configMapNamespace string + configMapName string + configMapDataKey string + labels map[string]string + annotations map[string]string + eventMask fsnotify.Op + reAddOnCreate bool + mergeAnnotations bool + deleteOnDisable bool +} + +type fileWatcher struct { + cfg fileWatcherConfig +} + +func (fw *fileWatcher) Name() string { return fw.cfg.serviceName } +func (fw *fileWatcher) Dependencies() []string { return fw.cfg.dependencies } + +func (fw *fileWatcher) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error { + defer close(stopped) + + if !fw.cfg.enabled { + klog.Infof("%s is disabled (not configured)", fw.Name()) + fw.cleanupOnDisable(ctx) + close(ready) + return ctx.Err() + } + + kubeClient, err := fw.createKubeClient() + if err != nil { + klog.Errorf("%s failed to create Kubernetes client: %v", fw.Name(), err) + return err + } + + if err := fw.updateConfigMap(ctx, kubeClient); err != nil { + klog.Errorf("%s failed to create initial ConfigMap: %v", fw.Name(), err) + return err + } + + watcher, err := fsnotify.NewWatcher() + if err != nil { + klog.Errorf("%s failed to create file watcher: %v", fw.Name(), err) + return err + } + defer func() { + if cerr := watcher.Close(); cerr != nil { + klog.Errorf("%s failed to close file watcher: %v", fw.Name(), cerr) + } + }() + + if err := fw.setupWatches(watcher); err != nil { + return err + } + close(ready) + + lastHash, err := fw.getFileHash() + if err != nil { + klog.Warningf("%s failed to get initial file hash: %v", fw.Name(), err) + } + + klog.Infof("%s is ready and watching %s", fw.Name(), fw.cfg.file) + + return fw.eventLoop(ctx, watcher, kubeClient, lastHash) +} + +func (fw *fileWatcher) setupWatches(watcher *fsnotify.Watcher) error { + filesToWatch := []string{ + fw.cfg.file, + filepath.Dir(fw.cfg.file), + } + for i, file := range filesToWatch { + if err := watcher.Add(file); err != nil { + if i == 0 { + klog.Errorf("%s failed to watch file %s: %v", fw.Name(), fw.cfg.file, err) + return err + } + klog.Warningf("%s failed to watch directory %s: %v", fw.Name(), file, err) + } + } + return nil +} + +func (fw *fileWatcher) eventLoop(ctx context.Context, watcher *fsnotify.Watcher, kubeClient kubernetes.Interface, initHash string) error { + lastHash := initHash + + for { + select { + case <-ctx.Done(): + klog.Infof("%s stopping", fw.Name()) + return ctx.Err() + + case event, ok := <-watcher.Events: + if !ok { + return fmt.Errorf("%s watcher channel closed", fw.Name()) + } + if fw.isRelevantEvent(event) { + if fw.cfg.reAddOnCreate && event.Op&fsnotify.Create == fsnotify.Create { + _ = watcher.Add(fw.cfg.file) + } + updated, newHash, updateErr := fw.handleChange(ctx, kubeClient, lastHash) + if updateErr != nil { + klog.Errorf("%s failed to process file change: %v", fw.Name(), updateErr) + continue + } + if updated { + lastHash = newHash + } + } + + case err, ok := <-watcher.Errors: + if !ok { + return fmt.Errorf("%s watcher error channel closed", fw.Name()) + } + klog.Errorf("%s watcher error: %v", fw.Name(), err) + } + } +} + +func (fw *fileWatcher) isRelevantEvent(event fsnotify.Event) bool { + if event.Name != fw.cfg.file { + return false + } + return event.Op&fw.cfg.eventMask != 0 +} + +func (fw *fileWatcher) handleChange(ctx context.Context, kubeClient kubernetes.Interface, lastHash string) (bool, string, error) { + klog.Infof("%s detected change in %s", fw.Name(), fw.cfg.file) + if _, err := os.Stat(fw.cfg.file); os.IsNotExist(err) { + klog.Warningf("%s watched file %s was removed, keeping last known ConfigMap content", fw.Name(), fw.cfg.file) + return false, lastHash, err + } + currentHash, err := fw.getFileHash() + if err != nil { + klog.Warningf("%s failed to get file hash after change: %v", fw.Name(), err) + return false, lastHash, err + } + if currentHash == lastHash { + klog.V(2).Infof("%s file hash unchanged, skipping update", fw.Name()) + return false, lastHash, nil + } + if err := fw.updateConfigMap(ctx, kubeClient); err != nil { + klog.Errorf("%s failed to update ConfigMap: %v", fw.Name(), err) + return false, lastHash, err + } + klog.Infof("%s successfully updated ConfigMap %s/%s", fw.Name(), fw.cfg.configMapNamespace, fw.cfg.configMapName) + return true, currentHash, nil +} + +func (fw *fileWatcher) createKubeClient() (*kubernetes.Clientset, error) { + cfg, err := clientcmd.BuildConfigFromFlags("", fw.cfg.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 (fw *fileWatcher) updateConfigMap(ctx context.Context, client kubernetes.Interface) error { + content, err := os.ReadFile(fw.cfg.file) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", fw.cfg.file, err) + } + return fw.createOrUpdateConfigMap(ctx, client, string(content)) +} + +func (fw *fileWatcher) createOrUpdateConfigMap(ctx context.Context, client kubernetes.Interface, content string) error { + annotations := fw.buildAnnotations() + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: fw.cfg.configMapName, + Namespace: fw.cfg.configMapNamespace, + Labels: fw.cfg.labels, + Annotations: annotations, + }, + Data: map[string]string{ + fw.cfg.configMapDataKey: content, + }, + } + + configMapsClient := client.CoreV1().ConfigMaps(fw.cfg.configMapNamespace) + + existing, err := configMapsClient.Get(ctx, fw.cfg.configMapName, metav1.GetOptions{}) + 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", fw.Name(), fw.cfg.configMapNamespace, fw.cfg.configMapName) + case err != nil: + return fmt.Errorf("failed to get ConfigMap: %w", err) + default: + existing.Data = configMap.Data + if fw.cfg.mergeAnnotations { + if existing.Annotations == nil { + existing.Annotations = map[string]string{} + } + for k, v := range annotations { + existing.Annotations[k] = v + } + } else { + existing.Annotations = annotations + } + 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", fw.Name(), fw.cfg.configMapNamespace, fw.cfg.configMapName) + } + + return nil +} + +func (fw *fileWatcher) buildAnnotations() map[string]string { + annotations := make(map[string]string, len(fw.cfg.annotations)+1) + for k, v := range fw.cfg.annotations { + annotations[k] = v + } + annotations["microshift.io/last-updated"] = time.Now().Format(time.RFC3339) + return annotations +} + +func (fw *fileWatcher) cleanupOnDisable(ctx context.Context) { + if !fw.cfg.deleteOnDisable { + return + } + kubeClient, err := fw.createKubeClient() + if err != nil { + klog.Warningf("%s could not create Kubernetes client to delete ConfigMap: %v", fw.Name(), err) + return + } + if err := fw.deleteConfigMap(ctx, kubeClient); err != nil { + klog.Warningf("%s failed to delete ConfigMap when disabled: %v", fw.Name(), err) + } +} + +func (fw *fileWatcher) deleteConfigMap(ctx context.Context, client kubernetes.Interface) error { + configMapsClient := client.CoreV1().ConfigMaps(fw.cfg.configMapNamespace) + err := configMapsClient.Delete(ctx, fw.cfg.configMapName, metav1.DeleteOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + klog.V(2).Infof("%s ConfigMap %s/%s does not exist, nothing to delete", fw.Name(), fw.cfg.configMapNamespace, fw.cfg.configMapName) + return nil + } + return fmt.Errorf("failed to delete ConfigMap: %w", err) + } + klog.Infof("%s deleted ConfigMap %s/%s", fw.Name(), fw.cfg.configMapNamespace, fw.cfg.configMapName) + return nil +} + +func (fw *fileWatcher) getFileHash() (string, error) { + content, err := os.ReadFile(fw.cfg.file) + if err != nil { + return "", err + } + hash := sha256.Sum256(content) + return fmt.Sprintf("%x", hash), nil +} diff --git a/pkg/controllers/hostswatcher.go b/pkg/controllers/hostswatcher.go index f666cd3e90..9ff4f36996 100644 --- a/pkg/controllers/hostswatcher.go +++ b/pkg/controllers/hostswatcher.go @@ -1,280 +1,36 @@ 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" - 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" - "k8s.io/klog/v2" -) - -const ( - targetNameSpace = "openshift-dns" - configMapName = "hosts-file" ) type HostsWatcherManager struct { - file string - status config.HostsStatusEnum - kubeconfig string + fileWatcher } func NewHostsWatcherManager(cfg *config.Config) *HostsWatcherManager { - return &HostsWatcherManager{ - file: cfg.DNS.Hosts.File, - status: cfg.DNS.Hosts.Status, - kubeconfig: cfg.KubeConfigPath(config.KubeAdmin), - } -} - -func (s *HostsWatcherManager) Name() string { return "hosts-watcher-manager" } -func (s *HostsWatcherManager) Dependencies() []string { return []string{"kube-apiserver"} } - -func (s *HostsWatcherManager) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error { - defer close(stopped) - - if s.status != config.HostsStatusEnabled { - klog.Infof("%s is disabled (not configured)", s.Name()) - // Delete ConfigMap if it exists when service is disabled - if kubeClient, err := s.createKubeClient(); err == nil { - if err := s.deleteConfigMap(ctx, kubeClient); err != nil { - klog.Warningf("%s failed to delete ConfigMap when disabled: %v", s.Name(), err) - } - } else { - klog.Warningf("%s could not create Kubernetes client to delete ConfigMap: %v", s.Name(), err) - } - defer 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.updateConfigMaps(ctx, kubeClient); err != nil { - klog.Errorf("%s failed to create initial ConfigMaps: %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) - klog.Infof("%s ready and watching for changes", s.Name()) - - lastHash, err := s.getFileHash(s.file) - if err != nil { - klog.Warningf("%s failed to get initial file hash: %v", s.Name(), err) - } - - klog.Infof("%s is ready", s.Name()) - - return s.eventLoop(ctx, watcher, kubeClient, lastHash) -} - -func (s *HostsWatcherManager) 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 { - // Warn if directory, error out if file - if i == 0 { - klog.Errorf("%s failed to watch hosts file %s: %v", s.Name(), s.file, err) - return err - } - klog.Warningf("%s failed to watch hosts directory %s: %v", s.Name(), file, err) - } - } - return nil -} - -func (s *HostsWatcherManager) 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.isRelevantHostsEvent(event) { - updated, newHash, updateErr := s.handleHostsChange(ctx, kubeClient, lastHash) - if updateErr != nil { - klog.Errorf("%s failed to process hosts 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 *HostsWatcherManager) isRelevantHostsEvent(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 *HostsWatcherManager) handleHostsChange(ctx context.Context, kubeClient kubernetes.Interface, lastHash string) (bool, string, error) { - klog.Infof("%s detected change in hosts file: %s", s.Name(), s.file) - currentHash, err := s.getFileHash(s.file) - 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.updateConfigMaps(ctx, kubeClient); err != nil { - klog.Errorf("%s failed to update ConfigMaps: %v", s.Name(), err) - return false, currentHash, err - } else { - klog.Infof("%s successfully updated ConfigMaps in namespaces: %v", s.Name(), targetNameSpace) - } - return true, currentHash, nil -} - -func (s *HostsWatcherManager) createKubeClient() (*kubernetes.Clientset, error) { - config, err := clientcmd.BuildConfigFromFlags("", s.kubeconfig) - if err != nil { - return nil, fmt.Errorf("failed to build kubeconfig: %w", err) - } - - client, err := kubernetes.NewForConfig(config) - if err != nil { - return nil, fmt.Errorf("failed to create Kubernetes client: %w", err) - } - - return client, nil -} - -func (s *HostsWatcherManager) updateConfigMaps(ctx context.Context, client kubernetes.Interface) error { - hostsContent, err := s.readHostsFile() - if err != nil { - return fmt.Errorf("failed to read hosts file: %w", err) - } - - if err := s.createOrUpdateConfigMap(ctx, client, targetNameSpace, hostsContent); err != nil { - klog.Errorf("%s failed to update ConfigMap in namespace %s: %v", s.Name(), targetNameSpace, err) - } - - return nil -} - -func (s *HostsWatcherManager) createOrUpdateConfigMap(ctx context.Context, client kubernetes.Interface, namespace string, hostsContent string) error { - configMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: configMapName, - Namespace: namespace, - Labels: map[string]string{ - "app.kubernetes.io/name": "microshift-hosts-watcher", - "app.kubernetes.io/component": "hosts-file-sync", - "app.kubernetes.io/managed-by": "microshift", - // Restrict access to only CoreDNS pods - "microshift.io/access-restricted": "coredns-only", - }, - Annotations: map[string]string{ - "microshift.io/hosts-file-path": s.file, - "microshift.io/last-updated": time.Now().Format(time.RFC3339), - }, + return &HostsWatcherManager{fileWatcher{cfg: fileWatcherConfig{ + serviceName: "hosts-watcher-manager", + dependencies: []string{"infrastructure-services-manager"}, + file: cfg.DNS.Hosts.File, + kubeconfig: cfg.KubeConfigPath(config.KubeAdmin), + enabled: cfg.DNS.Hosts.Status == config.HostsStatusEnabled, + configMapNamespace: "openshift-dns", + configMapName: "hosts-file", + configMapDataKey: "hosts", + labels: map[string]string{ + "app.kubernetes.io/name": "microshift-hosts-watcher", + "app.kubernetes.io/component": "hosts-file-sync", + "app.kubernetes.io/managed-by": "microshift", + "microshift.io/access-restricted": "coredns-only", }, - Data: map[string]string{ - "hosts": hostsContent, + annotations: map[string]string{ + "microshift.io/hosts-file-path": cfg.DNS.Hosts.File, }, - } - - configMapsClient := client.CoreV1().ConfigMaps(namespace) - - // Try to get existing ConfigMap - existing, err := configMapsClient.Get(ctx, configMapName, metav1.GetOptions{}) - if err != nil { - // ConfigMap doesn't exist, create it - _, 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 in namespace %s", s.Name(), configMapName, namespace) - } else { - // ConfigMap exists, update it - 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 in namespace %s", s.Name(), configMapName, namespace) - } - - return nil -} - -func (s *HostsWatcherManager) readHostsFile() (string, error) { - content, err := os.ReadFile(s.file) - if err != nil { - return "", fmt.Errorf("failed to read hosts file %s: %w", s.file, err) - } - return string(content), nil -} - -func (s *HostsWatcherManager) getFileHash(filePath string) (string, error) { - content, err := os.ReadFile(filePath) - if err != nil { - return "", err - } - hash := sha256.Sum256(content) - return fmt.Sprintf("%x", hash), nil -} - -func (s *HostsWatcherManager) deleteConfigMap(ctx context.Context, client kubernetes.Interface) error { - configMapsClient := client.CoreV1().ConfigMaps(targetNameSpace) - err := configMapsClient.Delete(ctx, configMapName, metav1.DeleteOptions{}) - if err != nil { - // If ConfigMap doesn't exist, that's fine - it's already deleted - if apierrors.IsNotFound(err) { - klog.V(2).Infof("%s ConfigMap %s in namespace %s does not exist, nothing to delete", s.Name(), configMapName, targetNameSpace) - return nil - } - return fmt.Errorf("failed to delete ConfigMap: %w", err) - } - klog.Infof("%s deleted ConfigMap %s in namespace %s", s.Name(), configMapName, targetNameSpace) - return nil + eventMask: fsnotify.Write | fsnotify.Create, + reAddOnCreate: false, + mergeAnnotations: false, + deleteOnDisable: true, + }}} } 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/resources/dns.resource b/test/resources/dns.resource new file mode 100644 index 0000000000..46a494408b --- /dev/null +++ b/test/resources/dns.resource @@ -0,0 +1,38 @@ +*** Settings *** +Documentation Shared keywords for DNS test suites + +Library SSHLibrary +Resource oc.resource + + +*** Keywords *** +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: + Should Contain ${output} ${hostname} + +Add Fake IP To NIC + [Documentation] Add the given IP to br-ex temporarily. + [Arguments] ${ip_address} ${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} ${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} diff --git a/test/suites/standard1/dns-custom-config.robot b/test/suites/standard1/dns-custom-config.robot new file mode 100644 index 0000000000..ab05732521 --- /dev/null +++ b/test/suites/standard1/dns-custom-config.robot @@ -0,0 +1,129 @@ +*** 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/dns.resource +Resource ../../resources/hosts.resource +Library ../../resources/journalctl.py + +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} + Wait Until Keyword Succeeds 20x 5s + ... ConfigMap Should Contain Hostname ${updated_hostname} + Resolve Host From Pod ${updated_hostname} + [Teardown] Teardown Custom Corefile + +ConfigMap Unchanged After Corefile Removal + [Documentation] Remove the custom Corefile while MicroShift is running + ... and verify the ConfigMap retains the last known content. + [Setup] Setup Custom Corefile With Hosts Entry + Verify ConfigMap Uses Custom Corefile + ${cursor}= Get Journal Cursor + Remove Custom Corefile + Pattern Should Appear In Log Output + ... ${cursor} watched file.*was removed, keeping last known ConfigMap content + ConfigMap Should Contain Hostname ${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. + 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} + +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} diff --git a/test/suites/standard1/dns.robot b/test/suites/standard1/dns.robot index 84a690ccac..5d22c4d5cb 100644 --- a/test/suites/standard1/dns.robot +++ b/test/suites/standard1/dns.robot @@ -4,6 +4,7 @@ Documentation Core DNS smoke tests Resource ../../resources/common.resource Resource ../../resources/oc.resource Resource ../../resources/microshift-network.resource +Resource ../../resources/dns.resource Resource ../../resources/microshift-config.resource Resource ../../resources/microshift-process.resource Resource ../../resources/hosts.resource @@ -91,18 +92,6 @@ Get Hosts Config Custom END RETURN ${config} -Resolve Host From Pod - [Documentation] Resolve host from pod - [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} - ${fuse_device}= Oc Exec router-default nslookup ${hostname} openshift-ingress deployment - Should Contain ${fuse_device} Name: ${hostname} - Setup With Custom Config [Documentation] Install a custom config and restart MicroShift [Arguments] ${config_content} ${hostsFile} @@ -128,24 +117,6 @@ Teardown Hosts File ... AND ... Remove Drop In MicroShift Config 20-dns -Add Fake IP To NIC - [Documentation] Add the given IP to the given NIC 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 the given NIC. - [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} - Check CoreDNS Hosts Feature [Documentation] Skip suite if CoreDNS hosts feature is not available ${config}= Show Config default