Revert "[supervisor] Support envvars from OTS"

This reverts commit 438d878.

Author: AlexTugarev

components/supervisor/pkg/supervisor/config.go

@@ -240,12 +240,6 @@ type WorkspaceConfig struct {
 	// DotfileRepo is a user-configurable repository which contains their dotfiles to customise
 	// the in-workspace epxerience.
 	DotfileRepo string `env:"SUPERVISOR_DOTFILE_REPO"`
-
-	// EnvvarOTS points to a URL from which environment variables for child processes can be downloaded from.
-	// This provides a safer means to transport environment variables compared to shipping them on the Kubernetes pod.
-	//
-	// The format of the content downloaded from this URL is expected to be JSON in the form of [{"name":"name", "value":"value"}]
-	EnvvarOTS string `env:"SUPERVISOR_ENVVAR_OTS"`
 }
 
 // WorkspaceGitpodToken is a list of tokens that should be added to supervisor's token service.

components/supervisor/pkg/supervisor/ssh.go

@@ -20,7 +20,7 @@ import (
 	"github.com/gitpod-io/gitpod/common-go/process"
 )
 
-func newSSHServer(ctx context.Context, cfg *Config, envvars []string) (*sshServer, error) {
+func newSSHServer(ctx context.Context, cfg *Config) (*sshServer, error) {
 	bin, err := os.Executable()
 	if err != nil {
 		return nil, xerrors.Errorf("cannot find executable path: %w", err)
@@ -33,23 +33,21 @@ func newSSHServer(ctx context.Context, cfg *Config, envvars []string) (*sshServe
 			return nil, xerrors.Errorf("unexpected error creating SSH key: %w", err)
 		}
 	}
-	err = writeSSHEnv(cfg, envvars)
+	err = writeSSHEnv(cfg)
 	if err != nil {
 		return nil, xerrors.Errorf("unexpected error creating SSH env: %w", err)
 	}
 
 	return &sshServer{
-		ctx:     ctx,
-		cfg:     cfg,
-		sshkey:  sshkey,
-		envvars: envvars,
+		ctx:    ctx,
+		cfg:    cfg,
+		sshkey: sshkey,
 	}, nil
 }
 
 type sshServer struct {
-	ctx     context.Context
-	cfg     *Config
-	envvars []string
+	ctx context.Context
+	cfg *Config
 
 	sshkey string
 }
@@ -116,7 +114,7 @@ func (s *sshServer) handleConn(ctx context.Context, conn net.Conn) {
 	log.WithField("args", args).Debug("sshd flags")
 	cmd := exec.CommandContext(ctx, openssh, args...)
 	cmd = runAsGitpodUser(cmd)
-	cmd.Env = s.envvars
+	cmd.Env = buildChildProcEnv(s.cfg, nil)
 	cmd.ExtraFiles = []*os.File{socketFD}
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = bufio.NewReader(socketFD)
@@ -192,7 +190,7 @@ func prepareSSHKey(ctx context.Context, sshkey string) error {
 	return nil
 }
 
-func writeSSHEnv(cfg *Config, envvars []string) error {
+func writeSSHEnv(cfg *Config) error {
 	home, err := os.UserHomeDir()
 	if err != nil {
 		return err
@@ -205,7 +203,8 @@ func writeSSHEnv(cfg *Config, envvars []string) error {
 	}
 
 	fn := filepath.Join(d, "supervisor_env")
-	err = os.WriteFile(fn, []byte(strings.Join(envvars, "\n")), 0o644)
+	env := strings.Join(buildChildProcEnv(cfg, nil), "\n")
+	err = os.WriteFile(fn, []byte(env), 0o644)
 	if err != nil {
 		return xerrors.Errorf("cannot write %s: %w", fn, err)
 	}

components/supervisor/pkg/supervisor/supervisor.go

@@ -153,16 +153,12 @@ func Run(options ...RunOption) {
 		return
 	}
 
-	// BEWARE: we can only call buildChildProcEnv once, because it might download env vars from a one-time-secret
-	//         URL, which would fail if we tried another time.
-	childProcEnvvars := buildChildProcEnv(cfg, nil)
-
 	err = AddGitpodUserIfNotExists()
 	if err != nil {
 		log.WithError(err).Fatal("cannot ensure Gitpod user exists")
 	}
 	symlinkBinaries(cfg)
-	configureGit(cfg, childProcEnvvars)
+	configureGit(cfg)
 
 	tokenService := NewInMemoryTokenService()
 	tkns, err := cfg.GetTokens(true)
@@ -250,7 +246,7 @@ func Run(options ...RunOption) {
 			return ""
 		}
 	}
-	termMuxSrv.Env = childProcEnvvars
+	termMuxSrv.Env = buildChildProcEnv(cfg, nil)
 	termMuxSrv.DefaultCreds = &syscall.Credential{
 		Uid: gitpodUID,
 		Gid: gitpodGID,
@@ -287,15 +283,15 @@ func Run(options ...RunOption) {
 
 		// We need to checkout dotfiles first, because they may be changing the path which affects the IDE.
 		// TODO(cw): provide better feedback if the IDE start fails because of the dotfiles (provide any feedback at all).
-		installDotfiles(ctx, cfg, childProcEnvvars)
+		installDotfiles(ctx, termMuxSrv, cfg)
 	}
 
 	var ideWG sync.WaitGroup
 	ideWG.Add(1)
-	go startAndWatchIDE(ctx, cfg, &cfg.IDE, childProcEnvvars, &ideWG, ideReady, WebIDE)
+	go startAndWatchIDE(ctx, cfg, &cfg.IDE, &ideWG, ideReady, WebIDE)
 	if cfg.DesktopIDE != nil {
 		ideWG.Add(1)
-		go startAndWatchIDE(ctx, cfg, cfg.DesktopIDE, childProcEnvvars, &ideWG, desktopIdeReady, DesktopIDE)
+		go startAndWatchIDE(ctx, cfg, cfg.DesktopIDE, &ideWG, desktopIdeReady, DesktopIDE)
 	}
 
 	var wg sync.WaitGroup
@@ -304,7 +300,7 @@ func Run(options ...RunOption) {
 	wg.Add(1)
 	go startAPIEndpoint(ctx, cfg, &wg, apiServices, tunneledPortsService, apiEndpointOpts...)
 	wg.Add(1)
-	go startSSHServer(ctx, cfg, &wg, childProcEnvvars)
+	go startSSHServer(ctx, cfg, &wg)
 	wg.Add(1)
 	tasksSuccessChan := make(chan taskSuccess, 1)
 	go taskManager.Run(ctx, &wg, tasksSuccessChan)
@@ -340,7 +336,7 @@ func Run(options ...RunOption) {
 			}()
 
 			cmd := runAsGitpodUser(exec.Command("git", "fetch", "--unshallow", "--tags"))
-			cmd.Env = childProcEnvvars
+			cmd.Env = buildChildProcEnv(cfg, nil)
 			cmd.Dir = cfg.RepoRoot
 			cmd.Stdout = os.Stdout
 			cmd.Stderr = os.Stderr
@@ -374,7 +370,7 @@ func Run(options ...RunOption) {
 	wg.Wait()
 }
 
-func installDotfiles(ctx context.Context, cfg *Config, childProcEnvvars []string) {
+func installDotfiles(ctx context.Context, term *terminal.MuxTerminalService, cfg *Config) {
 	repo := cfg.DotfileRepo
 	if repo == "" {
 		return
@@ -389,7 +385,7 @@ func installDotfiles(ctx context.Context, cfg *Config, childProcEnvvars []string
 	prep := func(cfg *Config, out io.Writer, name string, args ...string) *exec.Cmd {
 		cmd := exec.Command(name, args...)
 		cmd.Dir = "/home/gitpod"
-		cmd.Env = childProcEnvvars
+		cmd.Env = buildChildProcEnv(cfg, nil)
 		cmd.SysProcAttr = &syscall.SysProcAttr{
 			// All supervisor children run as gitpod user. The environment variables we produce are also
 			// gitpod user specific.
@@ -592,7 +588,7 @@ func symlinkBinaries(cfg *Config) {
 	}
 }
 
-func configureGit(cfg *Config, childProcEnvvars []string) {
+func configureGit(cfg *Config) {
 	settings := [][]string{
 		{"push.default", "simple"},
 		{"alias.lg", "log --color --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit"},
@@ -608,7 +604,7 @@ func configureGit(cfg *Config, childProcEnvvars []string) {
 	for _, s := range settings {
 		cmd := exec.Command("git", append([]string{"config", "--global"}, s...)...)
 		cmd = runAsGitpodUser(cmd)
-		cmd.Env = childProcEnvvars
+		cmd.Env = buildChildProcEnv(cfg, nil)
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 		err := cmd.Run()
@@ -709,7 +705,7 @@ const (
 	statusShouldShutdown
 )
 
-func startAndWatchIDE(ctx context.Context, cfg *Config, ideConfig *IDEConfig, childProcEnvvars []string, wg *sync.WaitGroup, ideReady *ideReadyState, ide IDEKind) {
+func startAndWatchIDE(ctx context.Context, cfg *Config, ideConfig *IDEConfig, wg *sync.WaitGroup, ideReady *ideReadyState, ide IDEKind) {
 	defer wg.Done()
 	defer log.WithField("ide", ide.String()).Debug("startAndWatchIDE shutdown")
 
@@ -731,7 +727,7 @@ supervisorLoop:
 		}
 
 		ideStopped = make(chan struct{}, 1)
-		cmd = prepareIDELaunch(cfg, ideConfig, childProcEnvvars)
+		cmd = prepareIDELaunch(cfg, ideConfig)
 		launchIDE(cfg, ideConfig, cmd, ideStopped, ideReady, &ideStatus, ide)
 
 		select {
@@ -812,7 +808,7 @@ func launchIDE(cfg *Config, ideConfig *IDEConfig, cmd *exec.Cmd, ideStopped chan
 	}()
 }
 
-func prepareIDELaunch(cfg *Config, ideConfig *IDEConfig, childProcEnvvars []string) *exec.Cmd {
+func prepareIDELaunch(cfg *Config, ideConfig *IDEConfig) *exec.Cmd {
 	args := ideConfig.EntrypointArgs
 
 	// Add default args for IDE (not desktop IDE) to be backwards compatible
@@ -840,7 +836,7 @@ func prepareIDELaunch(cfg *Config, ideConfig *IDEConfig, childProcEnvvars []stri
 			Gid: gitpodGID,
 		},
 	}
-	cmd.Env = childProcEnvvars
+	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.
@@ -858,8 +854,6 @@ func prepareIDELaunch(cfg *Config, ideConfig *IDEConfig, childProcEnvvars []stri
 
 // 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.
-//
-// Beware: if config contains an OTS URL the results may differ on subsequent calls.
 func buildChildProcEnv(cfg *Config, envvars []string) []string {
 	if envvars == nil {
 		envvars = os.Environ()
@@ -882,20 +876,6 @@ func buildChildProcEnv(cfg *Config, envvars []string) []string {
 	}
 	envs["SUPERVISOR_ADDR"] = fmt.Sprintf("localhost:%d", cfg.APIEndpointPort)
 
-	if cfg.EnvvarOTS != "" {
-		es, err := downloadEnvvarOTS(cfg.EnvvarOTS)
-		if err != nil {
-			log.WithError(err).Warn("unable to download environment variables from OTS")
-		}
-		for k, v := range es {
-			if isBlacklistedEnvvar(k) {
-				continue
-			}
-
-			envs[k] = v
-		}
-	}
-
 	// We're forcing basic environment variables here, because supervisor acts like a login process at this point.
 	// The gitpod user might not have existed when supervisor was started, hence the HOME coming
 	// from the container runtime is probably wrong ("/" to be exact).
@@ -931,31 +911,6 @@ func buildChildProcEnv(cfg *Config, envvars []string) []string {
 	return env
 }
 
-func downloadEnvvarOTS(url string) (res map[string]string, err error) {
-	client := &http.Client{Timeout: 10 * time.Second}
-	resp, err := client.Get(url)
-	if err != nil {
-		return nil, err
-	}
-
-	defer resp.Body.Close()
-
-	var dl []struct {
-		Name  string `json:"name"`
-		Value string `json:"value"`
-	}
-	err = json.NewDecoder(resp.Body).Decode(&dl)
-	if err != nil {
-		return nil, err
-	}
-
-	res = make(map[string]string)
-	for _, e := range dl {
-		res[e.Name] = e.Value
-	}
-	return res, nil
-}
-
 func runIDEReadinessProbe(cfg *Config, ideConfig *IDEConfig, ide IDEKind) (desktopIDEStatus *DesktopIDEStatus) {
 	defer log.WithField("ide", ide.String()).Info("IDE is ready")
 
@@ -1224,15 +1179,15 @@ func stopWhenTasksAreDone(ctx context.Context, wg *sync.WaitGroup, shutdown chan
 	shutdown <- ShutdownReasonSuccess
 }
 
-func startSSHServer(ctx context.Context, cfg *Config, wg *sync.WaitGroup, childProcEnvvars []string) {
+func startSSHServer(ctx context.Context, cfg *Config, wg *sync.WaitGroup) {
 	defer wg.Done()
 
 	if cfg.isHeadless() {
 		return
 	}
 
 	go func() {
-		ssh, err := newSSHServer(ctx, cfg, childProcEnvvars)
+		ssh, err := newSSHServer(ctx, cfg)
 		if err != nil {
 			log.WithError(err).Error("err starting SSH server")
 		}

components/supervisor/pkg/supervisor/supervisor_test.go

@@ -6,8 +6,6 @@ package supervisor
 
 import (
 	"fmt"
-	"net/http"
-	"net/http/httptest"
 	"os"
 	"sort"
 	"testing"
@@ -28,7 +26,6 @@ func TestBuildChildProcEnv(t *testing.T) {
 		Name        string
 		Input       []string
 		Expectation []string
-		OTS         string
 		Assert      func(t *testing.T, act []string)
 	}{
 		{
@@ -84,18 +81,6 @@ func TestBuildChildProcEnv(t *testing.T) {
 				}
 			},
 		},
-		{
-			Name:        "ots",
-			Input:       []string{},
-			OTS:         `[{"name":"foo","value":"bar"},{"name":"GITPOD_TOKENS","value":"foobar"}]`,
-			Expectation: []string{"HOME=/home/gitpod", "SUPERVISOR_ADDR=localhost:8080", "USER=gitpod", "foo=bar"},
-		},
-		{
-			Name:        "failed ots",
-			Input:       []string{},
-			OTS:         `invalid json`,
-			Expectation: []string{"HOME=/home/gitpod", "SUPERVISOR_ADDR=localhost:8080", "USER=gitpod"},
-		},
 	}
 
 	for _, test := range tests {
@@ -112,17 +97,7 @@ func TestBuildChildProcEnv(t *testing.T) {
 				}
 			}
 
-			cfg := &Config{StaticConfig: StaticConfig{APIEndpointPort: 8080}}
-			if test.OTS != "" {
-				srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-					w.WriteHeader(http.StatusOK)
-					w.Header().Set("Content-Type", "application/json")
-					w.Write([]byte(test.OTS))
-				}))
-				cfg.EnvvarOTS = srv.URL
-			}
-
-			act := buildChildProcEnv(cfg, test.Input)
+			act := buildChildProcEnv(&Config{StaticConfig: StaticConfig{APIEndpointPort: 8080}}, test.Input)
 			assert(t, act)
 		})
 	}