Move metrics and health checks to separate port (#298)

* listen to metrics on another port

* update readme to work with token projection

* add metrics port to helm chart

* remove formatting issue

* Use net.JoinHostPort instead of plain fmt.Sprintf

README.md

@@ -280,23 +280,21 @@ K8S_PORT=$(echo ${HOST_PORT} | awk -F':' '{print $2}')
 mkdir -p tmp 
 kubectl config set-context kind-azad-kube-proxy
 kubectl apply -f test/test-manifest.yaml
-cat <<EOF | kubectl apply -f -
+
+kubectl apply -f - <<EOF
 apiVersion: v1
-kind: Pod
+kind: Secret
 metadata:
-  name: temp
+  name: azad-kube-proxy-test-secret
   namespace: azad-kube-proxy-test
-spec:
-  serviceAccountName: azad-kube-proxy-test
-  containers:
-  - image: busybox
-    name: test
-    command: ["sleep"]
-    args: ["3000"]
+  annotations:
+    kubernetes.io/service-account.name: azad-kube-proxy-test
+type: kubernetes.io/service-account-token
 EOF
-kubectl exec -n azad-kube-proxy-test temp -- cat "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" > tmp/ca.crt
-kubectl exec -n azad-kube-proxy-test temp -- cat "/var/run/secrets/kubernetes.io/serviceaccount/token" > tmp/token
-kubectl delete -n azad-kube-proxy-test pod temp
+
+kubectl get secret azad-kube-proxy-test-secret --output=jsonpath={.data.ca\\.crt} | base64 -d > tmp/ca.crt
+kubectl get secret azad-kube-proxy-test-secret --output=jsonpath={.data.token} | base64 -d > tmp/token
+
 KUBE_CA_PATH="${PWD}/tmp/ca.crt"
 KUBE_TOKEN_PATH="${PWD}/tmp/token"
 ```

charts/azad-kube-proxy/templates/deployment.yaml

@@ -32,11 +32,14 @@ spec:
           env:
             - name: PORT
               value: {{ .Values.application.port | quote }}
+            - name: METRICS_PORT
+              value: {{ .Values.application.metricsPort | quote }}
             {{- if .Values.podEnv }}
 {{ toYaml .Values.podEnv | indent 12 }}
             {{- end }}
           args:
             - --port=$(PORT)
+            - --metrics-port=$(METRICS_PORT)
             {{- if .Values.podArgs }}
 {{ toYaml .Values.podArgs | indent 12 }}
             {{- end }}
@@ -48,15 +51,18 @@ spec:
             - name: http
               containerPort: {{ .Values.application.port }}
               protocol: TCP
+            - name: metrics
+              containerPort: {{ .Values.application.metricsPort }}
+              protocol: TCP
           livenessProbe:
             httpGet:
               path: /healthz
-              port: http
+              port: metrics
               scheme: {{ .Values.application.scheme }}
           readinessProbe:
             httpGet:
               path: /readyz
-              port: http
+              port: metrics
               scheme: {{ .Values.application.scheme }}
           resources:
             {{- toYaml .Values.resources | nindent 12 }}

charts/azad-kube-proxy/values.yaml

@@ -1,5 +1,6 @@
 application:
   port: 8080
+  metricsPort: 8081
   scheme: HTTP # HTTP or HTTPS
 
 # Using environment variables by default

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/urfave/cli/v2 v2.3.0
 	go.uber.org/zap v1.19.1
 	golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f
+	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
 	gopkg.in/square/go-jose.v2 v2.6.0 // indirect
 	k8s.io/api v0.22.2
 	k8s.io/apimachinery v0.22.2

go.sum

@@ -516,6 +516,7 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

pkg/config/config.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/x509"
 	"fmt"
+	"net"
 	"net/url"
 	"os"
 	"time"
@@ -16,22 +17,23 @@ import (
 
 // Config contains the configuration that is used for the application
 type Config struct {
-	ClientID             string `validate:"required,uuid"`
-	ClientSecret         string `validate:"required,min=1"`
-	TenantID             string `validate:"required,uuid"`
-	ListenerAddress      string `validate:"hostname_port"`
-	ListenerTLSConfig    ListenerTLSConfig
-	CacheEngine          models.CacheEngine
-	RedisURI             string `validate:"uri"`
-	AzureADGroupPrefix   string
-	AzureADMaxGroupCount int `validate:"min=1,max=1000"`
-	GroupSyncInterval    time.Duration
-	GroupIdentifier      models.GroupIdentifier
-	KubernetesConfig     KubernetesConfig
-	Dashboard            models.Dashboard
-	Metrics              models.Metrics
-	K8dashConfig         K8dashConfig
-	CORSConfig           CORSConfig
+	ClientID               string `validate:"required,uuid"`
+	ClientSecret           string `validate:"required,min=1"`
+	TenantID               string `validate:"required,uuid"`
+	ListenerAddress        string `validate:"hostname_port"`
+	MetricsListenerAddress string `validate:"hostname_port"`
+	ListenerTLSConfig      ListenerTLSConfig
+	CacheEngine            models.CacheEngine
+	RedisURI               string `validate:"uri"`
+	AzureADGroupPrefix     string
+	AzureADMaxGroupCount   int `validate:"min=1,max=1000"`
+	GroupSyncInterval      time.Duration
+	GroupIdentifier        models.GroupIdentifier
+	KubernetesConfig       KubernetesConfig
+	Dashboard              models.Dashboard
+	Metrics                models.Metrics
+	K8dashConfig           K8dashConfig
+	CORSConfig             CORSConfig
 }
 
 // KubernetesConfig contains the Kubernetes specific configuration
@@ -101,6 +103,13 @@ func Flags(ctx context.Context) []cli.Flag {
 			EnvVars:  []string{"PORT"},
 			Value:    8080,
 		},
+		&cli.IntFlag{
+			Name:     "metrics-port",
+			Usage:    "Port number for metrics and health checks to listen on",
+			Required: false,
+			EnvVars:  []string{"METRICS_PORT"},
+			Value:    8081,
+		},
 		&cli.StringFlag{
 			Name:     "tls-certificate-path",
 			Usage:    "Path for the TLS Certificate",
@@ -326,10 +335,11 @@ func NewConfig(ctx context.Context, cli *cli.Context) (Config, error) {
 	}
 
 	config := Config{
-		ClientID:        cli.String("client-id"),
-		ClientSecret:    cli.String("client-secret"),
-		TenantID:        cli.String("tenant-id"),
-		ListenerAddress: fmt.Sprintf("%s:%d", cli.String("address"), cli.Int("port")),
+		ClientID:               cli.String("client-id"),
+		ClientSecret:           cli.String("client-secret"),
+		TenantID:               cli.String("tenant-id"),
+		ListenerAddress:        net.JoinHostPort(cli.String("address"), fmt.Sprintf("%d", cli.Int("port"))),
+		MetricsListenerAddress: net.JoinHostPort(cli.String("address"), fmt.Sprintf("%d", cli.Int("metrics-port"))),
 		ListenerTLSConfig: ListenerTLSConfig{
 			Enabled:         cli.Bool("tls-enabled"),
 			CertificatePath: cli.String("tls-certificate-path"),

pkg/config/config_test.go

@@ -402,6 +402,7 @@ func TestConfigValidate(t *testing.T) {
 		c.config.TenantID = "00000000-0000-0000-0000-000000000000"
 		c.config.ClientSecret = "00000000-0000-0000-0000-000000000000"
 		c.config.ListenerAddress = "0.0.0.0:8080"
+		c.config.MetricsListenerAddress = "0.0.0.0:8081"
 		c.config.RedisURI = "redis://127.0.0.1:6379/0"
 		c.config.AzureADMaxGroupCount = 50
 		err := c.config.Validate()

pkg/proxy/proxy.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"net/http"
 	"net/http/httputil"
 	"os"
@@ -23,6 +24,7 @@ import (
 	"github.com/xenitab/azad-kube-proxy/pkg/health"
 	"github.com/xenitab/azad-kube-proxy/pkg/metrics"
 	"github.com/xenitab/azad-kube-proxy/pkg/user"
+	"golang.org/x/sync/errgroup"
 )
 
 // ClientInterface ...
@@ -99,8 +101,13 @@ func (client *Client) Start(ctx context.Context) error {
 	log := logr.FromContextOrDiscard(ctx)
 
 	// Signal handler
-	done := make(chan os.Signal, 1)
-	signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
+	stopChan := make(chan os.Signal, 2)
+	signal.Notify(stopChan, os.Interrupt, syscall.SIGINT, syscall.SIGTERM, syscall.SIGPIPE)
+
+	// Error group
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+	g, ctx := errgroup.WithContext(ctx)
 
 	// Initiate group sync
 	log.Info("Starting group sync")
@@ -119,19 +126,36 @@ func (client *Client) Start(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	log.Info("Initializing reverse proxy", "ListenerAddress", client.Config.ListenerAddress)
+	log.Info("Initializing reverse proxy", "ListenerAddress", client.Config.ListenerAddress, "MetricsListenerAddress", client.Config.MetricsListenerAddress, "ListenerTLSConfig.Enabled", client.Config.ListenerTLSConfig.Enabled)
 	proxy := client.getReverseProxy(ctx)
 	proxy.ErrorHandler = proxyHandlers.ErrorHandler(ctx)
 
-	router := mux.NewRouter()
-	router.HandleFunc("/readyz", proxyHandlers.ReadinessHandler(ctx)).Methods("GET")
-	router.HandleFunc("/healthz", proxyHandlers.LivenessHandler(ctx)).Methods("GET")
+	// Setup metrics router
+	metricsRouter := mux.NewRouter()
+
+	metricsRouter.HandleFunc("/readyz", proxyHandlers.ReadinessHandler(ctx)).Methods("GET")
+	metricsRouter.HandleFunc("/healthz", proxyHandlers.LivenessHandler(ctx)).Methods("GET")
 
-	router, err = client.MetricsClient.MetricsHandler(ctx, router)
+	metricsRouter, err = client.MetricsClient.MetricsHandler(ctx, metricsRouter)
 	if err != nil {
 		return err
 	}
 
+	metricsHttpServer := client.getHTTPMetricsServer(metricsRouter)
+
+	// Start metrics server
+	g.Go(func() error {
+		err := client.listenAndServe(metricsHttpServer)
+		if err != nil && err != http.ErrServerClosed {
+			return err
+		}
+
+		return nil
+	})
+
+	// Setup http router
+	router := mux.NewRouter()
+
 	router, err = client.DashboardClient.DashboardHandler(ctx, router)
 	if err != nil {
 		return err
@@ -143,30 +167,56 @@ func (client *Client) Start(ctx context.Context) error {
 	httpServer := client.getHTTPServer(router)
 
 	// Start HTTP server
-	go func() {
+	g.Go(func() error {
 		err := client.listenAndServe(httpServer)
 		if err != nil && err != http.ErrServerClosed {
-			log.Error(err, "Server error")
+			return err
 		}
-	}()
+
+		return nil
+	})
 
 	log.Info("Server started")
 
 	// Blocks until signal is sent
-	<-done
+	var doneMsg string
+	select {
+	case sig := <-stopChan:
+		doneMsg = fmt.Sprintf("os.Signal (%s)", sig)
+	case <-ctx.Done():
+		doneMsg = "context"
+	}
 
-	log.Info("Server shutdown initiated")
+	log.Info("Server shutdown initiated", "reason", doneMsg)
 
 	// Shutdown http server
 	shutdownCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
-	defer func() {
-		cancel()
-	}()
+	defer cancel()
+
+	g.Go(func() error {
+		err = httpServer.Shutdown(shutdownCtx)
+		if err != nil {
+			log.Error(err, "http server shutdown failed")
+			return err
+		}
+
+		return nil
+	})
+
+	// Shutdown metrics server
+	g.Go(func() error {
+		err = metricsHttpServer.Shutdown(shutdownCtx)
+		if err != nil {
+			log.Error(err, "metrics server shutdown failed")
+			return err
+		}
+
+		return nil
+	})
 
-	err = httpServer.Shutdown(shutdownCtx)
+	err = g.Wait()
 	if err != nil {
-		log.Error(err, "Server shutdown failed")
-		return err
+		return fmt.Errorf("error groups error: %w", err)
 	}
 
 	log.Info("Server exited properly")
@@ -186,6 +236,10 @@ func (client *Client) getHTTPServer(handler http.Handler) *http.Server {
 	return &http.Server{Addr: client.Config.ListenerAddress, Handler: handler}
 }
 
+func (client *Client) getHTTPMetricsServer(handler http.Handler) *http.Server {
+	return &http.Server{Addr: client.Config.MetricsListenerAddress, Handler: handler}
+}
+
 func (client *Client) getReverseProxy(ctx context.Context) *httputil.ReverseProxy {
 	reverseProxy := httputil.NewSingleHostReverseProxy(client.Config.KubernetesConfig.URL)
 	reverseProxy.Transport = client.getProxyTransport()