From 1f285bdee8d57550b6e3d3f4d83c7d77b9e359f5 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 12 Mar 2024 13:31:45 +0100 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 | 25 +++++++-------- .../worker/containerd/component_test.go | 32 ++++++++++++++++++- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/pkg/component/worker/containerd/component.go b/pkg/component/worker/containerd/component.go index 9d67afe71fbc..88305fed4c73 100644 --- a/pkg/component/worker/containerd/component.go +++ b/pkg/component/worker/containerd/component.go @@ -22,12 +22,12 @@ import ( "context" "crypto/md5" "encoding/hex" + "errors" "fmt" "io" "os" "path/filepath" "runtime" - "strings" "syscall" "time" @@ -47,8 +47,6 @@ import ( "github.com/k0sproject/k0s/pkg/supervisor" ) -const magicMarker = "# k0s_managed=true" - const confTmpl = ` # k0s_managed=true # This is a placeholder configuration for k0s managed containerd. @@ -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,12 @@ 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) { - return true, nil - } + if scanner.Scan() { + return scanner.Text() == "# k0s_managed=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..7f803404c8fc 100644 --- a/pkg/component/worker/containerd/component_test.go +++ b/pkg/component/worker/containerd/component_test.go @@ -21,17 +21,47 @@ import ( "path/filepath" "testing" + "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" + workerconfig "github.com/k0sproject/k0s/pkg/component/worker/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{ + confPath:/* this is where the config file is written to: */ 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 if md5 matches with pre 1.27 default config", func(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "containerd.toml")