diff --git a/components/linux/v20260301/configure.go b/components/linux/v20260301/configure.go index 41ab94e..118f3fe 100644 --- a/components/linux/v20260301/configure.go +++ b/components/linux/v20260301/configure.go @@ -120,12 +120,13 @@ kernel.panic = 10 kernel.panic_on_oops = 1 ` -const sysctlConfigPath = "/etc/sysctl.d/999-sysctl-aks.conf" +const sysctlAKSConfigPath = "/etc/sysctl.d/999-sysctl-aks.conf" +const sysctlConfigPath = "/etc/sysctl.conf" func (a *configureBaseOSAction) ensureSysctlConfig(ctx context.Context) error { expectedConfig := []byte(strings.TrimSpace(sysctlSettings)) - currentConfig, err := os.ReadFile(sysctlConfigPath) + currentConfig, err := os.ReadFile(sysctlAKSConfigPath) switch { case errors.Is(err, os.ErrNotExist): // file does not exist, will create later @@ -138,9 +139,17 @@ func (a *configureBaseOSAction) ensureSysctlConfig(ctx context.Context) error { } } - if err := utilio.WriteFile(sysctlConfigPath, expectedConfig, 0644); err != nil { + if err := utilio.WriteFile(sysctlAKSConfigPath, expectedConfig, 0644); err != nil { return err } + + // sysctl --system loads /etc/sysctl.conf AFTER all /etc/sysctl.d/*.conf + // files, so any conflicting settings in /etc/sysctl.conf will override + // our 999-sysctl-aks.conf. Comment out conflicting lines before applying. + if err := a.sanitizeSysctlConf(sysctlConfigPath); err != nil { + return err + } + if err := utilexec.New().CommandContext(ctx, "sysctl", "--system").Run(); err != nil { return err } @@ -148,6 +157,30 @@ func (a *configureBaseOSAction) ensureSysctlConfig(ctx context.Context) error { return nil } +// sanitizeSysctlConf reads /etc/sysctl.conf and comments out any uncommented +// lines that set keys also present in sysctlSettings. This prevents +// /etc/sysctl.conf (loaded last by sysctl --system) from overriding the +// settings written to 999-sysctl-aks.conf. +func (a *configureBaseOSAction) sanitizeSysctlConf(path string) error { + // extract sysctl keys from our managed settings + managedKeys := make(map[string]bool) + for _, line := range strings.Split(sysctlSettings, "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + key, _, ok := strings.Cut(line, "=") + if ok { + managedKeys[strings.TrimSpace(key)] = true + } + } + + return commentOutMatchingLines(path, func(trimmedLine string) bool { + key, _, ok := strings.Cut(trimmedLine, "=") + return ok && managedKeys[strings.TrimSpace(key)] + }) +} + const fstabPath = "/etc/fstab" func (a *configureBaseOSAction) disableSwap(ctx context.Context) error { @@ -162,14 +195,23 @@ func (a *configureBaseOSAction) disableSwap(ctx context.Context) error { return nil } -// commentOutSwapInFstab reads the fstab file at the given path, comments out -// any uncommented lines containing "swap", and writes the result back. A backup -// of the original file is saved to .bak before any modifications are made. +// commentOutSwapInFstab comments out any uncommented fstab lines containing +// "swap". A backup is saved to .bak before any modifications. func (a *configureBaseOSAction) commentOutSwapInFstab(path string) error { + return commentOutMatchingLines(path, func(trimmedLine string) bool { + return strings.Contains(trimmedLine, "swap") + }) +} + +// commentOutMatchingLines reads the file at path, comments out any uncommented +// lines for which shouldComment returns true (called with the trimmed line), +// and writes the result back. A backup of the original file is saved to +// .bak before any modifications are made. If the file does not exist, +// it returns nil. +func commentOutMatchingLines(path string, shouldComment func(trimmedLine string) bool) error { content, err := os.ReadFile(path) // #nosec - path has been validated by caller if err != nil { if errors.Is(err, os.ErrNotExist) { - // no fstab, nothing to do return nil } return err @@ -179,11 +221,10 @@ func (a *configureBaseOSAction) commentOutSwapInFstab(path string) error { modified := false for i, line := range lines { trimmed := strings.TrimSpace(line) - // skip empty lines and already-commented lines if trimmed == "" || strings.HasPrefix(trimmed, "#") { continue } - if strings.Contains(trimmed, "swap") { + if shouldComment(trimmed) { lines[i] = "# " + line modified = true } @@ -193,7 +234,6 @@ func (a *configureBaseOSAction) commentOutSwapInFstab(path string) error { return nil } - // back up the original fstab before writing changes if err := utilio.WriteFile(path+".bak", content, 0644); err != nil { return err } diff --git a/components/linux/v20260301/configure_test.go b/components/linux/v20260301/configure_test.go index b4acd5a..014a89c 100644 --- a/components/linux/v20260301/configure_test.go +++ b/components/linux/v20260301/configure_test.go @@ -3,9 +3,42 @@ package v20260301 import ( "os" "path/filepath" + "strings" "testing" ) +func assertFileAndBackup(t *testing.T, path, wantOutput, originalInput string, wantBackup bool) { + t.Helper() + + got, err := os.ReadFile(path) // #nosec - test helper + if err != nil { + t.Fatalf("failed to read file after call: %v", err) + } + if string(got) != wantOutput { + t.Errorf("file content mismatch\ngot:\n%s\nwant:\n%s", string(got), wantOutput) + } + + backupPath := path + ".bak" + _, backupErr := os.Stat(backupPath) + backupExists := backupErr == nil + + if wantBackup && !backupExists { + t.Errorf("expected backup file %s to exist, but it does not", backupPath) + } + if !wantBackup && backupExists { + t.Errorf("expected no backup file, but %s exists", backupPath) + } + if wantBackup && backupExists { + backup, err := os.ReadFile(backupPath) // #nosec - test helper + if err != nil { + t.Fatalf("failed to read backup file: %v", err) + } + if string(backup) != originalInput { + t.Errorf("backup content mismatch\ngot:\n%s\nwant:\n%s", string(backup), originalInput) + } + } +} + func TestCommentOutSwapInFstab(t *testing.T) { t.Parallel() @@ -117,33 +150,7 @@ func TestCommentOutSwapInFstab(t *testing.T) { t.Fatalf("commentOutSwapInFstab() returned error: %v", err) } - got, err := os.ReadFile(fstab) // #nosec - path has been validated by caller - if err != nil { - t.Fatalf("failed to read fstab after call: %v", err) - } - if string(got) != tt.wantOutput { - t.Errorf("fstab content mismatch\ngot:\n%s\nwant:\n%s", string(got), tt.wantOutput) - } - - backupPath := fstab + ".bak" - _, backupErr := os.Stat(backupPath) - backupExists := backupErr == nil - - if tt.wantBackup && !backupExists { - t.Errorf("expected backup file %s to exist, but it does not", backupPath) - } - if !tt.wantBackup && backupExists { - t.Errorf("expected no backup file, but %s exists", backupPath) - } - if tt.wantBackup && backupExists { - backup, err := os.ReadFile(backupPath) // #nosec - path has been validated by caller - if err != nil { - t.Fatalf("failed to read backup file: %v", err) - } - if string(backup) != tt.input { - t.Errorf("backup content mismatch\ngot:\n%s\nwant:\n%s", string(backup), tt.input) - } - } + assertFileAndBackup(t, fstab, tt.wantOutput, tt.input, tt.wantBackup) }) } } @@ -203,3 +210,245 @@ func TestCommentOutSwapInFstab_Idempotent(t *testing.T) { t.Errorf("after second call: got:\n%s\nwant:\n%s", string(got2), want) } } + +func TestSanitizeSysctlConf(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantOutput string + wantBackup bool + }{ + { + name: "comments out conflicting rp_filter lines", + input: `# sysctl settings +net.ipv4.conf.default.rp_filter=1 +net.ipv4.conf.all.rp_filter=1 +`, + wantOutput: `# sysctl settings +# net.ipv4.conf.default.rp_filter=1 +# net.ipv4.conf.all.rp_filter=1 +`, + wantBackup: true, + }, + { + name: "comments out conflicting ip_forward", + input: `net.ipv4.ip_forward = 0 +net.ipv6.conf.all.disable_ipv6 = 1 +`, + wantOutput: `# net.ipv4.ip_forward = 0 +net.ipv6.conf.all.disable_ipv6 = 1 +`, + wantBackup: true, + }, + { + name: "leaves non-conflicting settings unchanged", + input: `net.ipv6.conf.all.disable_ipv6 = 1 +fs.file-max = 65535 +`, + wantOutput: `net.ipv6.conf.all.disable_ipv6 = 1 +fs.file-max = 65535 +`, + wantBackup: false, + }, + { + name: "already commented lines are left alone", + input: `# net.ipv4.conf.all.rp_filter=1 +# net.ipv4.ip_forward = 0 +`, + wantOutput: `# net.ipv4.conf.all.rp_filter=1 +# net.ipv4.ip_forward = 0 +`, + wantBackup: false, + }, + { + name: "empty file leaves file unchanged", + input: "", + wantOutput: "", + wantBackup: false, + }, + { + name: "mix of commented and uncommented conflicting lines", + input: `# net.ipv4.conf.all.rp_filter=2 +net.ipv4.conf.default.rp_filter=1 +net.ipv6.conf.all.disable_ipv6 = 1 +`, + wantOutput: `# net.ipv4.conf.all.rp_filter=2 +# net.ipv4.conf.default.rp_filter=1 +net.ipv6.conf.all.disable_ipv6 = 1 +`, + wantBackup: true, + }, + { + name: "handles spaces around equals sign", + input: `net.ipv4.ip_forward = 0 +net.ipv4.conf.all.rp_filter = 1 +`, + wantOutput: `# net.ipv4.ip_forward = 0 +# net.ipv4.conf.all.rp_filter = 1 +`, + wantBackup: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + sysctlConf := filepath.Join(dir, "sysctl.conf") + + if err := os.WriteFile(sysctlConf, []byte(tt.input), 0600); err != nil { + t.Fatalf("failed to write test sysctl.conf: %v", err) + } + + a := &configureBaseOSAction{} + if err := a.sanitizeSysctlConf(sysctlConf); err != nil { + t.Fatalf("sanitizeSysctlConf() returned error: %v", err) + } + + assertFileAndBackup(t, sysctlConf, tt.wantOutput, tt.input, tt.wantBackup) + }) + } +} + +func TestSanitizeSysctlConf_FileNotExist(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "nonexistent") + + a := &configureBaseOSAction{} + if err := a.sanitizeSysctlConf(path); err != nil { + t.Fatalf("expected no error for missing file, got: %v", err) + } +} + +func TestSanitizeSysctlConf_Idempotent(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + sysctlConf := filepath.Join(dir, "sysctl.conf") + + input := `net.ipv4.conf.all.rp_filter=1 +net.ipv4.ip_forward = 0 +` + want := `# net.ipv4.conf.all.rp_filter=1 +# net.ipv4.ip_forward = 0 +` + + if err := os.WriteFile(sysctlConf, []byte(input), 0600); err != nil { + t.Fatalf("failed to write test sysctl.conf: %v", err) + } + + a := &configureBaseOSAction{} + + if err := a.sanitizeSysctlConf(sysctlConf); err != nil { + t.Fatalf("first call returned error: %v", err) + } + got, err := os.ReadFile(sysctlConf) // #nosec - path has been validated by caller + if err != nil { + t.Fatalf("failed to read sysctl.conf: %v", err) + } + if string(got) != want { + t.Errorf("after first call: got:\n%s\nwant:\n%s", string(got), want) + } + + if err := a.sanitizeSysctlConf(sysctlConf); err != nil { + t.Fatalf("second call returned error: %v", err) + } + got2, err := os.ReadFile(sysctlConf) // #nosec - path has been validated by caller + if err != nil { + t.Fatalf("failed to read sysctl.conf: %v", err) + } + if string(got2) != want { + t.Errorf("after second call: got:\n%s\nwant:\n%s", string(got2), want) + } +} + +func TestCommentOutMatchingLines(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + shouldComment func(string) bool + wantOutput string + wantBackup bool + }{ + { + name: "comments out lines matching predicate", + input: "keep this\nremove this\nkeep that\n", + shouldComment: func(line string) bool { + return strings.Contains(line, "remove") + }, + wantOutput: "keep this\n# remove this\nkeep that\n", + wantBackup: true, + }, + { + name: "no matches leaves file unchanged", + input: "keep this\nkeep that\n", + shouldComment: func(line string) bool { + return strings.Contains(line, "remove") + }, + wantOutput: "keep this\nkeep that\n", + wantBackup: false, + }, + { + name: "skips already-commented lines", + input: "# remove this\nremove this\n", + shouldComment: func(line string) bool { + return strings.Contains(line, "remove") + }, + wantOutput: "# remove this\n# remove this\n", + wantBackup: true, + }, + { + name: "preserves leading whitespace when commenting", + input: " remove this\n", + shouldComment: func(line string) bool { + return strings.Contains(line, "remove") + }, + wantOutput: "# remove this\n", + wantBackup: true, + }, + { + name: "empty file is a no-op", + input: "", + shouldComment: func(string) bool { return true }, + wantOutput: "", + wantBackup: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "testfile") + + if err := os.WriteFile(path, []byte(tt.input), 0600); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + if err := commentOutMatchingLines(path, tt.shouldComment); err != nil { + t.Fatalf("commentOutMatchingLines() returned error: %v", err) + } + + assertFileAndBackup(t, path, tt.wantOutput, tt.input, tt.wantBackup) + }) + } +} + +func TestCommentOutMatchingLines_FileNotExist(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "nonexistent") + + if err := commentOutMatchingLines(path, func(string) bool { return true }); err != nil { + t.Fatalf("expected no error for missing file, got: %v", err) + } +}