From 8df37469272629e6357bfafc1cda4ee6b3cce3d1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 23 Mar 2020 10:49:50 -0400 Subject: [PATCH] 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 | 17 +++-------------- command/connect/envoy/envoy_test.go | 23 ++++------------------- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index a9e1785fa5..a62f80fe0f 100644 --- a/command/connect/envoy/envoy.go +++ b/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 { diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go index 2b4228ff08..8b2d1996a2 100644 --- a/command/connect/envoy/envoy_test.go +++ b/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 == "" {