cmd: use env vars as defaults

Insted of setting them afterward in Run.

This change required a small re-ordering of the test to patch the
environment before calling New()

command/connect/envoy/envoy.go

@@ -74,13 +74,13 @@ const defaultEnvoyVersion = "1.13.1"
 func (c *cmd) init() {
 	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
 
-	c.flags.StringVar(&c.proxyID, "proxy-id", "",
+	c.flags.StringVar(&c.proxyID, "proxy-id", os.Getenv("CONNECT_PROXY_ID"),
 		"The proxy's ID on the local agent.")
 
 	c.flags.BoolVar(&c.meshGateway, "mesh-gateway", false,
 		"Configure Envoy as a Mesh Gateway.")
 
-	c.flags.StringVar(&c.sidecarFor, "sidecar-for", "",
+	c.flags.StringVar(&c.sidecarFor, "sidecar-for", os.Getenv("CONNECT_SIDECAR_FOR"),
 		"The ID of a service instance on the local agent that this proxy should "+
 			"become a sidecar for. It requires that the proxy service is registered "+
 			"with the agent as a connect-proxy with Proxy.DestinationServiceID set "+
@@ -110,7 +110,7 @@ func (c *cmd) init() {
 			"cases where either assumption is violated this flag will prevent the "+
 			"command attempting to resolve config from the local agent.")
 
-	c.flags.StringVar(&c.grpcAddr, "grpc-addr", "",
+	c.flags.StringVar(&c.grpcAddr, "grpc-addr", os.Getenv(api.GRPCAddrEnvName),
 		"Set the agent's gRPC address and port (in http(s)://host:port format). "+
 			"Alternatively, you can specify CONSUL_GRPC_ADDR in ENV.")
 
@@ -222,17 +222,6 @@ func (c *cmd) Run(args []string) int {
 	}
 	passThroughArgs := c.flags.Args()
 
-	// Load the proxy ID and token from env vars if they're set
-	if c.proxyID == "" {
-		c.proxyID = os.Getenv("CONNECT_PROXY_ID")
-	}
-	if c.sidecarFor == "" {
-		c.sidecarFor = os.Getenv("CONNECT_SIDECAR_FOR")
-	}
-	if c.grpcAddr == "" {
-		c.grpcAddr = os.Getenv(api.GRPCAddrEnvName)
-	}
-
 	// Setup Consul client
 	client, err := c.http.APIClient()
 	if err != nil {

command/connect/envoy/envoy_test.go

@@ -81,7 +81,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "defaults",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion:          defaultEnvoyVersion,
 				ProxyCluster:          "test-proxy",
@@ -98,7 +97,6 @@ func TestGenerateConfig(t *testing.T) {
 			Name: "token-arg",
 			Flags: []string{"-proxy-id", "test-proxy",
 				"-token", "c9a52720-bf6c-4aa6-b8bc-66881a5ade95"},
-			Env: []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion:          defaultEnvoyVersion,
 				ProxyCluster:          "test-proxy",
@@ -136,7 +134,6 @@ func TestGenerateConfig(t *testing.T) {
 			Flags: []string{"-proxy-id", "test-proxy",
 				"-token-file", "@@TEMPDIR@@token.txt",
 			},
-			Env: []string{},
 			Files: map[string]string{
 				"token.txt": "c9a52720-bf6c-4aa6-b8bc-66881a5ade95",
 			},
@@ -179,7 +176,6 @@ func TestGenerateConfig(t *testing.T) {
 			Name: "grpc-addr-flag",
 			Flags: []string{"-proxy-id", "test-proxy",
 				"-grpc-addr", "localhost:9999"},
-			Env: []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion: defaultEnvoyVersion,
 				ProxyCluster: "test-proxy",
@@ -220,7 +216,6 @@ func TestGenerateConfig(t *testing.T) {
 			Name: "grpc-addr-unix",
 			Flags: []string{"-proxy-id", "test-proxy",
 				"-grpc-addr", "unix:///var/run/consul.sock"},
-			Env: []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion:          defaultEnvoyVersion,
 				ProxyCluster:          "test-proxy",
@@ -254,7 +249,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "access-log-path",
 			Flags: []string{"-proxy-id", "test-proxy", "-admin-access-log-path", "/some/path/access.log"},
-			Env:   []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion: defaultEnvoyVersion,
 				ProxyCluster: "test-proxy",
@@ -273,7 +267,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "missing-ca-file",
 			Flags: []string{"-proxy-id", "test-proxy", "-ca-file", "some/path"},
-			Env:   []string{},
 			WantArgs: BootstrapTplArgs{
 				EnvoyVersion: defaultEnvoyVersion,
 				ProxyCluster: "test-proxy",
@@ -310,7 +303,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "custom-bootstrap",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			ProxyConfig: map[string]interface{}{
 				// Add a completely custom bootstrap template. Never mind if this is
 				// invalid envoy config just as long as it works and gets the variables
@@ -348,7 +340,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "extra_-single",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			ProxyConfig: map[string]interface{}{
 				// Add a custom sections with interpolated variables. These are all
 				// invalid config syntax too but we are just testing they have the right
@@ -381,7 +372,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "extra_-multiple",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			ProxyConfig: map[string]interface{}{
 				// Add a custom sections with interpolated variables. These are all
 				// invalid config syntax too but we are just testing they have the right
@@ -419,7 +409,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "stats-config-override",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			ProxyConfig: map[string]interface{}{
 				// Add a custom sections with interpolated variables. These are all
 				// invalid config syntax too but we are just testing they have the right
@@ -444,7 +433,6 @@ func TestGenerateConfig(t *testing.T) {
 		{
 			Name:  "zipkin-tracing-config",
 			Flags: []string{"-proxy-id", "test-proxy"},
-			Env:   []string{},
 			ProxyConfig: map[string]interface{}{
 				// Add a custom sections with interpolated variables. These are all
 				// invalid config syntax too but we are just testing they have the right
@@ -520,9 +508,6 @@ func TestGenerateConfig(t *testing.T) {
 				}
 			}
 
-			ui := cli.NewMockUi()
-			c := New(ui)
-
 			// Run a mock agent API that just always returns the proxy config in the
 			// test.
 			srv := httptest.NewServer(testMockAgent(tc.ProxyConfig, tc.GRPCPort))
@@ -530,15 +515,15 @@ func TestGenerateConfig(t *testing.T) {
 
 			// Set the agent HTTP address in ENV to be our mock
 			tc.Env = append(tc.Env, "CONSUL_HTTP_ADDR="+srv.URL)
-
 			testDirPrefix := testDir + string(filepath.Separator)
-
-			myFlags := copyAndReplaceAll(tc.Flags, "@@TEMPDIR@@", testDirPrefix)
 			myEnv := copyAndReplaceAll(tc.Env, "@@TEMPDIR@@", testDirPrefix)
-
 			defer testSetAndResetEnv(t, myEnv)()
 
+			ui := cli.NewMockUi()
+			c := New(ui)
+
 			// Run the command
+			myFlags := copyAndReplaceAll(tc.Flags, "@@TEMPDIR@@", testDirPrefix)
 			args := append([]string{"-bootstrap"}, myFlags...)
 			code := c.Run(args)
 			if tc.WantErr == "" {