Fix InstallFlags change detection with kubelet-extra-args

Signed-off-by: Kimmo Lehto <[email protected]>

internal/shell/split.go

@@ -0,0 +1,62 @@
+package shell
+
+// this is borrowed as-is from rig v2 until k0sctl is updated to use it
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Split splits the input string respecting shell-like quoted segments.
+func Split(input string) ([]string, error) { //nolint:cyclop
+	var segments []string
+
+	currentSegment, ok := builderPool.Get().(*strings.Builder)
+	if !ok {
+		currentSegment = &strings.Builder{}
+	}
+	defer builderPool.Put(currentSegment)
+	defer currentSegment.Reset()
+
+	var inDoubleQuotes, inSingleQuotes, isEscaped bool
+
+	for i := range len(input) {
+		currentChar := input[i]
+
+		if isEscaped {
+			currentSegment.WriteByte(currentChar)
+			isEscaped = false
+			continue
+		}
+
+		switch {
+		case currentChar == '\\' && !inSingleQuotes:
+			isEscaped = true
+		case currentChar == '"' && !inSingleQuotes:
+			inDoubleQuotes = !inDoubleQuotes
+		case currentChar == '\'' && !inDoubleQuotes:
+			inSingleQuotes = !inSingleQuotes
+		case currentChar == ' ' && !inDoubleQuotes && !inSingleQuotes:
+			// Space outside quotes; delimiter for a new segment
+			segments = append(segments, currentSegment.String())
+			currentSegment.Reset()
+		default:
+			currentSegment.WriteByte(currentChar)
+		}
+	}
+
+	if inDoubleQuotes || inSingleQuotes {
+		return nil, fmt.Errorf("split `%q`: %w", input, ErrMismatchedQuotes)
+	}
+
+	if isEscaped {
+		return nil, fmt.Errorf("split `%q`: %w", input, ErrTrailingBackslash)
+	}
+
+	// Add the last segment if present
+	if currentSegment.Len() > 0 {
+		segments = append(segments, currentSegment.String())
+	}
+
+	return segments, nil
+}

internal/shell/unquote.go

@@ -0,0 +1,80 @@
+package shell
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"sync"
+)
+
+// This is borrowed from rig v2 until k0sctl is updated to use it
+
+var (
+	builderPool = sync.Pool{
+		New: func() interface{} {
+			return &strings.Builder{}
+		},
+	}
+
+	// ErrMismatchedQuotes is returned when the input string has mismatched quotes when unquoting.
+	ErrMismatchedQuotes = errors.New("mismatched quotes")
+
+	// ErrTrailingBackslash is returned when the input string ends with a trailing backslash.
+	ErrTrailingBackslash = errors.New("trailing backslash")
+)
+
+// Unquote is a mostly POSIX compliant implementation of unquoting a string the same way a shell would.
+// Variables and command substitutions are not handled.
+func Unquote(input string) (string, error) { //nolint:cyclop
+	sb, ok := builderPool.Get().(*strings.Builder)
+	if !ok {
+		sb = &strings.Builder{}
+	}
+	defer builderPool.Put(sb)
+	defer sb.Reset()
+
+	var inDoubleQuotes, inSingleQuotes, isEscaped bool
+
+	for i := range len(input) {
+		currentChar := input[i]
+
+		if isEscaped {
+			sb.WriteByte(currentChar)
+			isEscaped = false
+			continue
+		}
+
+		switch currentChar {
+		case '\\':
+			if !inSingleQuotes { // Escape works in double quotes or outside any quotes
+				isEscaped = true
+			} else {
+				sb.WriteByte(currentChar) // Treat as a regular character within single quotes
+			}
+		case '"':
+			if !inSingleQuotes { // Toggle double quotes only if not in single quotes
+				inDoubleQuotes = !inDoubleQuotes
+			} else {
+				sb.WriteByte(currentChar) // Treat as a regular character within single quotes
+			}
+		case '\'':
+			if !inDoubleQuotes { // Toggle single quotes only if not in double quotes
+				inSingleQuotes = !inSingleQuotes
+			} else {
+				sb.WriteByte(currentChar) // Treat as a regular character within double quotes
+			}
+		default:
+			sb.WriteByte(currentChar)
+		}
+	}
+
+	if inDoubleQuotes || inSingleQuotes {
+		return "", fmt.Errorf("unquote `%q`: %w", input, ErrMismatchedQuotes)
+	}
+
+	if isEscaped {
+		return "", fmt.Errorf("unquote `%q`: %w", input, ErrTrailingBackslash)
+	}
+
+	return sb.String(), nil
+}

