From 088c5c07a77fe8a5d41a73347cfe0af404a9a99a Mon Sep 17 00:00:00 2001
From: James Healy <james@yob.id.au>
Date: Mon, 20 Jul 2020 16:31:32 +1000
Subject: [PATCH] Exit the agent of reflection on flags doesn't work the way we
 expect

This method is used to ensure Environment Variables that configure the
agent process aren't automatically passed down to the child process that
runs each job. We want to be very explicit about which variables end up
in the job process.

It's implemented using reflection on the flag structs from the ufave/cli
package we use for CLI structure. It checks the EnvVar field on the flag
struct, which works just fine with ufave/cli/v1. However, that field
doesn't exist any more in ufave/cli/v2 and this method has started
silently failing since we upgraded to v2 in #1233.

I haven't added tests because no files in this package have tests and
I'm not 100% sure where to start adding them. However, I'm treating this
like an assertion. If the reflection doesn't work the way we expect, we
hard exit with an error, like other should-never-happen errors in
agent_start.go.

I'll fix the method to work with ufave/cli/v2 in the following commit.
---
 clicommand/agent_start.go | 6 +++++-
 clicommand/global.go      | 7 ++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/clicommand/agent_start.go b/clicommand/agent_start.go
index e98839075d..910955686d 100644
--- a/clicommand/agent_start.go
+++ b/clicommand/agent_start.go
@@ -489,7 +489,11 @@ var AgentStartCommand = cli.Command{
 		defer done()
 
 		// Remove any config env from the environment to prevent them propagating to bootstrap
-		UnsetConfigFromEnvironment(c)
+		err = UnsetConfigFromEnvironment(c)
+		if err != nil {
+			fmt.Printf("%s", err)
+			os.Exit(1)
+		}
 
 		// Check if git-mirrors are enabled
 		if experiments.IsEnabled(`git-mirrors`) {
diff --git a/clicommand/global.go b/clicommand/global.go
index 6fa6a43f1f..e69ca8dcb8 100644
--- a/clicommand/global.go
+++ b/clicommand/global.go
@@ -1,6 +1,7 @@
 package clicommand
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"reflect"
@@ -150,12 +151,15 @@ func HandleGlobalFlags(l logger.Logger, cfg interface{}) func() {
 	return HandleProfileFlag(l, cfg)
 }
 
-func UnsetConfigFromEnvironment(c *cli.Context) {
+func UnsetConfigFromEnvironment(c *cli.Context) error {
 	flags := append(c.App.Flags, c.Command.Flags...)
 	for _, fl := range flags {
 		// use golang reflection to find EnvVar values on flags
 		r := reflect.ValueOf(fl)
 		f := reflect.Indirect(r).FieldByName(`EnvVar`)
+		if !f.IsValid() {
+			return errors.New("EnvVar field not found on flag")
+		}
 		// split comma delimited env
 		if envVars := f.String(); envVars != `` {
 			for _, env := range strings.Split(envVars, ",") {
@@ -163,6 +167,7 @@ func UnsetConfigFromEnvironment(c *cli.Context) {
 			}
 		}
 	}
+	return nil
 }
 
 func loadAPIClientConfig(cfg interface{}, tokenField string) api.Config {