From 0780516e0b50d94476da1d2799a16a397a1db19b Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 2 Apr 2024 08:58:59 +0200 Subject: [PATCH] Improve k0s-managed containerd config detection The previous detection method was to read the whole file and search each line for the magic string "# k0s_managed=true". This seems excessive and unexpected, as it will catch any substring, even if it is not at the beginning of the file, the start of a line, or even inside a comment. It would even have caught this as part of a TOML string value. Change this behavior by expecting the first line of the file to be exactly this magic string. All files generated by k0s will have exactly that first line. Also return any previously swallowed errors back to the caller. Add more test cases to ensure that the generated default config is recognized as being managed by k0s, and that a file not starting with the magic line is recognized as not being managed by k0s. Signed-off-by: Tom Wieczorek --- pkg/component/worker/containerd/component.go | 26 +++++---- .../worker/containerd/component_test.go | 56 ++++++++++++++++++- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/pkg/component/worker/containerd/component.go b/pkg/component/worker/containerd/component.go index 9d67afe71fbc..53c1cbb3a07f 100644 --- a/pkg/component/worker/containerd/component.go +++ b/pkg/component/worker/containerd/component.go @@ -22,6 +22,7 @@ import ( "context" "crypto/md5" "encoding/hex" + "errors" "fmt" "io" "os" @@ -47,10 +48,7 @@ import ( "github.com/k0sproject/k0s/pkg/supervisor" ) -const magicMarker = "# k0s_managed=true" - -const confTmpl = ` -# k0s_managed=true +const confTmpl = `# k0s_managed=true # This is a placeholder configuration for k0s managed containerd. # If you wish to override the config, remove the first line and replace this file with your custom configuration. # For reference see https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md @@ -322,11 +320,12 @@ func (c *Component) Stop() error { // This is the md5sum of the default k0s containerd config file before 1.27 const pre1_27ConfigSum = "59039b43303742a5496b13fd57f9beec" -// isK0sManagedConfig checks if the config file is k0s managed -// - If the config file does not exist, it's k0s managed -// - If the config file md5sum matches the pre 1.27 config, it's k0s managed -// - If the config file has the magic marker, it's k0s managed -func isK0sManagedConfig(path string) (bool, error) { +// isK0sManagedConfig checks if the config file is k0s managed: +// - If the config file doesn't exist, it's k0s managed. +// - If the config file's md5sum matches the pre 1.27 config, it's k0s managed. +// - If the config file starts with the magic marker line "# k0s_managed=true", +// it's k0s managed. +func isK0sManagedConfig(path string) (_ bool, err error) { // If the file does not exist, it's k0s managed (new install) if !file.Exists(path) { return true, nil @@ -343,14 +342,17 @@ func isK0sManagedConfig(path string) (bool, error) { if err != nil { return false, err } - defer f.Close() + defer func() { err = errors.Join(err, f.Close()) }() scanner := bufio.NewScanner(f) for scanner.Scan() { - if strings.Contains(scanner.Text(), magicMarker) { + switch scanner.Text() { + case "": // K0s versions before 1.30 had a leading empty line. + continue + case "# k0s_managed=true": return true, nil } } - return false, nil + return false, scanner.Err() } func isPre1_27ManagedConfig(path string) (bool, error) { diff --git a/pkg/component/worker/containerd/component_test.go b/pkg/component/worker/containerd/component_test.go index fc40cf431664..1921fcba6a67 100644 --- a/pkg/component/worker/containerd/component_test.go +++ b/pkg/component/worker/containerd/component_test.go @@ -21,13 +21,67 @@ import ( "path/filepath" "testing" + "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" + workerconfig "github.com/k0sproject/k0s/pkg/component/worker/config" + "github.com/k0sproject/k0s/pkg/config" + "github.com/stretchr/testify/require" ) func Test_isK0sManagedConfig(t *testing.T) { t.Run("should return true if file does not exist", func(t *testing.T) { - isManaged, err := isK0sManagedConfig("non-existent.toml") + isManaged, err := isK0sManagedConfig(filepath.Join(t.TempDir(), "non-existent.toml")) + require.NoError(t, err) + require.True(t, isManaged) + }) + + t.Run("should return true for generated default config", func(t *testing.T) { + defaultConfigPath := filepath.Join(t.TempDir(), "default.toml") + + underTest := Component{ + K0sVars: &config.CfgVars{ + RunDir: /* The merged config file will be written here: */ t.TempDir(), + }, + confPath:/* The main config file will be written here: */ defaultConfigPath, + importsPath:/* some empty dir: */ t.TempDir(), + Profile:/* Some non-nil pause image: */ &workerconfig.Profile{PauseImage: &v1beta1.ImageSpec{}}, + } + err := underTest.setupConfig() + + require.NoError(t, err) + require.FileExists(t, defaultConfigPath, "The generated config file is missing.") + + isManaged, err := isK0sManagedConfig(defaultConfigPath) + require.NoError(t, err) + require.True(t, isManaged, "The generated config file should qualify as k0s-managed, but doesn't.") + }) + + t.Run("should return false if file has no marker", func(t *testing.T) { + unmanagedPath := filepath.Join(t.TempDir(), "unmanaged.toml") + require.NoError(t, os.WriteFile(unmanagedPath, []byte(" # k0s_managed=true"), 0644)) + + isManaged, err := isK0sManagedConfig(unmanagedPath) + require.NoError(t, err) + require.False(t, isManaged) + }) + + t.Run("should return true for pre-1.30 generated config", func(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "containerd.toml") + cfg := ` +# k0s_managed=true +# This is a placeholder configuration for k0s managed containerD. +# If you wish to override the config, remove the first line and replace this file with your custom configuration. +# For reference see https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md +version = 2 +imports = [ + "/run/k0s/containerd-cri.toml", +] +` + err := os.WriteFile(configPath, []byte(cfg), 0644) + require.NoError(t, err) + isManaged, err := isK0sManagedConfig(configPath) require.NoError(t, err) require.True(t, isManaged) })