From 1e3234be8769b93386a392d9110fdb70fddd7723 Mon Sep 17 00:00:00 2001
From: Claudia Beresford <claudiaberesford@gmail.com>
Date: Tue, 20 Apr 2021 12:47:42 +0100
Subject: [PATCH] Load env from host into exec

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 |  7 +++++-
 pkg/executor/executor.go   |  1 +
 pkg/flux/flux.go           | 45 ++++++++++++++------------------------
 pkg/flux/flux_test.go      | 17 +++++++++++++-
 4 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/pkg/actions/flux/enable.go b/pkg/actions/flux/enable.go
index 8ef3a20b49..83cbf3e7bc 100644
--- a/pkg/actions/flux/enable.go
+++ b/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
diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go
index 7a7fec1090..1945bda28d 100644
--- a/pkg/executor/executor.go
+++ b/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))
 	}
diff --git a/pkg/flux/flux.go b/pkg/flux/flux.go
index 163f50f72e..14cef7a342 100644
--- a/pkg/flux/flux.go
+++ b/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
 }
diff --git a/pkg/flux/flux_test.go b/pkg/flux/flux_test.go
index fe3083a665..1a6236a02b 100644
--- a/pkg/flux/flux_test.go
+++ b/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")))
+			})
+		})
+	})
 })