Skip to content

Commit

Permalink
Escape systemd special chars in —docker-env
Browse files Browse the repository at this point in the history
  • Loading branch information
11janci committed Mar 28, 2019
1 parent 9b8e3ed commit bce4bf3
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 6 deletions.
20 changes: 20 additions & 0 deletions pkg/provision/buildroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"path"
"path/filepath"
"strings"
"text/template"
"time"

Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
40 changes: 40 additions & 0 deletions pkg/util/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
27 changes: 21 additions & 6 deletions test/integration/start_stop_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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")
}
}

0 comments on commit bce4bf3

Please sign in to comment.