Load env from host into exec (#3608)

Previously we loaded env vars as needed one by one, but it is easy to
missed things and as upstream needs change this can become tedious.

This commit also returns an error when the PAT token file cannot be
opened, rather than simply logging.

pkg/actions/flux/enable.go

@@ -33,10 +33,15 @@ func New(k8sClientSet kubeclient.Interface, opts *api.GitOps) (*Installer, error
 		return nil, errors.New("expected gitops.flux in cluster configuration but found nil")
 	}
 
+	fluxClient, err := flux.NewClient(opts.Flux)
+	if err != nil {
+		return nil, err
+	}
+
 	installer := &Installer{
 		opts:       opts.Flux,
 		kubeClient: k8sClientSet,
-		fluxClient: flux.NewClient(opts.Flux),
+		fluxClient: fluxClient,
 	}
 
 	return installer, nil

pkg/executor/executor.go

@@ -45,6 +45,7 @@ func (e ShellExecutor) ExecInDir(command string, dir string, args ...string) err
 func (e ShellExecutor) buildCmd(command string, args ...string) *exec.Cmd {
 	cmd := exec.Command(command, args...)
 
+	cmd.Env = os.Environ()
 	for k, v := range e.envVars {
 		cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", k, v))
 	}

pkg/flux/flux.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"os/exec"
 	"strings"
 
@@ -20,11 +19,16 @@ type Client struct {
 	opts     *api.Flux
 }
 
-func NewClient(opts *api.Flux) *Client {
+func NewClient(opts *api.Flux) (*Client, error) {
+	env, err := setTokenEnv(opts.AuthTokenPath, opts.GitProvider)
+	if err != nil {
+		return nil, err
+	}
+
 	return &Client{
-		executor: executor.NewShellExecutor(setEnvVars(opts.AuthTokenPath, opts.GitProvider)),
+		executor: executor.NewShellExecutor(env),
 		opts:     opts,
-	}
+	}, nil
 }
 
 func (c *Client) PreFlight() error {
@@ -69,40 +73,25 @@ func (c *Client) runFluxCmd(args ...string) error {
 	return c.executor.Exec(fluxBin, args...)
 }
 
-func setEnvVars(tokenPath, gitProvider string) executor.EnvVars {
-	envVars := executor.EnvVars{
-		"PATH":                  os.Getenv("PATH"),
-		"HOME":                  os.Getenv("HOME"),
-		"AWS_ACCESS_KEY_ID":     os.Getenv("AWS_ACCESS_KEY_ID"),
-		"AWS_SECRET_ACCESS_KEY": os.Getenv("AWS_SECRET_ACCESS_KEY"),
+func setTokenEnv(tokenPath, gitProvider string) (executor.EnvVars, error) {
+	if tokenPath == "" {
+		return executor.EnvVars{}, nil
 	}
 
 	var token string
-	if tokenPath != "" {
-		data, err := ioutil.ReadFile(tokenPath)
-		if err != nil {
-			logger.Warning("reading auth token file %s", err)
-		}
-
-		token = strings.Replace(string(data), "\n", "", -1)
+	data, err := ioutil.ReadFile(tokenPath)
+	if err != nil {
+		return executor.EnvVars{}, fmt.Errorf("reading auth token file %w", err)
 	}
 
+	token = strings.Replace(string(data), "\n", "", -1)
+	envVars := executor.EnvVars{}
 	switch gitProvider {
 	case "github":
-		if token == "" {
-			if githubToken, ok := os.LookupEnv("GITHUB_TOKEN"); ok {
-				token = githubToken
-			}
-		}
 		envVars["GITHUB_TOKEN"] = token
 	case "gitlab":
-		if token == "" {
-			if gitlabToken, ok := os.LookupEnv("GITLAB_TOKEN"); ok {
-				token = gitlabToken
-			}
-		}
 		envVars["GITLAB_TOKEN"] = token
 	}
 
-	return envVars
+	return envVars, nil
 }

pkg/flux/flux_test.go

@@ -28,7 +28,9 @@ var _ = Describe("Flux", func() {
 		}
 
 		fakeExecutor = new(fakes.FakeExecutor)
-		fluxClient = flux.NewClient(opts)
+		var err error
+		fluxClient, err = flux.NewClient(opts)
+		Expect(err).NotTo(HaveOccurred())
 		fluxClient.SetExecutor(fakeExecutor)
 
 		binDir, err := ioutil.TempDir("", "bin")
@@ -180,4 +182,17 @@ var _ = Describe("Flux", func() {
 			})
 		})
 	})
+
+	Context("setting token env", func() {
+		When("given PAT file does not exist", func() {
+			BeforeEach(func() {
+				opts.AuthTokenPath = "/road/to/nowhere"
+			})
+
+			It("creating the client should fail", func() {
+				_, err := flux.NewClient(opts)
+				Expect(err).To(MatchError(ContainSubstring("reading auth token file open /road/to/nowhere: no such file or directory")))
+			})
+		})
+	})
 })