From 60422c8090376d474a948db8e390714aead12b8f Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 4 Aug 2020 10:48:59 -0700 Subject: [PATCH 1/5] Ignore TMPDIR when run container --- kyaml/fn/runtime/container/container.go | 14 +++++++++++++- kyaml/fn/runtime/container/container_test.go | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index 9b4d43876..590232d11 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -195,7 +195,7 @@ func (c *Filter) getCommand() (string, []string) { // export the local environment vars to the container for _, pair := range os.Environ() { items := strings.Split(pair, "=") - if items[0] == "" || items[1] == "" { + if items[0] == "" || items[1] == "" || shouldEnvIgnored(items[0]) { continue } args = append(args, "-e", items[0]) @@ -203,3 +203,15 @@ func (c *Filter) getCommand() (string, []string) { a := append(args, c.Image) return "docker", a } + +// shouldEnvIgnored returns true if the environment variable key should be ignored +// by the container runtime. +func shouldEnvIgnored(envKey string) bool { + ignoreEnvKey := []string{"TMPDIR"} + for _, k := range ignoreEnvKey { + if k == envKey { + return true + } + } + return false +} diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index 2e054f945..1c465c7d2 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -210,3 +210,20 @@ func TestFilter_ExitCode(t *testing.T) { t.FailNow() } } + +func TestIgnoreEnv(t *testing.T) { + ignoredEnvKey := []string{"TMPDIR"} + for _, key := range ignoredEnvKey { + os.Setenv(key, "") + } + + fltr := Filter{Image: "example.com:version"} + _, args := fltr.getCommand() + for _, arg := range args { + for _, key := range ignoredEnvKey { + if arg == key { + t.Fatalf("%s should not be exported to container", key) + } + } + } +} From c99bc47c8d039b28e2c3080bf64cad8bca0e2538 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 4 Aug 2020 11:01:06 -0700 Subject: [PATCH 2/5] fix test on macos --- kyaml/fn/runtime/container/container_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index 1c465c7d2..f7a071ad6 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -106,7 +106,7 @@ metadata: for _, e := range os.Environ() { // the process env parts := strings.Split(e, "=") - if parts[0] == "" || parts[1] == "" { + if parts[0] == "" || parts[1] == "" || shouldEnvIgnored(parts[0]) { continue } tt.expectedArgs = append(tt.expectedArgs, "-e", parts[0]) From 2f7241f4c33cff35ff5515bd00dc8f523769256a Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 4 Aug 2020 17:11:18 -0700 Subject: [PATCH 3/5] code review --- kyaml/fn/runtime/container/container.go | 3 ++- kyaml/fn/runtime/container/container_test.go | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index 590232d11..806d36702 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -204,10 +204,11 @@ func (c *Filter) getCommand() (string, []string) { return "docker", a } +var ignoreEnvKey []string = []string{"TMPDIR"} + // shouldEnvIgnored returns true if the environment variable key should be ignored // by the container runtime. func shouldEnvIgnored(envKey string) bool { - ignoreEnvKey := []string{"TMPDIR"} for _, k := range ignoreEnvKey { if k == envKey { return true diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index f7a071ad6..c5907576b 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -212,15 +212,14 @@ func TestFilter_ExitCode(t *testing.T) { } func TestIgnoreEnv(t *testing.T) { - ignoredEnvKey := []string{"TMPDIR"} - for _, key := range ignoredEnvKey { + for _, key := range ignoreEnvKey { os.Setenv(key, "") } fltr := Filter{Image: "example.com:version"} _, args := fltr.getCommand() for _, arg := range args { - for _, key := range ignoredEnvKey { + for _, key := range ignoreEnvKey { if arg == key { t.Fatalf("%s should not be exported to container", key) } From 8cdc97a0ddcf8f8bb0266d6b777325b39ef6511b Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 5 Aug 2020 11:56:19 -0700 Subject: [PATCH 4/5] code review --- kyaml/fn/runtime/container/container.go | 17 +++-------------- kyaml/fn/runtime/container/container_test.go | 12 ++++-------- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index 806d36702..a561a9bf8 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -189,13 +189,15 @@ func (c *Filter) getCommand() (string, []string) { args = append(args, "--mount", storageMount.String()) } + // TODO: put these env processes into a separate function and call it in the outside of + // getCommand os.Setenv("LOG_TO_STDERR", "true") os.Setenv("STRUCTURED_RESULTS", "true") // export the local environment vars to the container for _, pair := range os.Environ() { items := strings.Split(pair, "=") - if items[0] == "" || items[1] == "" || shouldEnvIgnored(items[0]) { + if items[0] == "" || items[1] == "" || items[0] == "TMPDIR" { continue } args = append(args, "-e", items[0]) @@ -203,16 +205,3 @@ func (c *Filter) getCommand() (string, []string) { a := append(args, c.Image) return "docker", a } - -var ignoreEnvKey []string = []string{"TMPDIR"} - -// shouldEnvIgnored returns true if the environment variable key should be ignored -// by the container runtime. -func shouldEnvIgnored(envKey string) bool { - for _, k := range ignoreEnvKey { - if k == envKey { - return true - } - } - return false -} diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index c5907576b..df4375db2 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -106,7 +106,7 @@ metadata: for _, e := range os.Environ() { // the process env parts := strings.Split(e, "=") - if parts[0] == "" || parts[1] == "" || shouldEnvIgnored(parts[0]) { + if parts[0] == "" || parts[1] == "" || parts[0] == "TMPDIR" { continue } tt.expectedArgs = append(tt.expectedArgs, "-e", parts[0]) @@ -212,17 +212,13 @@ func TestFilter_ExitCode(t *testing.T) { } func TestIgnoreEnv(t *testing.T) { - for _, key := range ignoreEnvKey { - os.Setenv(key, "") - } + os.Setenv("TMPDIR", "") fltr := Filter{Image: "example.com:version"} _, args := fltr.getCommand() for _, arg := range args { - for _, key := range ignoreEnvKey { - if arg == key { - t.Fatalf("%s should not be exported to container", key) - } + if arg == "TMPDIR" { + t.Fatalf("TMPDIR should not be exported to container") } } } From 8cd7c13faddf92926e29ce485db1c1fc275ed272 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 5 Aug 2020 12:04:46 -0700 Subject: [PATCH 5/5] fix linter issue --- kyaml/fn/runtime/container/container.go | 4 +++- kyaml/fn/runtime/container/container_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index a561a9bf8..209aba93d 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -162,6 +162,8 @@ func (c *Filter) setupExec() { c.Exec.Args = args } +var tmpDirEnvKey string = "TMPDIR" + // getArgs returns the command + args to run to spawn the container func (c *Filter) getCommand() (string, []string) { // run the container using docker. this is simpler than using the docker @@ -197,7 +199,7 @@ func (c *Filter) getCommand() (string, []string) { // export the local environment vars to the container for _, pair := range os.Environ() { items := strings.Split(pair, "=") - if items[0] == "" || items[1] == "" || items[0] == "TMPDIR" { + if items[0] == "" || items[1] == "" || items[0] == tmpDirEnvKey { continue } args = append(args, "-e", items[0]) diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index df4375db2..fdf037fab 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -106,7 +106,7 @@ metadata: for _, e := range os.Environ() { // the process env parts := strings.Split(e, "=") - if parts[0] == "" || parts[1] == "" || parts[0] == "TMPDIR" { + if parts[0] == "" || parts[1] == "" || parts[0] == tmpDirEnvKey { continue } tt.expectedArgs = append(tt.expectedArgs, "-e", parts[0]) @@ -212,13 +212,13 @@ func TestFilter_ExitCode(t *testing.T) { } func TestIgnoreEnv(t *testing.T) { - os.Setenv("TMPDIR", "") + os.Setenv(tmpDirEnvKey, "") fltr := Filter{Image: "example.com:version"} _, args := fltr.getCommand() for _, arg := range args { - if arg == "TMPDIR" { - t.Fatalf("TMPDIR should not be exported to container") + if arg == tmpDirEnvKey { + t.Fatalf("%s should not be exported to container", tmpDirEnvKey) } } }