From 0a4931dee52288cd67fee3f90d8d1cf63954f3bf Mon Sep 17 00:00:00 2001 From: Alex Tugarev Date: Tue, 8 Feb 2022 10:20:38 +0100 Subject: [PATCH 1/3] Revert "[server] Keep pushing env vars until supervisor is up to date" This reverts commit e94cb93ae70821dd7b1ec9fb6fa34df694de2def. --- components/server/src/workspace/workspace-starter.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index 82b896216a91cb..135b96a73b7ecf 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -697,15 +697,6 @@ export class WorkspaceStarter { ev.setValue(envvarOTS.token); envvars.push(ev); - // TODO(cw): for the time being we're still pushing the env vars as we did before. - // Once everything is running with the latest supervisor, we can stop doing that. - allEnvVars.forEach(e => { - const ev = new EnvironmentVariable(); - ev.setName(e.name); - ev.setValue(e.name); - envvars.push(ev); - }) - const ideAlias = user.additionalData?.ideSettings?.defaultIde; if (ideAlias && ideConfig.ideOptions.options[ideAlias]) { const ideAliasEnv = new EnvironmentVariable(); From 9a591f7c3e98ca7ea97ad9a8544fd985b0755955 Mon Sep 17 00:00:00 2001 From: Alex Tugarev Date: Tue, 8 Feb 2022 10:20:38 +0100 Subject: [PATCH 2/3] Revert "[server] Use envvar OTS to ship user/prj env vars" This reverts commit d8847a9c12e0a29a1d801bfa48dec5dd8452e317. --- .../server/src/workspace/workspace-starter.ts | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index 135b96a73b7ecf..cbc99bdf34cfc3 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -680,22 +680,12 @@ export class WorkspaceStarter { if (WithEnvvarsContext.is(context)) { allEnvVars = allEnvVars.concat(context.envvars); } - - // we copy the envvars to a stable format so that things don't break when someone changes the - // EnvVarWithValue shape. The JSON.stringify(envvars) will be consumed by supervisor and we - // need to make sure we're speaking the same language. - const stableEnvvars = allEnvVars.map(e => { return { name: e.name, value: e.value }}); - - // we ship the user-specific env vars as OTS because they might contain secrets - const envvarOTSExpirationTime = new Date(); - envvarOTSExpirationTime.setMinutes(envvarOTSExpirationTime.getMinutes() + 30); - const envvarOTS = await this.otsServer.serve(traceCtx, JSON.stringify(stableEnvvars), envvarOTSExpirationTime); - - const envvars: EnvironmentVariable[] = []; - const ev = new EnvironmentVariable(); - ev.setName("SUPERVISOR_ENVVAR_OTS"); - ev.setValue(envvarOTS.token); - envvars.push(ev); + const envvars = allEnvVars.map(uv => { + const ev = new EnvironmentVariable(); + ev.setName(uv.name); + ev.setValue(uv.value); + return ev; + }); const ideAlias = user.additionalData?.ideSettings?.defaultIde; if (ideAlias && ideConfig.ideOptions.options[ideAlias]) { From dcd11b1404ca4d9278b76d64ee673ece5ad54413 Mon Sep 17 00:00:00 2001 From: Alex Tugarev Date: Tue, 8 Feb 2022 10:20:38 +0100 Subject: [PATCH 3/3] Revert "[supervisor] Support envvars from OTS" This reverts commit 438d87876fa3429dd59be47db24a868076e545bd. --- .../supervisor/pkg/supervisor/config.go | 6 -- components/supervisor/pkg/supervisor/ssh.go | 23 +++--- .../supervisor/pkg/supervisor/supervisor.go | 79 ++++--------------- .../pkg/supervisor/supervisor_test.go | 27 +------ 4 files changed, 29 insertions(+), 106 deletions(-) diff --git a/components/supervisor/pkg/supervisor/config.go b/components/supervisor/pkg/supervisor/config.go index 03fecc6cc37f97..00d9a8d00d40d8 100644 --- a/components/supervisor/pkg/supervisor/config.go +++ b/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. diff --git a/components/supervisor/pkg/supervisor/ssh.go b/components/supervisor/pkg/supervisor/ssh.go index 65b74e5c3fef2b..01a177566be9cd 100644 --- a/components/supervisor/pkg/supervisor/ssh.go +++ b/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) } diff --git a/components/supervisor/pkg/supervisor/supervisor.go b/components/supervisor/pkg/supervisor/supervisor.go index c7b6efcd0f747f..6d162d958aa88b 100644 --- a/components/supervisor/pkg/supervisor/supervisor.go +++ b/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,7 +1179,7 @@ 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() { @@ -1232,7 +1187,7 @@ func startSSHServer(ctx context.Context, cfg *Config, wg *sync.WaitGroup, childP } go func() { - ssh, err := newSSHServer(ctx, cfg, childProcEnvvars) + ssh, err := newSSHServer(ctx, cfg) if err != nil { log.WithError(err).Error("err starting SSH server") } diff --git a/components/supervisor/pkg/supervisor/supervisor_test.go b/components/supervisor/pkg/supervisor/supervisor_test.go index 9f24904d0b4a2f..b52f15ae3b958b 100644 --- a/components/supervisor/pkg/supervisor/supervisor_test.go +++ b/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) }) }