Extreme Makeover: server refactoring

healthcheck/healthcheck.go

@@ -19,12 +19,18 @@ import (
 var singleton *HealthCheck
 var once sync.Once
 
-// GetHealthCheck returns the health check which is guaranteed to be a singleton.
-func GetHealthCheck(metrics metricsRegistry) *HealthCheck {
-	once.Do(func() {
-		singleton = newHealthCheck(metrics)
-	})
-	return singleton
+// BalancerHandler includes functionality for load-balancing management.
+type BalancerHandler interface {
+	ServeHTTP(w http.ResponseWriter, req *http.Request)
+	Servers() []*url.URL
+	RemoveServer(u *url.URL) error
+	UpsertServer(u *url.URL, options ...roundrobin.ServerOption) error
+}
+
+// metricsRegistry is a local interface in the health check package, exposing only the required metrics
+// necessary for the health check package. This makes it easier for the tests.
+type metricsRegistry interface {
+	BackendServerUpGauge() metrics.Gauge
 }
 
 // Options are the public health check options.
@@ -36,59 +42,59 @@ type Options struct {
 	Port      int
 	Transport http.RoundTripper
 	Interval  time.Duration
-	LB        LoadBalancer
+	LB        BalancerHandler
 }
 
 func (opt Options) String() string {
 	return fmt.Sprintf("[Hostname: %s Headers: %v Path: %s Port: %d Interval: %s]", opt.Hostname, opt.Headers, opt.Path, opt.Port, opt.Interval)
 }
 
-// BackendHealthCheck HealthCheck configuration for a backend
-type BackendHealthCheck struct {
+// BackendConfig HealthCheck configuration for a backend
+type BackendConfig struct {
 	Options
 	name           string
 	disabledURLs   []*url.URL
 	requestTimeout time.Duration
 }
 
-// HealthCheck struct
-type HealthCheck struct {
-	Backends map[string]*BackendHealthCheck
-	metrics  metricsRegistry
-	cancel   context.CancelFunc
-}
+func (b *BackendConfig) newRequest(serverURL *url.URL) (*http.Request, error) {
+	u := &url.URL{}
+	*u = *serverURL
 
-// LoadBalancer includes functionality for load-balancing management.
-type LoadBalancer interface {
-	RemoveServer(u *url.URL) error
-	UpsertServer(u *url.URL, options ...roundrobin.ServerOption) error
-	Servers() []*url.URL
-}
+	if len(b.Scheme) > 0 {
+		u.Scheme = b.Scheme
+	}
 
-func newHealthCheck(metrics metricsRegistry) *HealthCheck {
-	return &HealthCheck{
-		Backends: make(map[string]*BackendHealthCheck),
-		metrics:  metrics,
+	if b.Port != 0 {
+		u.Host = net.JoinHostPort(u.Hostname(), strconv.Itoa(b.Port))
 	}
-}
 
-// metricsRegistry is a local interface in the health check package, exposing only the required metrics
-// necessary for the health check package. This makes it easier for the tests.
-type metricsRegistry interface {
-	BackendServerUpGauge() metrics.Gauge
+	u.Path += b.Path
+
+	return http.NewRequest(http.MethodGet, u.String(), nil)
 }
 
-// NewBackendHealthCheck Instantiate a new BackendHealthCheck
-func NewBackendHealthCheck(options Options, backendName string) *BackendHealthCheck {
-	return &BackendHealthCheck{
-		Options:        options,
-		name:           backendName,
-		requestTimeout: 5 * time.Second,
+// this function adds additional http headers and hostname to http.request
+func (b *BackendConfig) addHeadersAndHost(req *http.Request) *http.Request {
+	if b.Options.Hostname != "" {
+		req.Host = b.Options.Hostname
+	}
+
+	for k, v := range b.Options.Headers {
+		req.Header.Set(k, v)
 	}
+	return req
+}
+
+// HealthCheck struct
+type HealthCheck struct {
+	Backends map[string]*BackendConfig
+	metrics  metricsRegistry
+	cancel   context.CancelFunc
 }
 
 // SetBackendsConfiguration set backends configuration
-func (hc *HealthCheck) SetBackendsConfiguration(parentCtx context.Context, backends map[string]*BackendHealthCheck) {
+func (hc *HealthCheck) SetBackendsConfiguration(parentCtx context.Context, backends map[string]*BackendConfig) {
 	hc.Backends = backends
 	if hc.cancel != nil {
 		hc.cancel()
@@ -104,7 +110,7 @@ func (hc *HealthCheck) SetBackendsConfiguration(parentCtx context.Context, backe
 	}
 }
 
-func (hc *HealthCheck) execute(ctx context.Context, backend *BackendHealthCheck) {
+func (hc *HealthCheck) execute(ctx context.Context, backend *BackendConfig) {
 	log.Debugf("Initial health check for backend: %q", backend.name)
 	hc.checkBackend(backend)
 	ticker := time.NewTicker(backend.Interval)
@@ -121,7 +127,7 @@ func (hc *HealthCheck) execute(ctx context.Context, backend *BackendHealthCheck)
 	}
 }
 
-func (hc *HealthCheck) checkBackend(backend *BackendHealthCheck) {
+func (hc *HealthCheck) checkBackend(backend *BackendConfig) {
 	enabledURLs := backend.LB.Servers()
 	var newDisabledURLs []*url.URL
 	for _, url := range backend.disabledURLs {
@@ -152,38 +158,33 @@ func (hc *HealthCheck) checkBackend(backend *BackendHealthCheck) {
 	}
 }
 
-func (b *BackendHealthCheck) newRequest(serverURL *url.URL) (*http.Request, error) {
-	u := &url.URL{}
-	*u = *serverURL
-
-	if len(b.Scheme) > 0 {
-		u.Scheme = b.Scheme
-	}
-
-	if b.Port != 0 {
-		u.Host = net.JoinHostPort(u.Hostname(), strconv.Itoa(b.Port))
-	}
-
-	u.Path += b.Path
-
-	return http.NewRequest(http.MethodGet, u.String(), nil)
+// GetHealthCheck returns the health check which is guaranteed to be a singleton.
+func GetHealthCheck(metrics metricsRegistry) *HealthCheck {
+	once.Do(func() {
+		singleton = newHealthCheck(metrics)
+	})
+	return singleton
 }
 
-// this function adds additional http headers and hostname to http.request
-func (b *BackendHealthCheck) addHeadersAndHost(req *http.Request) *http.Request {
-	if b.Options.Hostname != "" {
-		req.Host = b.Options.Hostname
+func newHealthCheck(metrics metricsRegistry) *HealthCheck {
+	return &HealthCheck{
+		Backends: make(map[string]*BackendConfig),
+		metrics:  metrics,
 	}
+}
 
-	for k, v := range b.Options.Headers {
-		req.Header.Set(k, v)
+// NewBackendConfig Instantiate a new BackendConfig
+func NewBackendConfig(options Options, backendName string) *BackendConfig {
+	return &BackendConfig{
+		Options:        options,
+		name:           backendName,
+		requestTimeout: 5 * time.Second,
 	}
-	return req
 }
 
 // checkHealth returns a nil error in case it was successful and otherwise
 // a non-nil error with a meaningful description why the health check failed.
-func checkHealth(serverURL *url.URL, backend *BackendHealthCheck) error {
+func checkHealth(serverURL *url.URL, backend *BackendConfig) error {
 	req, err := backend.newRequest(serverURL)
 	if err != nil {
 		return fmt.Errorf("failed to create HTTP request: %s", err)

healthcheck/healthcheck_test.go

@@ -102,7 +102,7 @@ func TestSetBackendsConfiguration(t *testing.T) {
 			defer ts.Close()
 
 			lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}}
-			backend := NewBackendHealthCheck(Options{
+			backend := NewBackendConfig(Options{
 				Path:     "/path",
 				Interval: healthCheckInterval,
 				LB:       lb,
@@ -117,7 +117,7 @@ func TestSetBackendsConfiguration(t *testing.T) {
 
 			collectingMetrics := testhelpers.NewCollectingHealthCheckMetrics()
 			check := HealthCheck{
-				Backends: make(map[string]*BackendHealthCheck),
+				Backends: make(map[string]*BackendConfig),
 				metrics:  collectingMetrics,
 			}
 
@@ -209,7 +209,7 @@ func TestNewRequest(t *testing.T) {
 		t.Run(test.desc, func(t *testing.T) {
 			t.Parallel()
 
-			backend := NewBackendHealthCheck(test.options, "backendName")
+			backend := NewBackendConfig(test.options, "backendName")
 
 			u, err := url.Parse(test.serverURL)
 			require.NoError(t, err)
@@ -279,7 +279,7 @@ func TestAddHeadersAndHost(t *testing.T) {
 		t.Run(test.desc, func(t *testing.T) {
 			t.Parallel()
 
-			backend := NewBackendHealthCheck(test.options, "backendName")
+			backend := NewBackendConfig(test.options, "backendName")
 
 			u, err := url.Parse(test.serverURL)
 			require.NoError(t, err)
@@ -305,6 +305,10 @@ type testLoadBalancer struct {
 	servers            []*url.URL
 }
 
+func (lb *testLoadBalancer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	// noop
+}
+
 func (lb *testLoadBalancer) RemoveServer(u *url.URL) error {
 	lb.Lock()
 	defer lb.Unlock()

integration/access_log_test.go

@@ -102,7 +102,7 @@ func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) {
 			formatOnly:   false,
 			code:         "401",
 			user:         "-",
-			frontendName: "Auth for frontend-Host-frontend-auth-docker-local",
+			frontendName: "Basic Auth for frontend-Host-frontend-auth-docker-local",
 			backendURL:   "/",
 		},
 	}
@@ -354,7 +354,7 @@ func (s *AccessLogSuite) TestAccessLogEntrypointRedirect(c *check.C) {
 			formatOnly:   false,
 			code:         "302",
 			user:         "-",
-			frontendName: "entrypoint redirect for frontend-",
+			frontendName: "entrypoint redirect for httpRedirect",
 			backendURL:   "/",
 		},
 		{

middlewares/auth/authenticator.go

@@ -31,38 +31,43 @@ func NewAuthenticator(authConfig *types.Auth, tracingMiddleware *tracing.Tracing
 	if authConfig == nil {
 		return nil, fmt.Errorf("error creating Authenticator: auth is nil")
 	}
+
 	var err error
-	authenticator := Authenticator{}
-	tracingAuthenticator := tracingAuthenticator{}
+	authenticator := &Authenticator{}
+	tracingAuth := tracingAuthenticator{}
+
 	if authConfig.Basic != nil {
 		authenticator.users, err = parserBasicUsers(authConfig.Basic)
 		if err != nil {
 			return nil, err
 		}
+
 		basicAuth := goauth.NewBasicAuthenticator("traefik", authenticator.secretBasic)
-		tracingAuthenticator.handler = createAuthBasicHandler(basicAuth, authConfig)
-		tracingAuthenticator.name = "Auth Basic"
-		tracingAuthenticator.clientSpanKind = false
+		tracingAuth.handler = createAuthBasicHandler(basicAuth, authConfig)
+		tracingAuth.name = "Auth Basic"
+		tracingAuth.clientSpanKind = false
 	} else if authConfig.Digest != nil {
 		authenticator.users, err = parserDigestUsers(authConfig.Digest)
 		if err != nil {
 			return nil, err
 		}
+
 		digestAuth := goauth.NewDigestAuthenticator("traefik", authenticator.secretDigest)
-		tracingAuthenticator.handler = createAuthDigestHandler(digestAuth, authConfig)
-		tracingAuthenticator.name = "Auth Digest"
-		tracingAuthenticator.clientSpanKind = false
+		tracingAuth.handler = createAuthDigestHandler(digestAuth, authConfig)
+		tracingAuth.name = "Auth Digest"
+		tracingAuth.clientSpanKind = false
 	} else if authConfig.Forward != nil {
-		tracingAuthenticator.handler = createAuthForwardHandler(authConfig)
-		tracingAuthenticator.name = "Auth Forward"
-		tracingAuthenticator.clientSpanKind = true
+		tracingAuth.handler = createAuthForwardHandler(authConfig)
+		tracingAuth.name = "Auth Forward"
+		tracingAuth.clientSpanKind = true
 	}
+
 	if tracingMiddleware != nil {
-		authenticator.handler = tracingMiddleware.NewNegroniHandlerWrapper(tracingAuthenticator.name, tracingAuthenticator.handler, tracingAuthenticator.clientSpanKind)
+		authenticator.handler = tracingMiddleware.NewNegroniHandlerWrapper(tracingAuth.name, tracingAuth.handler, tracingAuth.clientSpanKind)
 	} else {
-		authenticator.handler = tracingAuthenticator.handler
+		authenticator.handler = tracingAuth.handler
 	}
-	return &authenticator, nil
+	return authenticator, nil
 }
 
 func createAuthForwardHandler(authConfig *types.Auth) negroni.HandlerFunc {

middlewares/cbreaker.go

@@ -23,14 +23,14 @@ func NewCircuitBreaker(next http.Handler, expression string, options ...cbreaker
 
 // NewCircuitBreakerOptions returns a new CircuitBreakerOption
 func NewCircuitBreakerOptions(expression string) cbreaker.CircuitBreakerOption {
-	return cbreaker.Fallback(
-		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			tracing.LogEventf(r, "blocked by circuitbreaker (%q)", expression)
-			w.WriteHeader(http.StatusServiceUnavailable)
-			w.Write([]byte(http.StatusText(http.StatusServiceUnavailable)))
-		}))
+	return cbreaker.Fallback(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		tracing.LogEventf(r, "blocked by circuit-breaker (%q)", expression)
+
+		w.WriteHeader(http.StatusServiceUnavailable)
+		w.Write([]byte(http.StatusText(http.StatusServiceUnavailable)))
+	}))
 }
 
-func (cb *CircuitBreaker) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
+func (cb *CircuitBreaker) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	cb.circuitBreaker.ServeHTTP(rw, r)
 }

middlewares/empty_backend_handler.go

@@ -10,13 +10,12 @@ import (
 // has at least one active Server in respect to the healthchecks and if this
 // is not the case, it will stop the middleware chain and respond with 503.
 type EmptyBackendHandler struct {
-	lb   healthcheck.LoadBalancer
-	next http.Handler
+	lb healthcheck.BalancerHandler
 }
 
 // NewEmptyBackendHandler creates a new EmptyBackendHandler instance.
-func NewEmptyBackendHandler(lb healthcheck.LoadBalancer, next http.Handler) *EmptyBackendHandler {
-	return &EmptyBackendHandler{lb: lb, next: next}
+func NewEmptyBackendHandler(lb healthcheck.BalancerHandler) *EmptyBackendHandler {
+	return &EmptyBackendHandler{lb: lb}
 }
 
 // ServeHTTP responds with 503 when there is no active Server and otherwise
@@ -26,6 +25,6 @@ func (h *EmptyBackendHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request)
 		rw.WriteHeader(http.StatusServiceUnavailable)
 		rw.Write([]byte(http.StatusText(http.StatusServiceUnavailable)))
 	} else {
-		h.next.ServeHTTP(rw, r)
+		h.lb.ServeHTTP(rw, r)
 	}
 }