From 0888c6575b0ba6408663294d951c2173141b6c9e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 7 Apr 2020 18:02:56 -0400 Subject: [PATCH] Step 3: fix a bug in api.NewClient and fix the tests The api client should never rever to HTTP if the user explicitly requested TLS. This change broke some tests because the tests always use an non-TLS http server, but some tests explicitly enable TLS. --- api/api.go | 6 +++--- command/connect/envoy/envoy.go | 13 ++++++++----- command/connect/envoy/envoy_test.go | 10 +++++++--- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/api/api.go b/api/api.go index a42a110bce..7b00be967a 100644 --- a/api/api.go +++ b/api/api.go @@ -551,11 +551,11 @@ func NewClient(config *Config) (*Client, error) { // bootstrap the config defConfig := DefaultConfig() - if len(config.Address) == 0 { + if config.Address == "" { config.Address = defConfig.Address } - if len(config.Scheme) == 0 { + if config.Scheme == "" { config.Scheme = defConfig.Scheme } @@ -599,7 +599,7 @@ func NewClient(config *Config) (*Client, error) { if len(parts) == 2 { switch parts[0] { case "http": - config.Scheme = "http" + // Never revert to http if TLS was explicitly requested. case "https": config.Scheme = "https" case "unix": diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index 13824d39b5..fd5177638d 100644 --- a/command/connect/envoy/envoy.go +++ b/command/connect/envoy/envoy.go @@ -199,16 +199,19 @@ func (c *cmd) Run(args []string) int { if err := c.flags.Parse(args); err != nil { return 1 } - passThroughArgs := c.flags.Args() // Setup Consul client - client, err := c.http.APIClient() + var err error + c.client, err = c.http.APIClient() if err != nil { c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 } - c.client = client + // TODO: refactor + return c.run(c.flags.Args()) +} +func (c *cmd) run(args []string) int { // Fixup for deprecated mesh-gateway flag if c.meshGateway && c.gateway != "" { c.UI.Error("The mesh-gateway flag is deprecated and cannot be used alongside the gateway flag") @@ -311,7 +314,7 @@ func (c *cmd) Run(args []string) int { }, } - if err := client.Agent().ServiceRegister(&svc); err != nil { + if err := c.client.Agent().ServiceRegister(&svc); err != nil { c.UI.Error(fmt.Sprintf("Error registering service %q: %s", svc.Name, err)) return 1 } @@ -363,7 +366,7 @@ func (c *cmd) Run(args []string) int { return 1 } - err = execEnvoy(binary, nil, passThroughArgs, bootstrapJson) + err = execEnvoy(binary, nil, args, bootstrapJson) if err == errUnsupportedOS { c.UI.Error("Directly running Envoy is only supported on linux and macOS " + "since envoy itself doesn't build on other platforms currently.") diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go index 32fe3dd7e3..2b45f8a3d3 100644 --- a/command/connect/envoy/envoy_test.go +++ b/command/connect/envoy/envoy_test.go @@ -589,20 +589,24 @@ func TestGenerateConfig(t *testing.T) { // test. srv := httptest.NewServer(testMockAgent(tc.ProxyConfig, tc.GRPCPort)) defer srv.Close() + client, err := api.NewClient(&api.Config{Address: srv.URL}) + require.NoError(err) - // 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) myEnv := copyAndReplaceAll(tc.Env, "@@TEMPDIR@@", testDirPrefix) defer testSetAndResetEnv(t, myEnv)() ui := cli.NewMockUi() c := New(ui) + // explicitly set the client to one which can connect to the httptest.Server + c.client = client // Run the command myFlags := copyAndReplaceAll(tc.Flags, "@@TEMPDIR@@", testDirPrefix) args := append([]string{"-bootstrap"}, myFlags...) - code := c.Run(args) + + require.NoError(c.flags.Parse(args)) + code := c.run(c.flags.Args()) if tc.WantErr == "" { require.Equal(0, code, ui.ErrorWriter.String()) } else {