phase/gather_k0s_facts.go

@@ -7,7 +7,6 @@ import (
 	"net"
 	"net/url"
 	"path"
-	"regexp"
 	"strings"
 
 	"github.com/k0sproject/dig"
@@ -183,8 +182,6 @@ func (p *GatherK0sFacts) listEtcdMembers(h *cluster.Host) error {
 	return nil
 }
 
-var escapedSpaceRe = regexp.MustCompile(`\\x20`)
-
 func (p *GatherK0sFacts) investigateK0s(h *cluster.Host) error {
 	output, err := h.ExecOutput(h.Configurer.K0sCmdf("version"), exec.Sudo(h))
 	if err != nil {
@@ -279,14 +276,10 @@ func (p *GatherK0sFacts) investigateK0s(h *cluster.Host) error {
 	if len(status.Args) > 2 {
 		// status.Args contains the binary path and the role as the first two elements, which we can ignore here.
 		for _, a := range status.Args[2:] {
-			parts := strings.SplitN(a, "=", 2)
-			key, value := parts[0], parts[1]
-			// status output has escaped spaces, replace them with real spaces
-			escapedSpaceRe.ReplaceAllString(value, " ")
-			args.Add(fmt.Sprintf("%s=%s", key, value))
+			args.Add(a)
 		}
 	}
-	h.Metadata.K0sStatusArgs = []string(args)
+	h.Metadata.K0sStatusArgs = args
 
 	log.Infof("%s: is running k0s %s version %s", h, h.Role, h.Metadata.K0sRunningVersion)
 	if h.IsController() {

phase/reinstall.go

@@ -77,7 +77,7 @@ func (p *Reinstall) reinstall(h *cluster.Host) error {
 		h.InstallFlags.AddOrReplace("--enable-dynamic-config")
 	}
 
-	h.InstallFlags.AddOrReplace("--force")
+	h.InstallFlags.AddOrReplace("--force=true")
 
 	cmd, err := h.K0sInstallCommand()
 	if err != nil {

pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/flags.go

@@ -1,39 +1,55 @@
 package cluster
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
+
+	"github.com/alessio/shellescape"
+	"github.com/k0sproject/k0sctl/internal/shell"
 )
 
 // Flags is a slice of strings with added functions to ease manipulating lists of command-line flags
 type Flags []string
 
 // Add adds a flag regardless if it exists already or not
 func (f *Flags) Add(s string) {
+	if ns, err := shell.Unquote(s); err == nil {
+		s = ns
+	}
 	*f = append(*f, s)
 }
 
 // Add a flag with a value
 func (f *Flags) AddWithValue(key, value string) {
-	*f = append(*f, key+" "+value)
+	if nv, err := shell.Unquote(value); err == nil {
+		value = nv
+	}
+	*f = append(*f, key+"="+value)
 }
 
 // AddUnlessExist adds a flag unless one with the same prefix exists
 func (f *Flags) AddUnlessExist(s string) {
+	if ns, err := shell.Unquote(s); err == nil {
+		s = ns
+	}
 	if f.Include(s) {
 		return
 	}
-	*f = append(*f, s)
+	f.Add(s)
 }
 
 // AddOrReplace replaces a flag with the same prefix or adds a new one if one does not exist
 func (f *Flags) AddOrReplace(s string) {
+	if ns, err := shell.Unquote(s); err == nil {
+		s = ns
+	}
 	idx := f.Index(s)
 	if idx > -1 {
 		(*f)[idx] = s
 		return
 	}
-	*f = append(*f, s)
+	f.Add(s)
 }
 
 // Include returns true if a flag with a matching prefix can be found
@@ -43,6 +59,9 @@ func (f Flags) Include(s string) bool {
 
 // Index returns an index to a flag with a matching prefix
 func (f Flags) Index(s string) int {
+	if ns, err := shell.Unquote(s); err == nil {
+		s = ns
+	}
 	var flag string
 	sepidx := strings.IndexAny(s, "= ")
 	if sepidx < 0 {
@@ -73,17 +92,16 @@ func (f Flags) GetValue(s string) string {
 	if fl == "" {
 		return ""
 	}
+	if nfl, err := shell.Unquote(fl); err == nil {
+		fl = nfl
+	}
 
 	idx := strings.IndexAny(fl, "= ")
 	if idx < 0 {
 		return ""
 	}
 
 	val := fl[idx+1:]
-	s, err := strconv.Unquote(val)
-	if err == nil {
-		return s
-	}
 
 	return val
 }
@@ -137,5 +155,49 @@ func (f *Flags) MergeAdd(b Flags) {
 
 // Join creates a string separated by spaces
 func (f *Flags) Join() string {
-	return strings.Join(*f, " ")
+	parts := make([]string, len(*f))
+	f.Each(func(k, v string) {
+		parts = append(parts, fmt.Sprintf("%s=%s", k, shellescape.Quote(v)))
+	})
+	return strings.Join(parts, " ")
+}
+
+// Each iterates over each flag and calls the function with the flag key and value as arguments
+func (f Flags) Each(fn func(string, string)) {
+	for _, flag := range f {
+		sepidx := strings.IndexAny(flag, "= ")
+		if sepidx < 0 {
+			fn(flag, "")
+		} else {
+			key, value := flag[:sepidx], flag[sepidx+1:]
+			if unq, err := shell.Unquote(value); err == nil {
+				value = unq
+			}
+			fn(key, value)
+		}
+	}
+}
+
+// Map returns a map[string]string of the flags where the key is the flag and the value is the value
+func (f Flags) Map() map[string]string {
+	res := make(map[string]string)
+	f.Each(func(k, v string) {
+		res[k] = v
+	})
+	return res
+}
+
+// Equals compares the flags with another Flags and returns true if they have the same flags and values, ignoring order
+func (f Flags) Equals(b Flags) bool {
+	if len(f) != len(b) {
+		return false
+	}
+	for _, flag := range f {
+		ourValue := f.GetValue(flag)
+		theirValue := b.GetValue(flag)
+		if ourValue != theirValue {
+			return false
+		}
+	}
+	return true
 }

pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/flags_test.go

@@ -99,6 +99,39 @@ func TestGetBoolean(t *testing.T) {
 		result, err := flags.GetBoolean("--flag3")
 		require.NoError(t, err)
 		require.Equal(t, result, false)
+	})
+}
 
+func TestEach(t *testing.T) {
+	flags := Flags{"--flag1", "--flag2=foo", "--flag3=bar"}
+	var countF, countV int
+	flags.Each(func(flag string, value string) {
+		countF++
+		if value != "" {
+			countV++
+		}
 	})
+	require.Equal(t, 3, countF)
+	require.Equal(t, 2, countV)
+}
+
+func TestMap(t *testing.T) {
+	flags := Flags{"--flag1", "--flag2=foo", "--flag3=bar"}
+	m := flags.Map()
+	require.Len(t, m, 3)
+	require.Equal(t, "", m["--flag1"])
+	require.Equal(t, "foo", m["--flag2"])
+	require.Equal(t, "bar", m["--flag3"])
+}
+
+func TestEquals(t *testing.T) {
+	flags1 := Flags{"--flag1", "--flag2=foo", "--flag3=bar"}
+	flags2 := Flags{"--flag1", "--flag2=foo", "--flag3=bar"}
+	require.True(t, flags1.Equals(flags2))
+
+	flags2 = Flags{"--flag1", "--flag2=foo"}
+	require.False(t, flags1.Equals(flags2))
+
+	flags2 = Flags{"--flag1", "--flag2=foo", "--flag3=baz"}
+	require.False(t, flags1.Equals(flags2))
 }