diff --git a/README.md b/README.md index 9344d0db9..936989fda 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,7 @@ users: - [No Impersonation](./docs/tasks/no-impersonation.md) - [Extra Impersonations Headers](./docs/tasks/extra-impersonation-headers.md) - [Auditing](./docs/tasks/auditing.md) + - [Metrics](./docs/tasks/metrics.md) ## Development *NOTE*: building kube-oidc-proxy requires Go version 1.12 or higher. diff --git a/cmd/app/options/app.go b/cmd/app/options/app.go index 0a600cfe8..5bec0c9ce 100644 --- a/cmd/app/options/app.go +++ b/cmd/app/options/app.go @@ -13,6 +13,7 @@ import ( type KubeOIDCProxyOptions struct { DisableImpersonation bool ReadinessProbePort int + MetricsListenAddress string FlushInterval time.Duration @@ -43,6 +44,10 @@ func (k *KubeOIDCProxyOptions) AddFlags(fs *pflag.FlagSet) *KubeOIDCProxyOptions fs.IntVarP(&k.ReadinessProbePort, "readiness-probe-port", "P", 8080, "Port to expose readiness probe.") + fs.StringVar(&k.MetricsListenAddress, "metrics-serving-address", "0.0.0.0:80", + "Address to serve metrics on at the /metrics path. An empty address will "+ + "disable serving metrics. Cannot use the same address as proxy or probe.") + fs.DurationVar(&k.FlushInterval, "flush-interval", time.Millisecond*50, "Specifies the interval to flush request bodies. If 0ms, "+ "no periodic flushing is done. A negative value means to flush "+ diff --git a/cmd/app/run.go b/cmd/app/run.go index 9040869fb..d5c01c4f3 100644 --- a/cmd/app/run.go +++ b/cmd/app/run.go @@ -7,10 +7,13 @@ import ( "github.com/spf13/cobra" "k8s.io/apiserver/pkg/server" "k8s.io/client-go/rest" + "k8s.io/klog" "github.com/jetstack/kube-oidc-proxy/cmd/app/options" + "github.com/jetstack/kube-oidc-proxy/pkg/metrics" "github.com/jetstack/kube-oidc-proxy/pkg/probe" "github.com/jetstack/kube-oidc-proxy/pkg/proxy" + "github.com/jetstack/kube-oidc-proxy/pkg/proxy/hooks" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/tokenreview" "github.com/jetstack/kube-oidc-proxy/pkg/util" ) @@ -38,6 +41,14 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma return err } + // Initialise hooks handler + hooks := hooks.New() + defer func() { + if err := hooks.RunPreShutdownHooks(); err != nil { + klog.Errorf("failed to run shut down hooks: %s", err) + } + }() + // Here we determine to either use custom or 'in-cluster' client configuration var err error var restConfig *rest.Config @@ -57,10 +68,15 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma } } + // Initialise metrics handler + metrics := metrics.New() + // Add the metrics server as a shutdown hook + hooks.AddPreShutdownHook("Metrics", metrics.Shutdown) + // Initialise token reviewer if enabled var tokenReviewer *tokenreview.TokenReview if opts.App.TokenPassthrough.Enabled { - tokenReviewer, err = tokenreview.New(restConfig, opts.App.TokenPassthrough.Audiences) + tokenReviewer, err = tokenreview.New(restConfig, metrics, opts.App.TokenPassthrough.Audiences) if err != nil { return err } @@ -85,7 +101,7 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma // Initialise proxy with OIDC token authenticator p, err := proxy.New(restConfig, opts.OIDCAuthentication, opts.Audit, - tokenReviewer, secureServingInfo, proxyConfig) + tokenReviewer, secureServingInfo, hooks, metrics, proxyConfig) if err != nil { return err } @@ -97,10 +113,20 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma } // Start readiness probe - if err := probe.Run(strconv.Itoa(opts.App.ReadinessProbePort), - fakeJWT, p.OIDCTokenAuthenticator()); err != nil { + readinessHandler, err := probe.Run( + strconv.Itoa(opts.App.ReadinessProbePort), fakeJWT, p.OIDCTokenAuthenticator()) + if err != nil { return err } + hooks.AddPreShutdownHook("Readiness Probe", readinessHandler.Shutdown) + + if len(opts.App.MetricsListenAddress) > 0 { + if err := metrics.Start(opts.App.MetricsListenAddress); err != nil { + return err + } + } else { + klog.Info("metrics listen address empty, disabling serving metrics") + } // Run proxy waitCh, err := p.Run(stopCh) @@ -110,10 +136,6 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma <-waitCh - if err := p.RunPreShutdownHooks(); err != nil { - return err - } - return nil }, } diff --git a/deploy/charts/kube-oidc-proxy/templates/deployment.yaml b/deploy/charts/kube-oidc-proxy/templates/deployment.yaml index 66aa0a24f..d90051cf3 100644 --- a/deploy/charts/kube-oidc-proxy/templates/deployment.yaml +++ b/deploy/charts/kube-oidc-proxy/templates/deployment.yaml @@ -18,6 +18,11 @@ spec: labels: app.kubernetes.io/name: {{ include "kube-oidc-proxy.name" . }} app.kubernetes.io/instance: {{ .Release.Name }} + annotations: + {{- if and .Values.metrics.enabled }} + prometheus.io/scrape: 'true' + prometheus.io/port: '{{ .Values.metrics.port }}' + {{ end }} spec: serviceAccountName: {{ include "kube-oidc-proxy.fullname" . }} containers: @@ -25,6 +30,9 @@ spec: image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" imagePullPolicy: {{ .Values.image.pullPolicy }} ports: + {{- if and .Values.metrics.enabled }} + - containerPort: {{ .Values.metrics.port }} + {{ end }} - containerPort: 443 - containerPort: 8080 readinessProbe: @@ -74,6 +82,9 @@ spec: {{- range $key, $value := .Values.extraArgs -}} - "--{{ $key }}={{ $value -}}" {{ end }} + {{- if and .Values.metrics.enabled }} + - "--metrics-serving-address={{ .Values.metrics.address }}:{{ .Values.metrics.port }}" + {{ end }} resources: {{- toYaml .Values.resources | nindent 12 }} env: diff --git a/deploy/charts/kube-oidc-proxy/values.yaml b/deploy/charts/kube-oidc-proxy/values.yaml index e85214a28..0a3de9b05 100644 --- a/deploy/charts/kube-oidc-proxy/values.yaml +++ b/deploy/charts/kube-oidc-proxy/values.yaml @@ -96,6 +96,13 @@ podDisruptionBudget: enabled: false minAvailable: 1 +# Set the Prometheus metrics listen address. Setting an empty address string +# will disable the metrics server. +metrics: + enabled: true + port: 80 + address: "0.0.0.0" + resources: {} # We usually recommend not to specify default resources and to leave this as a conscious # choice for the user. This also increases chances charts run on environments with little diff --git a/deploy/yaml/kube-oidc-proxy.yaml b/deploy/yaml/kube-oidc-proxy.yaml index ad033219c..f0e4d9725 100644 --- a/deploy/yaml/kube-oidc-proxy.yaml +++ b/deploy/yaml/kube-oidc-proxy.yaml @@ -26,6 +26,10 @@ spec: metadata: labels: app: kube-oidc-proxy + annotations: + prometheus.io/path: /metrics + prometheus.io/port: "80" + prometheus.io/scrape: "true" spec: serviceAccountName: kube-oidc-proxy containers: @@ -33,6 +37,7 @@ spec: ports: - containerPort: 443 - containerPort: 8080 + - containerPort: 80 readinessProbe: httpGet: path: /ready diff --git a/docs/tasks/metrics.md b/docs/tasks/metrics.md new file mode 100644 index 000000000..eeea124e7 --- /dev/null +++ b/docs/tasks/metrics.md @@ -0,0 +1,42 @@ +# Metrics + +kube-oidc-proxy exposes a number of Prometheus metrics to give some insights +into how the proxy is performing. These metrics are designed to be used to +better inform how the proxy is behaving, and the general usage from clients, +*_not_* to alert, or otherwise give other security insights. If you are +interested in auditing and access review functionality, please refer to +[auditing](../auditing.md). + +The proxy exposes the following metrics: + +### kube_oidc_proxy_http_client_requests +counter - {http status code, path, remote address} +The number of incoming requests. + +### kube_oidc_proxy_http_client_duration_seconds +histogram - {remote address} +The duration in seconds for incoming client requests to be responded to. + +### kube_oidc_proxy_http_server_requests +counter - {http status code, path, remote address} +The number of outgoing server requests. + +### kube_oidc_proxy_http_server_duration_seconds +histogram - {remote address} +The duration in seconds for outgoing server requests to be responded to. + +### kube_oidc_proxy_token_review_duration_seconds +histogram - {authenticated, http status code, remote address, user} +The duration in seconds for a token review lookup. Authenticated requests are 1, else 0. + +### kube_oidc_proxy_oidc_authentication_count +counter - {authenticated, remote address, user} +The count for OIDC authentication. Authenticated requests are 1, else 0. + +## Metrics Address + +By default, metrics are exposed on `0.0.0.0:80/metrics`. The flag +`--metrics-serving-address` can be used to change the address, however the +`/metrics` path will remain the same. The metrics address must _not_ conflict +with the proxy and probe addresses. Setting `--metrics-serving-address=""` will +disable the metrics server. diff --git a/go.mod b/go.mod index c459eba2c..2e40e4cfe 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/heptiolabs/healthcheck v0.0.0-20180807145615-6ff867650f40 github.com/onsi/ginkgo v1.11.0 github.com/onsi/gomega v1.7.0 + github.com/prometheus/client_golang v1.0.0 github.com/sebest/xff v0.0.0-20160910043805-6c115e0ffa35 github.com/sirupsen/logrus v1.4.2 github.com/spf13/cobra v0.0.5 diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go new file mode 100644 index 000000000..c7b73eeaa --- /dev/null +++ b/pkg/metrics/metrics.go @@ -0,0 +1,216 @@ +// Copyright Jetstack Ltd. See LICENSE for details. +package metrics + +import ( + "context" + "fmt" + "net" + "net/http" + "strconv" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + "k8s.io/klog" +) + +const ( + promNamespace = "kube_oidc_proxy" +) + +type Metrics struct { + *http.Server + + registry *prometheus.Registry + + // Metrics for incoming client requests + clientRequests *prometheus.CounterVec + clientDuration *prometheus.HistogramVec + + // Metrics for outgoing server requests + serverRequests *prometheus.CounterVec + serverDuration *prometheus.HistogramVec + + // Metrics for authentication of incoming requests + oidcAuthCount *prometheus.CounterVec + + // Metrics for token reviews + tokenReviewDuration *prometheus.HistogramVec +} + +func New() *Metrics { + var ( + clientRequests = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: promNamespace, + Name: "http_client_requests", + Help: "The number of incoming requests.", + }, + []string{"code", "path"}, + ) + clientDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: promNamespace, + Name: "http_client_duration_seconds", + Help: "The duration in seconds for incoming client requests to be responded to.", + Buckets: prometheus.ExponentialBuckets(0.001, 2.5, 10), + }, + []string{"remote_address"}, + ) + + serverRequests = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: promNamespace, + Name: "http_server_requests", + Help: "The number of outgoing server requests.", + }, + []string{"code", "path"}, + ) + serverDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: promNamespace, + Name: "http_server_duration_seconds", + Help: "The duration in seconds for outgoing server requests to be responded to.", + Buckets: prometheus.ExponentialBuckets(0.001, 2.5, 14), + }, + []string{"remote_address"}, + ) + + oidcAuthCount = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: promNamespace, + Name: "oidc_authentication_count", + Help: "The count for OIDC authentication. Authenticated requests are 1, else 0.", + }, + []string{"authenticated", "remote_address", "user"}, + ) + + tokenReviewDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: promNamespace, + Name: "token_review_duration_seconds", + Help: "The duration in seconds for a token review lookup. Authenticated requests are 1, else 0.", + Buckets: prometheus.ExponentialBuckets(0.001, 2.5, 14), + }, + []string{"authenticated", "code", "remote_address", "user"}, + ) + ) + + registry := prometheus.NewRegistry() + registry.MustRegister(clientRequests) + registry.MustRegister(clientDuration) + registry.MustRegister(serverRequests) + registry.MustRegister(serverDuration) + registry.MustRegister(oidcAuthCount) + registry.MustRegister(tokenReviewDuration) + + return &Metrics{ + registry: registry, + + clientRequests: clientRequests, + clientDuration: clientDuration, + + serverRequests: serverRequests, + serverDuration: serverDuration, + + oidcAuthCount: oidcAuthCount, + tokenReviewDuration: tokenReviewDuration, + } +} + +// Start will register the Prometheus metrics, and start the Prometheus server +func (m *Metrics) Start(listenAddress string) error { + router := http.NewServeMux() + router.Handle("/metrics", promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{})) + + ln, err := net.Listen("tcp", listenAddress) + if err != nil { + return err + } + + m.Server = &http.Server{ + Addr: ln.Addr().String(), + ReadTimeout: 8 * time.Second, + WriteTimeout: 8 * time.Second, + MaxHeaderBytes: 1 << 15, // 1 MiB + Handler: router, + } + + go func() { + klog.Infof("serving metrics on %s/metrics", ln.Addr()) + + if err := m.Serve(ln); err != nil { + klog.Errorf("failed to serve prometheus metrics: %s", err) + return + } + }() + + return nil +} + +func (m *Metrics) Shutdown() error { + // If metrics server is not started than exit early + if m.Server == nil { + return nil + } + + klog.Info("shutting down Prometheus metrics server...") + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + + if err := m.Server.Shutdown(ctx); err != nil { + return fmt.Errorf("prometheus metrics server shutdown failed: %s", err) + } + + klog.Info("prometheus metrics server gracefully stopped") + + return nil +} + +func (m *Metrics) ObserveClient(code int, path, remoteAddress string, duration time.Duration) { + m.clientRequests.With(prometheus.Labels{ + "code": strconv.Itoa(code), + "path": path, + }).Inc() + + m.clientDuration.With(prometheus.Labels{ + "remote_address": remoteAddress, + }).Observe(duration.Seconds()) +} + +func (m *Metrics) ObserveServer(code int, path, remoteAddress string, duration time.Duration) { + m.serverRequests.With(prometheus.Labels{ + "code": strconv.Itoa(code), + "path": path, + }).Inc() + + m.serverDuration.With(prometheus.Labels{ + "remote_address": remoteAddress, + }).Observe(duration.Seconds()) +} + +func (m *Metrics) IncrementOIDCAuthCount(authenticated bool, remoteAddress, user string) { + m.oidcAuthCount.With(prometheus.Labels{ + "authenticated": boolToIntString(authenticated), + "remote_address": remoteAddress, + "user": user, + }).Inc() +} + +func (m *Metrics) ObserveTokenReivewLookup(authenticated bool, code int, remoteAddress, user string, duration time.Duration) { + m.tokenReviewDuration.With(prometheus.Labels{ + "authenticated": boolToIntString(authenticated), + "code": strconv.Itoa(code), + "remote_address": remoteAddress, + "user": user, + }).Observe(duration.Seconds()) +} + +func boolToIntString(b bool) string { + var i int + if b { + i = 1 + } + return strconv.Itoa(i) +} diff --git a/pkg/probe/probe.go b/pkg/probe/probe.go index 3e0248d51..a38c81fe8 100644 --- a/pkg/probe/probe.go +++ b/pkg/probe/probe.go @@ -19,34 +19,45 @@ const ( ) type HealthCheck struct { - handler healthcheck.Handler - + *http.Server oidcAuther authenticator.Token fakeJWT string ready bool } -func Run(port, fakeJWT string, oidcAuther authenticator.Token) error { +func Run(port, fakeJWT string, oidcAuther authenticator.Token) (*HealthCheck, error) { + handler := healthcheck.NewHandler() + + ln, err := net.Listen("tcp", "0.0.0.0:"+port) + if err != nil { + return nil, fmt.Errorf("failed to listen on health check port: %s", err) + } + h := &HealthCheck{ - handler: healthcheck.NewHandler(), oidcAuther: oidcAuther, fakeJWT: fakeJWT, + Server: &http.Server{ + Addr: ln.Addr().String(), + ReadTimeout: 8 * time.Second, + WriteTimeout: 8 * time.Second, + MaxHeaderBytes: 1 << 20, // 1 MiB + Handler: handler, + }, } - h.handler.AddReadinessCheck("secure serving", h.Check) + handler.AddReadinessCheck("secure serving", h.Check) go func() { - for { - err := http.ListenAndServe(net.JoinHostPort("0.0.0.0", port), h.handler) - if err != nil { - klog.Errorf("ready probe listener failed: %s", err) - } - time.Sleep(5 * time.Second) + klog.Infof("serving readiness probe on %s/ready", ln.Addr()) + + if err := h.Serve(ln); err != nil { + klog.Errorf("failed to serve readiness probe: %s", err) + return } }() - return nil + return h, nil } func (h *HealthCheck) Check() error { @@ -71,3 +82,23 @@ func (h *HealthCheck) Check() error { return nil } + +func (h *HealthCheck) Shutdown() error { + // If readiness probe server is not started than exit early + if h.Server == nil { + return nil + } + + klog.Info("shutting down readiness probe server...") + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + + if err := h.Server.Shutdown(ctx); err != nil { + return fmt.Errorf("readiness probe server shutdown failed: %s", err) + } + + klog.Info("readines probe server gracefully stopped") + + return nil +} diff --git a/pkg/probe/probe_test.go b/pkg/probe/probe_test.go index bb2524c9c..3574e019d 100644 --- a/pkg/probe/probe_test.go +++ b/pkg/probe/probe_test.go @@ -44,11 +44,18 @@ func TestRun(t *testing.T) { t.FailNow() } - if err := Run(port, fakeJWT, f); err != nil { + readinessHandler, err := Run(port, fakeJWT, f) + if err != nil { t.Error(err.Error()) t.FailNow() } + defer func() { + if err := readinessHandler.Shutdown(); err != nil { + t.Error(err) + } + }() + url := fmt.Sprintf("http://0.0.0.0:%s", port) var resp *http.Response diff --git a/pkg/proxy/context/context.go b/pkg/proxy/context/context.go index e1a0cd7e5..b9eae98be 100644 --- a/pkg/proxy/context/context.go +++ b/pkg/proxy/context/context.go @@ -3,6 +3,7 @@ package context import ( "net/http" + "time" "github.com/sebest/xff" "k8s.io/apiserver/pkg/endpoints/request" @@ -21,8 +22,11 @@ const ( // bearerTokenKey is the context key for the bearer token. bearerTokenKey - // bearerTokenKey is the context key for the client address. + // clientAddressKey is the context key for the client address. clientAddressKey + + // clientRequestTimestampKey is the context key for the timestamp of a client request. + clientRequestTimestampKey ) // WithNoImpersonation returns a copy of the request in which the noImpersonation context value is set. @@ -58,6 +62,17 @@ func BearerToken(req *http.Request) string { return token } +// WithClientRequestTimestamp will add the current timestamp to the request context. +func WithClientRequestTimestamp(req *http.Request) *http.Request { + return req.WithContext(request.WithValue(req.Context(), clientRequestTimestampKey, time.Now())) +} + +// ClientRequestTimestamp will return thetimestamp that the client request was received. +func ClientRequestTimestamp(req *http.Request) time.Time { + stamp, _ := req.Context().Value(clientRequestTimestampKey).(time.Time) + return stamp +} + // RemoteAddress will attempt to return the source client address if available // in the request context. If it is not, it will be gathered from the request // and entered into the context. diff --git a/pkg/proxy/handlers.go b/pkg/proxy/handlers.go index 74b33bfab..8fc6096a7 100644 --- a/pkg/proxy/handlers.go +++ b/pkg/proxy/handlers.go @@ -2,8 +2,10 @@ package proxy import ( + "errors" "net/http" "strings" + "time" authuser "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" @@ -14,8 +16,25 @@ import ( "github.com/jetstack/kube-oidc-proxy/pkg/proxy/context" ) +const ( + UserHeaderClientIPKey = "Remote-Client-IP" +) + +var ( + errUnauthorized = errors.New("Unauthorized") + errImpersonateHeader = errors.New("Impersonate-User in header") + errNoName = errors.New("No name in OIDC info") + errNoImpersonationConfig = errors.New("No impersonation configuration in context") + + // http headers are case-insensitive + impersonateUserHeader = strings.ToLower(transport.ImpersonateUserHeader) + impersonateGroupHeader = strings.ToLower(transport.ImpersonateGroupHeader) + impersonateExtraHeader = strings.ToLower(transport.ImpersonateUserExtraHeaderPrefix) +) + func (p *Proxy) withHandlers(handler http.Handler) http.Handler { // Set up proxy handlers + handler = p.withClientTimestamp(handler) handler = p.auditor.WithRequest(handler) handler = p.withImpersonateRequest(handler) handler = p.withAuthenticateRequest(handler) @@ -31,27 +50,29 @@ func (p *Proxy) withAuthenticateRequest(handler http.Handler) http.Handler { tokenReviewHandler := p.withTokenReview(handler) return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + req, remoteAddr := context.RemoteAddr(req) + // Auth request and handle unauthed info, ok, err := p.oidcRequestAuther.AuthenticateRequest(req) if err != nil { // Since we have failed OIDC auth, we will try a token review, if enabled. + p.metrics.IncrementOIDCAuthCount(false, remoteAddr, "") tokenReviewHandler.ServeHTTP(rw, req) return } // Failed authorization if !ok { + p.metrics.IncrementOIDCAuthCount(false, remoteAddr, "") p.handleError(rw, req, errUnauthorized) return } - var remoteAddr string - req, remoteAddr = context.RemoteAddr(req) - klog.V(4).Infof("authenticated request: %s", remoteAddr) // Add the user info to the request context req = req.WithContext(genericapirequest.WithUser(req.Context(), info.User)) + p.metrics.IncrementOIDCAuthCount(true, remoteAddr, info.User.GetName()) handler.ServeHTTP(rw, req) }) } @@ -89,8 +110,7 @@ func (p *Proxy) withImpersonateRequest(handler http.Handler) http.Handler { return } - var remoteAddr string - req, remoteAddr = context.RemoteAddr(req) + req, remoteAddr := context.RemoteAddr(req) // If we have disabled impersonation we can forward the request right away if p.config.DisableImpersonation { @@ -163,14 +183,35 @@ func (p *Proxy) withImpersonateRequest(handler http.Handler) http.Handler { }) } +// withClientTimestamp adds the current timestamp for the client request to the +// request context. +func (p *Proxy) withClientTimestamp(handler http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + req = context.WithClientRequestTimestamp(req) + handler.ServeHTTP(rw, req) + }) +} + // newErrorHandler returns a handler failed requests. -func (p *Proxy) newErrorHandler() func(rw http.ResponseWriter, r *http.Request, err error) { - unauthedHandler := audit.NewUnauthenticatedHandler(p.auditor, func(rw http.ResponseWriter, r *http.Request) { - klog.V(2).Infof("unauthenticated user request %s", r.RemoteAddr) +func (p *Proxy) newErrorHandler() func(rw http.ResponseWriter, req *http.Request, err error) { + + // Setup unauthed handler so that it is passed through the audit + unauthedHandler := audit.NewUnauthenticatedHandler(p.auditor, func(rw http.ResponseWriter, req *http.Request) { + _, remoteAddr := context.RemoteAddr(req) + klog.V(2).Infof("unauthenticated user request %s", remoteAddr) http.Error(rw, "Unauthorized", http.StatusUnauthorized) }) - return func(rw http.ResponseWriter, r *http.Request, err error) { + return func(rw http.ResponseWriter, req *http.Request, err error) { + var statusCode int + req, remoteAddr := context.RemoteAddr(req) + + // Update client duration metrics from error + defer func() { + clientDuration := context.ClientRequestTimestamp(req) + p.metrics.ObserveClient(statusCode, req.URL.Path, remoteAddr, time.Since(clientDuration)) + }() + if err == nil { klog.Error("error was called with no error") http.Error(rw, "", http.StatusInternalServerError) @@ -182,31 +223,36 @@ func (p *Proxy) newErrorHandler() func(rw http.ResponseWriter, r *http.Request, // Failed auth case errUnauthorized: // If Unauthorized then error and report to audit - unauthedHandler.ServeHTTP(rw, r) + statusCode = http.StatusUnauthorized + unauthedHandler.ServeHTTP(rw, req) return // User request with impersonation case errImpersonateHeader: - klog.V(2).Infof("impersonation user request %s", r.RemoteAddr) - http.Error(rw, "Impersonation requests are disabled when using kube-oidc-proxy", http.StatusForbidden) + statusCode = http.StatusForbidden + klog.V(2).Infof("impersonation user request %s", remoteAddr) + http.Error(rw, "Impersonation requests are disabled when using kube-oidc-proxy", statusCode) return // No name given or available in oidc request case errNoName: - klog.V(2).Infof("no name available in oidc info %s", r.RemoteAddr) - http.Error(rw, "Username claim not available in OIDC Issuer response", http.StatusForbidden) + statusCode = http.StatusForbidden + klog.V(2).Infof("no name available in oidc info %s", remoteAddr) + http.Error(rw, "Username claim not available in OIDC Issuer response", statusCode) return // No impersonation configuration found in context case errNoImpersonationConfig: - klog.Errorf("if you are seeing this, there is likely a bug in the proxy (%s): %s", r.RemoteAddr, err) - http.Error(rw, "", http.StatusInternalServerError) + statusCode = http.StatusInternalServerError + klog.Errorf("if you are seeing this, there is likely a bug in the proxy (%s): %s", remoteAddr, err) + http.Error(rw, "", statusCode) return // Server or unknown error default: - klog.Errorf("unknown error (%s): %s", r.RemoteAddr, err) - http.Error(rw, "", http.StatusInternalServerError) + statusCode = http.StatusInternalServerError + klog.Errorf("unknown error (%s): %s", remoteAddr, err) + http.Error(rw, "", statusCode) } } } diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 5e4d620fd..e3f8830e4 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -2,12 +2,10 @@ package proxy import ( - "errors" "fmt" "net/http" "net/http/httputil" "net/url" - "strings" "time" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -19,28 +17,13 @@ import ( "k8s.io/klog" "github.com/jetstack/kube-oidc-proxy/cmd/app/options" + "github.com/jetstack/kube-oidc-proxy/pkg/metrics" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/audit" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/context" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/hooks" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/tokenreview" ) -const ( - UserHeaderClientIPKey = "Remote-Client-IP" -) - -var ( - errUnauthorized = errors.New("Unauthorized") - errImpersonateHeader = errors.New("Impersonate-User in header") - errNoName = errors.New("No name in OIDC info") - errNoImpersonationConfig = errors.New("No impersonation configuration in context") - - // http headers are case-insensitive - impersonateUserHeader = strings.ToLower(transport.ImpersonateUserHeader) - impersonateGroupHeader = strings.ToLower(transport.ImpersonateGroupHeader) - impersonateExtraHeader = strings.ToLower(transport.ImpersonateUserExtraHeaderPrefix) -) - type Config struct { DisableImpersonation bool TokenReview bool @@ -68,6 +51,7 @@ type Proxy struct { config *Config hooks *hooks.Hooks + metrics *metrics.Metrics handleError errorHandlerFn } @@ -76,6 +60,8 @@ func New(restConfig *rest.Config, auditOptions *options.AuditOptions, tokenReviewer *tokenreview.TokenReview, ssinfo *server.SecureServingInfo, + hooks *hooks.Hooks, + metrics *metrics.Metrics, config *Config) (*Proxy, error) { // generate tokenAuther from oidc config @@ -101,7 +87,8 @@ func New(restConfig *rest.Config, return &Proxy{ restConfig: restConfig, - hooks: hooks.New(), + hooks: hooks, + metrics: metrics, tokenReviewer: tokenReviewer, secureServingInfo: ssinfo, config: config, @@ -199,8 +186,26 @@ func (p *Proxy) RoundTrip(req *http.Request) (*http.Response, error) { // Set up impersonation request. rt := transport.NewImpersonatingRoundTripper(*conf, p.clientTransport) + req, remoteAddr := context.RemoteAddr(req) + serverDuration := time.Now() + clientDuration := context.ClientRequestTimestamp(req) + // Push request through round trippers to the API server. - return rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + + var statusCode int + if resp != nil { + statusCode = resp.StatusCode + } + + // If we get an error here, then the client metrics observation will happen + // at the proxy error handler. + if err == nil { + p.metrics.ObserveClient(statusCode, req.URL.Path, remoteAddr, time.Since(clientDuration)) + } + p.metrics.ObserveServer(statusCode, req.URL.Path, remoteAddr, time.Since(serverDuration)) + + return resp, err } func (p *Proxy) reviewToken(rw http.ResponseWriter, req *http.Request) bool { @@ -260,7 +265,3 @@ func (p *Proxy) roundTripperForRestConfig(config *rest.Config) (http.RoundTrippe func (p *Proxy) OIDCTokenAuthenticator() authenticator.Token { return p.tokenAuther } - -func (p *Proxy) RunPreShutdownHooks() error { - return p.hooks.RunPreShutdownHooks() -} diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 4f9acc61f..a7ba4e283 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -21,6 +21,7 @@ import ( "k8s.io/apiserver/pkg/server" "github.com/jetstack/kube-oidc-proxy/cmd/app/options" + "github.com/jetstack/kube-oidc-proxy/pkg/metrics" "github.com/jetstack/kube-oidc-proxy/pkg/mocks" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/audit" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/hooks" @@ -63,6 +64,7 @@ func (f *fakeRW) Header() http.Header { func newFakeR() *http.Request { return &http.Request{ RemoteAddr: "fakeAddr", + URL: new(url.URL), } } @@ -117,8 +119,7 @@ func (f *fakeRT) RoundTrip(h *http.Request) (*http.Response, error) { } func tryError(t *testing.T, expCode int, err error) *fakeRW { - p := new(Proxy) - p.handleError = p.newErrorHandler() + p := newTestProxy(t) frw := newFakeRW() fr := newFakeR() @@ -169,7 +170,7 @@ func TestError(t *testing.T) { } func TestHasImpersonation(t *testing.T) { - p := new(Proxy) + p := newTestProxy(t) // no impersonation headers noImpersonation := []http.Header{ @@ -269,6 +270,7 @@ func newTestProxy(t *testing.T) *fakeProxy { noAuthClientTransport: fakeRT, config: new(Config), hooks: hooks.New(), + metrics: metrics.New(), }, } @@ -425,7 +427,7 @@ func TestHandlers(t *testing.T) { expAuthToken: "fake-token", authResponse: &authResponse{ resp: &authenticator.Response{ - User: nil, + User: &user.DefaultInfo{}, }, pass: true, err: nil, diff --git a/pkg/proxy/tokenreview/tokenreview.go b/pkg/proxy/tokenreview/tokenreview.go index 4cf355b9b..b2cf20a6c 100644 --- a/pkg/proxy/tokenreview/tokenreview.go +++ b/pkg/proxy/tokenreview/tokenreview.go @@ -9,11 +9,14 @@ import ( "time" authv1 "k8s.io/api/authentication/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" clientauthv1 "k8s.io/client-go/kubernetes/typed/authentication/v1" "k8s.io/client-go/rest" + "github.com/jetstack/kube-oidc-proxy/pkg/metrics" + proxycontext "github.com/jetstack/kube-oidc-proxy/pkg/proxy/context" "github.com/jetstack/kube-oidc-proxy/pkg/util" ) @@ -23,10 +26,11 @@ var ( type TokenReview struct { reviewRequester clientauthv1.TokenReviewInterface + metrics *metrics.Metrics audiences []string } -func New(restConfig *rest.Config, audiences []string) (*TokenReview, error) { +func New(restConfig *rest.Config, metrics *metrics.Metrics, audiences []string) (*TokenReview, error) { kubeclient, err := kubernetes.NewForConfig(restConfig) if err != nil { return nil, err @@ -34,32 +38,61 @@ func New(restConfig *rest.Config, audiences []string) (*TokenReview, error) { return &TokenReview{ reviewRequester: kubeclient.AuthenticationV1().TokenReviews(), + metrics: metrics, audiences: audiences, }, nil } func (t *TokenReview) Review(req *http.Request) (bool, error) { + var ( + code int + user string + authenticated bool + err error + ) + + // Start clock on metrics + tokenReviewDuration := time.Now() + req, remoteAddr := proxycontext.RemoteAddr(req) + token, ok := util.ParseTokenFromRequest(req) if !ok { return false, errors.New("bearer token not found in request") } - review := t.buildReview(token) + // Setup metrics observation on defer + defer func() { + if err != nil { + if status := apierrors.APIStatus(nil); errors.As(err, &status) { + code = int(status.Status().Code) + } + } + + t.metrics.ObserveTokenReivewLookup(authenticated, code, remoteAddr, user, time.Since(tokenReviewDuration)) + }() + ctx, cancel := context.WithTimeout(req.Context(), timeout) defer cancel() - resp, err := t.reviewRequester.Create(ctx, review, metav1.CreateOptions{}) + var resp *authv1.TokenReview + resp, err = t.reviewRequester.Create(ctx, review, metav1.CreateOptions{}) if err != nil { return false, err } + // Since no error to the API server for token review, we have 200 response + // code. + code = 200 + user = resp.Status.User.Username + authenticated = resp.Status.Authenticated + if len(resp.Status.Error) > 0 { return false, fmt.Errorf("error authenticating using token review: %s", resp.Status.Error) } - return resp.Status.Authenticated, nil + return authenticated, nil } func (t *TokenReview) buildReview(token string) *authv1.TokenReview { diff --git a/pkg/proxy/tokenreview/tokenreview_test.go b/pkg/proxy/tokenreview/tokenreview_test.go index a36666f1e..f64f91a60 100644 --- a/pkg/proxy/tokenreview/tokenreview_test.go +++ b/pkg/proxy/tokenreview/tokenreview_test.go @@ -9,6 +9,7 @@ import ( authv1 "k8s.io/api/authentication/v1" + "github.com/jetstack/kube-oidc-proxy/pkg/metrics" "github.com/jetstack/kube-oidc-proxy/pkg/proxy/tokenreview/fake" ) @@ -74,6 +75,7 @@ func TestReview(t *testing.T) { func runTest(t *testing.T, test testT) { tReviewer := &TokenReview{ audiences: nil, + metrics: metrics.New(), reviewRequester: fake.New().WithCreate(test.reviewResp, test.errResp), } diff --git a/test/e2e/suite/cases/impersonation/impersonation.go b/test/e2e/suite/cases/impersonation/impersonation.go index 70dd5616d..d97998409 100644 --- a/test/e2e/suite/cases/impersonation/impersonation.go +++ b/test/e2e/suite/cases/impersonation/impersonation.go @@ -72,7 +72,7 @@ var _ = framework.CasesDescribe("Impersonation", func() { By("Creating ClusterRole for system:anonymous to impersonate") roleImpersonate, err := f.Helper().KubeClient.RbacV1().ClusterRoles().Create(context.TODO(), &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("test-user-role-impersonate-"), + GenerateName: "test-user-role-impersonate-", }, Rules: []rbacv1.PolicyRule{ {APIGroups: []string{""}, Resources: []string{"users"}, Verbs: []string{"impersonate"}}, @@ -83,7 +83,7 @@ var _ = framework.CasesDescribe("Impersonation", func() { By("Creating Role for user foo to list Pods") rolePods, err := f.Helper().KubeClient.RbacV1().Roles(f.Namespace.Name).Create(context.TODO(), &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("test-user-role-pods-"), + GenerateName: "test-user-role-pods-", }, Rules: []rbacv1.PolicyRule{ {APIGroups: []string{""}, Resources: []string{"pods"}, Verbs: []string{"get", "list"}},