Skip to content

Commit

Permalink
[supervisor] Add buildChildProcEnv unit test
Browse files Browse the repository at this point in the history
  • Loading branch information
csweichel authored and roboquat committed Aug 8, 2021
1 parent b2627c6 commit a31e09f
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 6 deletions.
20 changes: 14 additions & 6 deletions components/supervisor/pkg/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func Run(options ...RunOption) {
go analyseConfigChanges(ctx, cfg, analytics, gitpodConfigService)

termMuxSrv.DefaultWorkdir = cfg.RepoRoot
termMuxSrv.Env = buildChildProcEnv(cfg)
termMuxSrv.Env = buildChildProcEnv(cfg, nil)
termMuxSrv.DefaultCreds = &syscall.Credential{
Uid: gitpodUID,
Gid: gitpodGID,
Expand Down Expand Up @@ -580,7 +580,7 @@ func prepareIDELaunch(cfg *Config) *exec.Cmd {
Gid: gitpodGID,
},
}
cmd.Env = buildChildProcEnv(cfg)
cmd.Env = buildChildProcEnv(cfg, nil)

// Here we must resist the temptation to "neaten up" the IDE output for headless builds.
// This would break the JSON parsing of the headless builds.
Expand All @@ -596,9 +596,15 @@ func prepareIDELaunch(cfg *Config) *exec.Cmd {
return cmd
}

func buildChildProcEnv(cfg *Config) []string {
// buildChildProcEnv computes the environment variables passed to a child process, based on the total list
// of envvars. If envvars is nil, os.Environ() is used.
func buildChildProcEnv(cfg *Config, envvars []string) []string {
if envvars == nil {
envvars = os.Environ()
}

envs := make(map[string]string)
for _, e := range os.Environ() {
for _, e := range envvars {
segs := strings.SplitN(e, "=", 2)
if len(segs) < 2 {
log.Printf("\"%s\" has invalid format, not including in IDE environment", e)
Expand Down Expand Up @@ -633,7 +639,9 @@ func buildChildProcEnv(cfg *Config) []string {
envs["USER"] = "gitpod"

// Particular Java optimisation: Java pre v10 did not gauge it's available memory correctly, and needed explicitly setting "-Xmx" for all Hotspot/openJDK VMs
envs["JAVA_TOOL_OPTIONS"] += fmt.Sprintf(" -Xmx%sm", os.Getenv("GITPOD_MEMORY"))
if mem, ok := envs["GITPOD_MEMORY"]; ok {
envs["JAVA_TOOL_OPTIONS"] += fmt.Sprintf(" -Xmx%sm", mem)
}

var env, envn []string
for nme, val := range envs {
Expand Down Expand Up @@ -926,7 +934,7 @@ func startSSHServer(ctx context.Context, cfg *Config, wg *sync.WaitGroup) {
}

cmd := exec.Command(dropbear, "-F", "-E", "-w", "-s", "-p", fmt.Sprintf(":%d", cfg.SSHPort), "-r", hostkeyFN.Name())
cmd.Env = buildChildProcEnv(cfg)
cmd.Env = buildChildProcEnv(cfg, nil)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err = cmd.Start()
Expand Down
128 changes: 128 additions & 0 deletions components/supervisor/pkg/supervisor/supervisor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Copyright (c) 2020 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package supervisor

import (
"fmt"
"os"
"sort"
"testing"

"github.com/google/go-cmp/cmp"
)

func TestBuildChildProcEnv(t *testing.T) {
withBaseline := func(i []string) []string {
return append(i,
"SUPERVISOR_ADDR=localhost:8080",
"HOME=/home/gitpod",
"USER=gitpod",
)
}

tests := []struct {
Name string
Input []string
Expectation []string
Assert func(t *testing.T, act []string)
}{
{
Name: "empty set",
Input: []string{},
Expectation: withBaseline(nil),
},
{
Name: "= in value",
Input: []string{"FOO=BAR=BAZ"},
Expectation: withBaseline([]string{"FOO=BAR=BAZ"}),
},
{
Name: "override baseline",
Input: []string{
"SUPERVISOR_ADDR=foobar",
"HOME=foobar",
"USER=foobar",
},
Expectation: withBaseline(nil),
},
{
Name: "removes blacklisted vars",
Input: []string{"GITPOD_TOKENS=foobar"},
Expectation: withBaseline(nil),
},
{
Name: "invalid env var",
Input: []string{"FOOBAR"},
Expectation: withBaseline(nil),
},
{
// for testing purposes we can pass a set of envvars to buildChildProcEnv.
// When a caller passes nil, buildChildProcEnv calls os.Environ().
// This test case ensures we make that call and don't break on behaviour introduced
// just for testing.
Name: "os.Environ",
Input: nil,
Assert: func(t *testing.T, act []string) {
// if we've called os.Environ, we expect PATH to be present
var (
hasPath bool
path = fmt.Sprintf("PATH=%s", os.Getenv("PATH"))
)
for _, e := range act {
if e == path {
hasPath = true
break
}
}
if !hasPath {
t.Errorf("no PATH envvar found - probably did not call os.Environ()")
}
},
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
assert := test.Assert
if assert == nil {
assert = func(t *testing.T, act []string) {
exp := test.Expectation
sort.Strings(act)
sort.Strings(exp)
if diff := cmp.Diff(exp, act); diff != "" {
t.Errorf("unexpected buildChildProcEnv (-want +got):\n%s", diff)
}
}
}

act := buildChildProcEnv(&Config{StaticConfig: StaticConfig{APIEndpointPort: 8080}}, test.Input)
assert(t, act)
})
}
}

func TestIsBlacklistedEnvvar(t *testing.T) {
tests := []struct {
Name string
Input string
Expectation bool
}{
{Name: "deprecated theia envvars", Input: "THEIA_SUPERVISOR_FOOBAR", Expectation: true},
{Name: "gitpod tokens", Input: "GITPOD_TOKENS", Expectation: true},
{Name: "gitpod tokens child", Input: "GITPOD_TOKENS_GITHUB", Expectation: true},
{Name: "kubernetes services", Input: "KUBERNETES_SERVICE_FOOBAR", Expectation: true},
{Name: "kubernetes service ports", Input: "KUBERNETES_PORT_FOOBAR", Expectation: true},
{Name: "something with spaces", Input: " I_DO_NOT_UNDERSTAND", Expectation: true},
{Name: "path", Input: "PATH", Expectation: false},
}
for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
act := isBlacklistedEnvvar(test.Input)
if diff := cmp.Diff(test.Expectation, act); diff != "" {
t.Errorf("unexpected isBlacklistedEnvvar (-want +got):\n%s", diff)
}
})
}
}

0 comments on commit a31e09f

Please sign in to comment.