Escape systemd special chars in —docker-env

pkg/provision/buildroot.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"path"
 	"path/filepath"
+	"strings"
 	"text/template"
 	"time"
 
@@ -47,6 +48,9 @@ type BuildrootProvisioner struct {
 	provision.SystemdProvisioner
 }
 
+// for escaping systemd template specifiers (e.g. '%i'), which are not supported by minikube
+var systemdSpecifierEscaper = strings.NewReplacer("%", "%%")
+
 func init() {
 	provision.Register("Buildroot", &provision.RegisteredProvisioner{
 		New: NewBuildrootProvisioner,
@@ -64,6 +68,17 @@ func (p *BuildrootProvisioner) String() string {
 	return "buildroot"
 }
 
+// escapeSystemdDirectives escapes special characters in the input variables used to create the
+// systemd unit file, which would otherwise be interpreted as systemd directives. An example
+// are template specifiers (e.g. '%i') which are predefined variables that get evaluated dynamically
+// (see systemd man pages for more info). This is not supported by minikube, thus needs to be escaped.
+func escapeSystemdDirectives(engineConfigContext *provision.EngineConfigContext) {
+	// escape '%' in Environment option so that it does not evaluate into a template specifier
+	engineConfigContext.EngineOptions.Env = util.ReplaceChars(engineConfigContext.EngineOptions.Env, systemdSpecifierEscaper)
+	// input might contain whitespaces, wrap it in quotes
+	engineConfigContext.EngineOptions.Env = util.ConcatStrings(engineConfigContext.EngineOptions.Env, "\"", "\"")
+}
+
 // GenerateDockerOptions generates the *provision.DockerOptions for this provisioner
 func (p *BuildrootProvisioner) GenerateDockerOptions(dockerPort int) (*provision.DockerOptions, error) {
 	var engineCfg bytes.Buffer
@@ -127,6 +142,11 @@ WantedBy=multi-user.target
 		EngineOptions: p.EngineOptions,
 	}
 
+	escapeSystemdDirectives(&engineConfigContext)
+
+	fmt.Printf("======> %s", engineConfigContext.EngineOptions.Env[0])
+	fmt.Printf("======> %s", engineConfigContext.EngineOptions.Env[1])
+
 	if err := t.Execute(&engineCfg, engineConfigContext); err != nil {
 		return nil, err
 	}

pkg/util/utils.go

@@ -250,3 +250,26 @@ func TeePrefix(prefix string, r io.Reader, w io.Writer, logger func(format strin
 	}
 	return nil
 }
+
+// ReplaceChars returns a copy of the src slice with each string modified by the replacer
+func ReplaceChars(src []string, replacer *strings.Replacer) []string {
+	ret := make([]string, len(src))
+	for i, s := range src {
+		ret[i] = replacer.Replace(s)
+	}
+	return ret
+}
+
+// ConcatStrings concatenates each string in the src slice with prefix and postfix and returns a new slice
+func ConcatStrings(src []string, prefix string, postfix string) []string {
+	var buf bytes.Buffer
+	ret := make([]string, len(src))
+	for i, s := range src {
+		buf.WriteString(prefix)
+		buf.WriteString(s)
+		buf.WriteString(postfix)
+		ret[i] = buf.String()
+		buf.Reset()
+	}
+	return ret
+}

pkg/util/utils_test.go

@@ -197,3 +197,43 @@ func TestTeePrefix(t *testing.T) {
 		t.Errorf("log=%q, want: %q", gotLog, wantLog)
 	}
 }
+
+func TestReplaceChars(t *testing.T) {
+	testData := []struct {
+		src         []string
+		replacer    *strings.Replacer
+		expectedRes []string
+	}{
+		{[]string{"abc%def", "%Y%"}, strings.NewReplacer("%", "X"), []string{"abcXdef", "XYX"}},
+	}
+
+	for _, tt := range testData {
+		res := ReplaceChars(tt.src, tt.replacer)
+		for i, val := range res {
+			if val != tt.expectedRes[i] {
+				t.Fatalf("Expected '%s' but got '%s'", tt.expectedRes, res)
+			}
+		}
+	}
+}
+
+func TestConcatStrings(t *testing.T) {
+	testData := []struct {
+		src         []string
+		prefix      string
+		postfix     string
+		expectedRes []string
+	}{
+		{[]string{"abc", ""}, "xx", "yy", []string{"xxabcyy", "xxyy"}},
+		{[]string{"abc", ""}, "", "", []string{"abc", ""}},
+	}
+
+	for _, tt := range testData {
+		res := ConcatStrings(tt.src, tt.prefix, tt.postfix)
+		for i, val := range res {
+			if val != tt.expectedRes[i] {
+				t.Fatalf("Expected '%s' but got '%s'", tt.expectedRes, res)
+			}
+		}
+	}
+}

test/integration/start_stop_delete_test.go

@@ -19,42 +19,44 @@ limitations under the License.
 package integration
 
 import (
-	"fmt"
 	"net"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/docker/machine/libmachine/state"
-	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/test/integration/util"
 )
 
 func TestStartStop(t *testing.T) {
 	tests := []struct {
-		name string
-		args []string
+		name         string
+		args         []string
+		assertCustom func(t *testing.T, r *util.MinikubeRunner)
 	}{
 		{"nocache_oldest", []string{
 			"--cache-images=false",
 			fmt.Sprintf("--kubernetes-version=%s", constants.OldestKubernetesVersion),
-		}},
+		}, null},
 		{"feature_gates_newest_cni", []string{
 			"--feature-gates",
 			"ServerSideApply=true",
 			"--network-plugin=cni",
 			"--extra-config=kubelet.network-plugin=cni",
 			fmt.Sprintf("--kubernetes-version=%s", constants.NewestKubernetesVersion),
-		}},
+		}, null},
 		{"containerd", []string{
 			"--container-runtime=containerd",
 			"--docker-opt containerd=/var/run/containerd/containerd.sock",
-		}},
+		}, null},
 		{"crio_ignore_preflights", []string{
 			"--container-runtime=crio",
 			"--extra-config",
 			"kubeadm.ignore-preflight-errors=SystemVerification",
-		}},
+		}, null},
+		{"docker_env_special_char", []string{
+			"--docker-env TEST_VAR=with%40specifier TEST_VAR2=\"with space\"",
+		}, assertDockerEnv},
 	}
 
 	for _, test := range tests {
@@ -70,6 +72,10 @@ func TestStartStop(t *testing.T) {
 			r.Start(test.args...)
 			r.CheckStatus(state.Running.String())
 
+			if test.assertCustom != nil {
+				test.assertCustom(t, &r)
+			}
+
 			ip := r.RunCommand("ip", true)
 			ip = strings.TrimRight(ip, "\n")
 			if net.ParseIP(ip) == nil {
@@ -93,3 +99,10 @@ func TestStartStop(t *testing.T) {
 		})
 	}
 }
+
+func assertDockerEnv(t *testing.T, r *util.MinikubeRunner) {
+	out, _ := r.SSH("systemctl show --property=Environment docker")
+	if strings.Contains(out, "encoded%40space") {
+		t.Fatalf("Docker environment %s does not contain expected value %s", out, "encoded%40space")
+	}
+}