Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Ship env vars as one-time secret" #8082

Merged
merged 3 commits into from
Feb 8, 2022
Merged

Revert "Ship env vars as one-time secret" #8082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a4931d
Revert "[server] Keep pushing env vars until supervisor is up to date"
AlexTugarev Feb 8, 2022
9a591f7
Revert "[server] Use envvar OTS to ship user/prj env vars"
AlexTugarev Feb 8, 2022
dcd11b1
Revert "[supervisor] Support envvars from OTS"
AlexTugarev Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 5 additions & 24 deletions components/server/src/workspace/workspace-starter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,31 +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);

// 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 envvars = allEnvVars.map(uv => {
const ev = new EnvironmentVariable();
ev.setName(e.name);
ev.setValue(e.name);
envvars.push(ev);
})
ev.setName(uv.name);
ev.setValue(uv.value);
return ev;
});

const ideAlias = user.additionalData?.ideSettings?.defaultIde;
if (ideAlias && ideConfig.ideOptions.options[ideAlias]) {
Expand Down
6 changes: 0 additions & 6 deletions components/supervisor/pkg/supervisor/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 11 additions & 12 deletions components/supervisor/pkg/supervisor/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
79 changes: 17 additions & 62 deletions components/supervisor/pkg/supervisor/supervisor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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"},
Expand All @@ -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()
Expand Down Expand Up @@ -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")

Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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()
Expand All @@ -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).
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
}
Expand Down
27 changes: 1 addition & 26 deletions components/supervisor/pkg/supervisor/supervisor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package supervisor

import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"sort"
"testing"
Expand All @@ -28,7 +26,6 @@ func TestBuildChildProcEnv(t *testing.T) {
Name string
Input []string
Expectation []string
OTS string
Assert func(t *testing.T, act []string)
}{
{
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
})
}
Expand Down