From bce4bf3a9f165c864f55890b573f839187c5bcc2 Mon Sep 17 00:00:00 2001 From: Jan Janik <11janci@seznam.cz> Date: Wed, 27 Mar 2019 22:05:20 +1300 Subject: [PATCH] =?UTF-8?q?Escape=20systemd=20special=20chars=20in=20?= =?UTF-8?q?=E2=80=94docker-env?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/provision/buildroot.go | 20 +++++++++++ pkg/util/utils.go | 23 +++++++++++++ pkg/util/utils_test.go | 40 ++++++++++++++++++++++ test/integration/start_stop_delete_test.go | 27 +++++++++++---- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/pkg/provision/buildroot.go b/pkg/provision/buildroot.go index 50c701580da2..686e17c22c2f 100644 --- a/pkg/provision/buildroot.go +++ b/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 } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 7637e2a588b3..cf812512194b 100644 --- a/pkg/util/utils.go +++ b/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 +} diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index f7d1c7c372fe..864a5400955f 100644 --- a/pkg/util/utils_test.go +++ b/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) + } + } + } +} diff --git a/test/integration/start_stop_delete_test.go b/test/integration/start_stop_delete_test.go index 839283654d68..6570d19c11e6 100644 --- a/test/integration/start_stop_delete_test.go +++ b/test/integration/start_stop_delete_test.go @@ -32,29 +32,33 @@ import ( 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), - }}, + }, nil}, {"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), - }}, + }, nil}, {"containerd", []string{ "--container-runtime=containerd", "--docker-opt containerd=/var/run/containerd/containerd.sock", - }}, + }, nil}, {"crio_ignore_preflights", []string{ "--container-runtime=crio", "--extra-config", "kubeadm.ignore-preflight-errors=SystemVerification", - }}, + }, nil}, + {"docker_env_special_char", []string{ + "--docker-env TEST_VAR=with%40specifier TEST_VAR2=\"with space\"", + }, assertDockerEnv}, } for _, test := range tests { @@ -70,6 +74,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 +101,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") + } +}