Merge branch 'master' of github.com:mailgun/gubernator into PIP-1490-…

…hashring

config.go

@@ -173,6 +173,13 @@ type DaemonConfig struct {
 	// (Required) The `address:port` that will accept HTTP requests
 	HTTPListenAddress string
 
+	// (Optional) The `address:port` that will accept HTTP requests for /v1/HealthCheck
+	// without verifying client certificates. Only starts listener when TLS config is provided.
+	// TLS config is identical to what is applied on HTTPListenAddress, except that server
+	// Does not attempt to verify client certificate. Useful when your health probes cannot
+	// provide client certificate but you want to enforce mTLS in other RPCs (like in K8s)
+	HTTPStatusListenAddress string
+
 	// (Optional) Defines the max age connection from client in seconds.
 	// Default is infinity
 	GRPCMaxConnectionAgeSeconds int
@@ -257,6 +264,7 @@ func SetupDaemonConfig(logger *logrus.Logger, configFile string) (DaemonConfig,
 	// Main config
 	setter.SetDefault(&conf.GRPCListenAddress, os.Getenv("GUBER_GRPC_ADDRESS"), "localhost:81")
 	setter.SetDefault(&conf.HTTPListenAddress, os.Getenv("GUBER_HTTP_ADDRESS"), "localhost:80")
+	setter.SetDefault(&conf.HTTPStatusListenAddress, os.Getenv("GUBER_STATUS_HTTP_ADDRESS"), "")
 	setter.SetDefault(&conf.GRPCMaxConnectionAgeSeconds, getEnvInteger(log, "GUBER_GRPC_MAX_CONN_AGE_SEC"), 0)
 	setter.SetDefault(&conf.CacheSize, getEnvInteger(log, "GUBER_CACHE_SIZE"), 50_000)
 	setter.SetDefault(&conf.AdvertiseAddress, os.Getenv("GUBER_ADVERTISE_ADDRESS"), conf.GRPCListenAddress)

daemon.go

@@ -18,6 +18,7 @@ package gubernator
 
 import (
 	"context"
+	"crypto/tls"
 	"log"
 	"net"
 	"net/http"
@@ -47,16 +48,17 @@ type Daemon struct {
 	HTTPListener  net.Listener
 	V1Server      *V1Instance
 
-	log          logrus.FieldLogger
-	pool         PoolInterface
-	conf         DaemonConfig
-	httpSrv      *http.Server
-	grpcSrvs     []*grpc.Server
-	wg           syncutil.WaitGroup
-	statsHandler *GRPCStatsHandler
-	promRegister *prometheus.Registry
-	gwCancel     context.CancelFunc
-	gubeConfig   Config
+	log           logrus.FieldLogger
+	pool          PoolInterface
+	conf          DaemonConfig
+	httpSrv       *http.Server
+	httpSrvNoMTLS *http.Server
+	grpcSrvs      []*grpc.Server
+	wg            syncutil.WaitGroup
+	statsHandler  *GRPCStatsHandler
+	promRegister  *prometheus.Registry
+	gwCancel      context.CancelFunc
+	gubeConfig    Config
 }
 
 // SpawnDaemon starts a new gubernator daemon according to the provided DaemonConfig.
@@ -279,7 +281,37 @@ func (s *Daemon) Start(ctx context.Context) error {
 		return errors.Wrap(err, "while starting HTTP listener")
 	}
 
+	addrs := []string{s.conf.HTTPListenAddress}
+
 	if s.conf.ServerTLS() != nil {
+
+		// If configured, start another listener at configured address and server only
+		// /v1/HealthCheck while not requesting or verifying client certificate.
+		if s.conf.HTTPStatusListenAddress != "" {
+			addrs = append(addrs, s.conf.HTTPStatusListenAddress)
+			muxNoMTLS := http.NewServeMux()
+			muxNoMTLS.Handle("/v1/HealthCheck", gateway)
+			s.httpSrvNoMTLS = &http.Server{
+				Addr:      s.conf.HTTPStatusListenAddress,
+				Handler:   muxNoMTLS,
+				ErrorLog:  log,
+				TLSConfig: s.conf.ServerTLS().Clone(),
+			}
+			s.httpSrvNoMTLS.TLSConfig.ClientAuth = tls.NoClientCert
+			httpListener, err := net.Listen("tcp", s.conf.HTTPStatusListenAddress)
+			if err != nil {
+				return errors.Wrap(err, "while starting HTTP listener for health metric")
+			}
+			s.wg.Go(func() {
+				s.log.Infof("HTTPS Status Handler Listening on %s ...", s.conf.HTTPStatusListenAddress)
+				if err := s.httpSrvNoMTLS.ServeTLS(httpListener, "", ""); err != nil {
+					if err != http.ErrServerClosed {
+						s.log.WithError(err).Error("while starting TLS Status HTTP server")
+					}
+				}
+			})
+		}
+
 		// This is to avoid any race conditions that might occur
 		// since the tls config is a shared pointer.
 		s.httpSrv.TLSConfig = s.conf.ServerTLS().Clone()
@@ -303,7 +335,6 @@ func (s *Daemon) Start(ctx context.Context) error {
 	}
 
 	// Validate we can reach the GRPC and HTTP endpoints before returning
-	addrs := []string{s.conf.HTTPListenAddress}
 	for _, l := range s.GRPCListeners {
 		addrs = append(addrs, l.Addr().String())
 	}
@@ -316,7 +347,7 @@ func (s *Daemon) Start(ctx context.Context) error {
 
 // Close gracefully closes all server connections and listening sockets
 func (s *Daemon) Close() {
-	if s.httpSrv == nil {
+	if s.httpSrv == nil && s.httpSrvNoMTLS == nil {
 		return
 	}
 
@@ -326,6 +357,10 @@ func (s *Daemon) Close() {
 
 	s.log.Infof("HTTP Gateway close for %s ...", s.conf.HTTPListenAddress)
 	s.httpSrv.Shutdown(context.Background())
+	if s.httpSrvNoMTLS != nil {
+		s.log.Infof("HTTP Status Gateway close for %s ...", s.conf.HTTPStatusListenAddress)
+		s.httpSrvNoMTLS.Shutdown(context.Background())
+	}
 	for i, srv := range s.grpcSrvs {
 		s.log.Infof("GRPC close for %s ...", s.GRPCListeners[i].Addr())
 		srv.GracefulStop()
@@ -334,6 +369,7 @@ func (s *Daemon) Close() {
 	s.statsHandler.Close()
 	s.gwCancel()
 	s.httpSrv = nil
+	s.httpSrvNoMTLS = nil
 	s.grpcSrvs = nil
 }
 

tls_test.go

@@ -278,8 +278,9 @@ func TestTLSClusterWithClientAuthentication(t *testing.T) {
 
 func TestHTTPSClientAuth(t *testing.T) {
 	conf := gubernator.DaemonConfig{
-		GRPCListenAddress: "127.0.0.1:9695",
-		HTTPListenAddress: "127.0.0.1:9685",
+		GRPCListenAddress:       "127.0.0.1:9695",
+		HTTPListenAddress:       "127.0.0.1:9685",
+		HTTPStatusListenAddress: "127.0.0.1:9686",
 		TLS: &gubernator.TLSConfig{
 			CaFile:     "certs/ca.pem",
 			CertFile:   "certs/gubernator.pem",
@@ -291,17 +292,40 @@ func TestHTTPSClientAuth(t *testing.T) {
 	d := spawnDaemon(t, conf)
 	defer d.Close()
 
-	client := &http.Client{
+	clientWithCert := &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: conf.TLS.ClientTLS,
 		},
 	}
 
-	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s/v1/HealthCheck", conf.HTTPListenAddress), nil)
+	clientWithoutCert := &http.Client{
+		Transport: &http.Transport{
+			TLSClientConfig: &tls.Config{
+				RootCAs: conf.TLS.ServerTLS.RootCAs,
+			},
+		},
+	}
+
+	reqCertRequired, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s/v1/HealthCheck", conf.HTTPListenAddress), nil)
 	require.NoError(t, err)
-	resp, err := client.Do(req)
+	reqNoClientCertRequired, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s/v1/HealthCheck", conf.HTTPStatusListenAddress), nil)
+	require.NoError(t, err)
+
+	// Test that a client without a cert can access /v1/HealthCheck at status address
+	resp, err := clientWithoutCert.Do(reqNoClientCertRequired)
 	require.NoError(t, err)
 	b, err := ioutil.ReadAll(resp.Body)
 	require.NoError(t, err)
 	assert.Equal(t, `{"status":"healthy","message":"","peer_count":1}`, strings.ReplaceAll(string(b), " ", ""))
+
+	// Verify we get an error when we try to access existing HTTPListenAddress without cert
+	_, err = clientWithoutCert.Do(reqCertRequired)
+	assert.Error(t, err)
+
+	// Check that with a valid client cert we can access /v1/HealthCheck at existing HTTPListenAddress
+	resp, err = clientWithCert.Do(reqCertRequired)
+	require.NoError(t, err)
+	b, err = ioutil.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.Equal(t, `{"status":"healthy","message":"","peer_count":1}`, strings.ReplaceAll(string(b), " ", ""))
 }