From 9c589ae111156b479c9b6fd2d5be8251f9efeace Mon Sep 17 00:00:00 2001 From: Ali Syed Date: Thu, 13 Aug 2026 11:13:33 +0100 Subject: [PATCH] UPSTREAM: : OCPBUGS-90506: add secure metrics serving with TokenReview/SAR authentication --- main.go | 6 +- metrics_openshift.go | 116 ++++++++++++++++++ metrics_openshift_test.go | 219 ++++++++++++++++++++++++++++++++++ pkg/apis/externaldns/types.go | 2 + 4 files changed, 342 insertions(+), 1 deletion(-) create mode 100644 metrics_openshift.go create mode 100644 metrics_openshift_test.go diff --git a/main.go b/main.go index 96e79c7400..094b3fdd4c 100644 --- a/main.go +++ b/main.go @@ -117,7 +117,11 @@ func main() { ctx, cancel := context.WithCancel(context.Background()) - go serveMetrics(cfg.MetricsAddress) + if cfg.MetricsTLSCertDir != "" { + go serveSecureMetrics(cfg) + } else { + go serveMetrics(cfg.MetricsAddress) + } go handleSigterm(cancel) // error is explicitly ignored because the filter is already validated in validation.ValidateConfig diff --git a/metrics_openshift.go b/metrics_openshift.go new file mode 100644 index 0000000000..885650db12 --- /dev/null +++ b/metrics_openshift.go @@ -0,0 +1,116 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "net/http" + "path/filepath" + "strings" + + "github.com/prometheus/client_golang/prometheus/promhttp" + log "github.com/sirupsen/logrus" + authenticationv1 "k8s.io/api/authentication/v1" + authorizationv1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + + "sigs.k8s.io/external-dns/pkg/apis/externaldns" +) + +// serveSecureMetrics starts an HTTPS metrics server with Kubernetes TokenReview/SAR authentication. +func serveSecureMetrics(cfg *externaldns.Config) { + restCfg, err := clientcmd.BuildConfigFromFlags(cfg.APIServerURL, cfg.KubeConfig) + if err != nil { + restCfg, err = rest.InClusterConfig() + if err != nil { + log.Fatalf("metrics auth: failed to build rest config: %v", err) + } + } + httpClient, err := rest.HTTPClientFor(restCfg) + if err != nil { + log.Fatalf("metrics auth: failed to build http client: %v", err) + } + kubeClient, err := kubernetes.NewForConfigAndClient(restCfg, httpClient) + if err != nil { + log.Fatalf("metrics auth: failed to build kube client: %v", err) + } + + server := &http.Server{Addr: cfg.MetricsAddress, Handler: newMetricsMux(kubeClient, cfg.MetricsTLSCertDir)} + log.Fatal(server.ListenAndServeTLS( + filepath.Join(cfg.MetricsTLSCertDir, "tls.crt"), + filepath.Join(cfg.MetricsTLSCertDir, "tls.key"), + )) +} + +// newMetricsMux builds the ServeMux for the secure metrics server. +// /metrics is wrapped with withMetricsAuth; /healthz is always unauthenticated. +func newMetricsMux(kubeClient kubernetes.Interface, tlsCertDir string) *http.ServeMux { + mux := http.NewServeMux() + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("OK")) + }) + mux.Handle("/metrics", withMetricsAuth(kubeClient, promhttp.Handler())) + return mux +} + +// withMetricsAuth authenticates requests via TokenReview and authorizes via SubjectAccessReview. +func withMetricsAuth(kubeClient kubernetes.Interface, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader := r.Header.Get("Authorization") + if !strings.HasPrefix(authHeader, "Bearer ") { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + token := strings.TrimPrefix(authHeader, "Bearer ") + + tr, err := kubeClient.AuthenticationV1().TokenReviews().Create( + r.Context(), + &authenticationv1.TokenReview{ + Spec: authenticationv1.TokenReviewSpec{Token: token}, + }, + metav1.CreateOptions{}, + ) + if err != nil || !tr.Status.Authenticated { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + sar, err := kubeClient.AuthorizationV1().SubjectAccessReviews().Create( + r.Context(), + &authorizationv1.SubjectAccessReview{ + Spec: authorizationv1.SubjectAccessReviewSpec{ + User: tr.Status.User.Username, + Groups: tr.Status.User.Groups, + NonResourceAttributes: &authorizationv1.NonResourceAttributes{ + Path: "/metrics", + Verb: "get", + }, + }, + }, + metav1.CreateOptions{}, + ) + if err != nil || !sar.Status.Allowed { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/metrics_openshift_test.go b/metrics_openshift_test.go new file mode 100644 index 0000000000..e2fc84a862 --- /dev/null +++ b/metrics_openshift_test.go @@ -0,0 +1,219 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + authenticationv1 "k8s.io/api/authentication/v1" + authorizationv1 "k8s.io/api/authorization/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func TestWithMetricsAuth(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + title string + authHeader string + tokenReview *authenticationv1.TokenReview + tokenReviewErr error + sar *authorizationv1.SubjectAccessReview + sarErr error + wantStatus int + wantNextCalled bool + }{ + { + title: "no authorization header yields 401", + authHeader: "", + wantStatus: http.StatusUnauthorized, + wantNextCalled: false, + }, + { + title: "non-bearer scheme yields 401", + authHeader: "Basic dXNlcjpwYXNz", + wantStatus: http.StatusUnauthorized, + wantNextCalled: false, + }, + { + title: "token review API error yields 401", + authHeader: "Bearer some-token", + tokenReviewErr: fmt.Errorf("api server unavailable"), + wantStatus: http.StatusUnauthorized, + wantNextCalled: false, + }, + { + title: "unauthenticated token review yields 401", + authHeader: "Bearer bad-token", + tokenReview: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{Authenticated: false}, + }, + wantStatus: http.StatusUnauthorized, + wantNextCalled: false, + }, + { + title: "subject access review API error yields 403", + authHeader: "Bearer valid-token", + tokenReview: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{Username: "prometheus"}, + }, + }, + sarErr: fmt.Errorf("api server unavailable"), + wantStatus: http.StatusForbidden, + wantNextCalled: false, + }, + { + title: "subject access review not allowed yields 403", + authHeader: "Bearer valid-token", + tokenReview: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{Username: "prometheus"}, + }, + }, + sar: &authorizationv1.SubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{Allowed: false}, + }, + wantStatus: http.StatusForbidden, + wantNextCalled: false, + }, + { + title: "authenticated and authorized passes through to handler", + authHeader: "Bearer valid-token", + tokenReview: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{ + Username: "system:serviceaccount:monitoring:prometheus-k8s", + Groups: []string{"system:serviceaccounts", "system:serviceaccounts:monitoring"}, + }, + }, + }, + sar: &authorizationv1.SubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true}, + }, + wantStatus: http.StatusOK, + wantNextCalled: true, + }, + } { + tc := tc + t.Run(tc.title, func(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewSimpleClientset() + + if tc.tokenReviewErr != nil { + fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, tc.tokenReviewErr + }) + } else if tc.tokenReview != nil { + fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, tc.tokenReview, nil + }) + } + + if tc.sarErr != nil { + fakeClient.PrependReactor("create", "subjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, tc.sarErr + }) + } else if tc.sar != nil { + fakeClient.PrependReactor("create", "subjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, tc.sar, nil + }) + } + + nextCalled := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextCalled = true + w.WriteHeader(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/metrics", nil) + if tc.authHeader != "" { + req.Header.Set("Authorization", tc.authHeader) + } + rec := httptest.NewRecorder() + + withMetricsAuth(fakeClient, next).ServeHTTP(rec, req) + + assert.Equal(t, tc.wantStatus, rec.Code) + assert.Equal(t, tc.wantNextCalled, nextCalled) + }) + } +} + +// TestWithMetricsAuthSARPayload verifies that the user identity and NonResourceAttributes +// from the TokenReview response are forwarded correctly to the SubjectAccessReview. +func TestWithMetricsAuthSARPayload(t *testing.T) { + t.Parallel() + + expectedUser := "system:serviceaccount:monitoring:prometheus-k8s" + expectedGroups := []string{"system:serviceaccounts", "system:serviceaccounts:monitoring"} + + fakeClient := fake.NewSimpleClientset() + fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{ + Username: expectedUser, + Groups: expectedGroups, + }, + }, + }, nil + }) + fakeClient.PrependReactor("create", "subjectaccessreviews", func(action k8stesting.Action) (bool, runtime.Object, error) { + sar := action.(k8stesting.CreateAction).GetObject().(*authorizationv1.SubjectAccessReview) + assert.Equal(t, expectedUser, sar.Spec.User) + assert.Equal(t, expectedGroups, sar.Spec.Groups) + assert.NotNil(t, sar.Spec.NonResourceAttributes) + assert.Equal(t, "/metrics", sar.Spec.NonResourceAttributes.Path) + assert.Equal(t, "get", sar.Spec.NonResourceAttributes.Verb) + return true, &authorizationv1.SubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{Allowed: true}, + }, nil + }) + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) + req := httptest.NewRequest(http.MethodGet, "/metrics", nil) + req.Header.Set("Authorization", "Bearer some-token") + rec := httptest.NewRecorder() + + withMetricsAuth(fakeClient, next).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) +} + +func TestNewMetricsMux(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewSimpleClientset() + mux := newMetricsMux(fakeClient, "/some/cert/dir") + + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/healthz", nil)) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "OK", rec.Body.String()) +} diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 26b579f829..d337d102bf 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -164,6 +164,7 @@ type Config struct { UpdateEvents bool LogFormat string MetricsAddress string + MetricsTLSCertDir string LogLevel string TXTCacheInterval time.Duration TXTWildcardReplacement string @@ -636,6 +637,7 @@ func (cfg *Config) ParseFlags(args []string) error { // Miscellaneous flags app.Flag("log-format", "The format in which log messages are printed (default: text, options: text, json)").Default(defaultConfig.LogFormat).EnumVar(&cfg.LogFormat, "text", "json") app.Flag("metrics-address", "Specify where to serve the metrics and health check endpoint (default: :7979)").Default(defaultConfig.MetricsAddress).StringVar(&cfg.MetricsAddress) + app.Flag("metrics-tls-cert-dir", "Directory containing tls.crt and tls.key for serving metrics over HTTPS with TokenReview/SAR authentication. If empty, metrics are served over plain HTTP (default: \"\")").Default("").StringVar(&cfg.MetricsTLSCertDir) app.Flag("log-level", "Set the level of logging. (default: info, options: panic, debug, info, warning, error, fatal)").Default(defaultConfig.LogLevel).EnumVar(&cfg.LogLevel, allLogLevelsAsStrings()...) // Webhook provider