From 280f14d64ca25bb6441919fe7c3aa07003357e6b Mon Sep 17 00:00:00 2001
From: Paul Banks <banks@banksco.de>
Date: Fri, 15 Jun 2018 21:04:04 +0100
Subject: [PATCH] Make proxy only listen after initial certs are fetched

---
 agent/agent.go               | 11 ++++++++---
 agent/agent_endpoint_test.go |  2 +-
 connect/proxy/proxy.go       | 36 ++++++++++++++++++++++++++----------
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/agent/agent.go b/agent/agent.go
index 987a063c2c..d3f672448c 100644
--- a/agent/agent.go
+++ b/agent/agent.go
@@ -2168,7 +2168,7 @@ func (a *Agent) applyProxyDefaults(proxy *structs.ConnectManagedProxy) error {
 	// If there is no globally configured default we need to get the
 	// default command so we can do "consul connect proxy"
 	if len(proxy.Command) == 0 {
-		command, err := defaultProxyCommand()
+		command, err := defaultProxyCommand(a.config)
 		if err != nil {
 			return err
 		}
@@ -2970,7 +2970,7 @@ func (a *Agent) registerCache() {
 }
 
 // defaultProxyCommand returns the default Connect managed proxy command.
-func defaultProxyCommand() ([]string, error) {
+func defaultProxyCommand(agentCfg *config.RuntimeConfig) ([]string, error) {
 	// Get the path to the current exectuable. This is cached once by the
 	// library so this is effectively just a variable read.
 	execPath, err := os.Executable()
@@ -2979,5 +2979,10 @@ func defaultProxyCommand() ([]string, error) {
 	}
 
 	// "consul connect proxy" default value for managed daemon proxy
-	return []string{execPath, "connect", "proxy"}, nil
+	cmd := []string{execPath, "connect", "proxy"}
+
+	if agentCfg != nil && agentCfg.LogLevel != "INFO" {
+		cmd = append(cmd, "-log-level", agentCfg.LogLevel)
+	}
+	return cmd, nil
 }
diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go
index b7d7c2755a..8e0685e6c8 100644
--- a/agent/agent_endpoint_test.go
+++ b/agent/agent_endpoint_test.go
@@ -3260,7 +3260,7 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) {
 	t.Parallel()
 
 	// Get the default command to compare below
-	defaultCommand, err := defaultProxyCommand()
+	defaultCommand, err := defaultProxyCommand(nil)
 	require.NoError(t, err)
 
 	// Define a local service with a managed proxy. It's registered in the test
diff --git a/connect/proxy/proxy.go b/connect/proxy/proxy.go
index ba2d3e8f11..8f74168b4a 100644
--- a/connect/proxy/proxy.go
+++ b/connect/proxy/proxy.go
@@ -36,9 +36,18 @@ func New(client *api.Client, cw ConfigWatcher, logger *log.Logger) (*Proxy, erro
 func (p *Proxy) Serve() error {
 	var cfg *Config
 
+	// failCh is used to stop Serve and return an error from another goroutine we
+	// spawn.
+	failCh := make(chan error, 1)
+
 	// Watch for config changes (initial setup happens on first "change")
 	for {
 		select {
+		case err := <-failCh:
+			// don't log here, we can log with better context at the point where we
+			// write the err to the chan
+			return err
+
 		case newCfg := <-p.cfgWatcher.Watch():
 			p.logger.Printf("[DEBUG] got new config")
 
@@ -64,20 +73,27 @@ func (p *Proxy) Serve() error {
 					tcfg := service.ServerTLSConfig()
 					cert, _ := tcfg.GetCertificate(nil)
 					leaf, _ := x509.ParseCertificate(cert.Certificate[0])
+<<<<<<< HEAD
 					p.logger.Printf("[DEBUG] leaf: %s roots: %s", leaf.URIs[0],
 						bytes.Join(tcfg.RootCAs.Subjects(), []byte(",")))
 				}()
-
-				// Only start a listener if we have a port set. This allows
-				// the configuration to disable our public listener.
-				if newCfg.PublicListener.BindPort != 0 {
-					newCfg.PublicListener.applyDefaults()
-					l := NewPublicListener(p.service, newCfg.PublicListener, p.logger)
-					err = p.startListener("public listener", l)
-					if err != nil {
-						return err
+=======
+					p.logger.Printf("[DEBUG] leaf: %s roots: %s", leaf.URIs[0], bytes.Join(tcfg.RootCAs.Subjects(), []byte(",")))
+>>>>>>> Make proxy only listen after initial certs are fetched
+
+					// Only start a listener if we have a port set. This allows
+					// the configuration to disable our public listener.
+					if newCfg.PublicListener.BindPort != 0 {
+						newCfg.PublicListener.applyDefaults()
+						l := NewPublicListener(p.service, newCfg.PublicListener, p.logger)
+						err = p.startListener("public listener", l)
+						if err != nil {
+							// This should probably be fatal.
+							p.logger.Printf("[ERR] failed to start public listener: %s", err)
+							failCh <- err
+						}
 					}
-				}
+				}()
 			}
 
 			// TODO(banks) update/remove upstreams properly based on a diff with current. Can