Skip to content

Commit f764e41

Browse files
committed
move filter to separate package
1 parent 66ae79b commit f764e41

File tree

6 files changed

+181
-126
lines changed

6 files changed

+181
-126
lines changed

pkg/manager/internal.go

Lines changed: 7 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ import (
3232
"k8s.io/apimachinery/pkg/api/meta"
3333
"k8s.io/apimachinery/pkg/runtime"
3434
kerrors "k8s.io/apimachinery/pkg/util/errors"
35-
"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
36-
"k8s.io/apiserver/pkg/authorization/authorizer"
37-
"k8s.io/apiserver/pkg/authorization/authorizerfactory"
38-
"k8s.io/apiserver/pkg/server/options"
39-
authenticationv1 "k8s.io/client-go/kubernetes/typed/authentication/v1"
40-
authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
4135
"k8s.io/client-go/rest"
4236
"k8s.io/client-go/tools/leaderelection"
4337
"k8s.io/client-go/tools/leaderelection/resourcelock"
@@ -93,19 +87,14 @@ type controllerManager struct {
9387
// metricsListener is used to serve prometheus metrics
9488
metricsListener net.Listener
9589

96-
// metricsSecureServing enables secure metrics serving.
97-
// This means metrics will be served via https and with authentication and authorization.
98-
metricsSecureServing bool
90+
// metricsFilter is a func that is added before the metrics handler on the metrics server.
91+
// This can be e.g. used to enforce authentication and authorization on the metrics
92+
// endpoint.
93+
metricsFilter metrics.Filter
9994

10095
// metricsExtraHandlers contains extra handlers to register on http server that serves metrics.
10196
metricsExtraHandlers map[string]http.Handler
10297

103-
// metricsAuthenticationClient is the client used to authenticate requests to the metrics endpoint.
104-
metricsAuthenticationClient authenticationv1.AuthenticationV1Interface
105-
106-
// metricsAuthorizationClient is the client used to authorize requests to the metrics endpoint.
107-
metricsAuthorizationClient authorizationv1.AuthorizationV1Interface
108-
10998
// healthProbeListener is used to serve liveness probe
11099
healthProbeListener net.Listener
111100

@@ -322,11 +311,11 @@ func (cm *controllerManager) addMetricsServer() error {
322311

323312
log := cm.logger.WithValues("path", defaultMetricsEndpoint)
324313

325-
if cm.metricsSecureServing {
314+
if cm.metricsFilter != nil {
326315
var err error
327-
handler, err = withAuthenticationAndAuthorization(log, cm.metricsAuthenticationClient, cm.metricsAuthorizationClient, handler)
316+
handler, err = cm.metricsFilter(log, handler)
328317
if err != nil {
329-
return fmt.Errorf("failed to add metrics server: %w", err)
318+
return fmt.Errorf("failed to add metrics server: failed to add metrics filter %w", err)
330319
}
331320
}
332321

@@ -344,74 +333,6 @@ func (cm *controllerManager) addMetricsServer() error {
344333
})
345334
}
346335

347-
func withAuthenticationAndAuthorization(log logr.Logger, authenticationClient authenticationv1.AuthenticationV1Interface, authorizationClient authorizationv1.AuthorizationV1Interface, handler http.Handler) (http.Handler, error) {
348-
authenticatorConfig := authenticatorfactory.DelegatingAuthenticatorConfig{
349-
Anonymous: false, // Require authentication.
350-
CacheTTL: 1 * time.Minute,
351-
TokenAccessReviewClient: authenticationClient,
352-
TokenAccessReviewTimeout: 10 * time.Second,
353-
WebhookRetryBackoff: options.DefaultAuthWebhookRetryBackoff(),
354-
}
355-
delegatingAuthenticator, _, err := authenticatorConfig.New()
356-
if err != nil {
357-
return nil, fmt.Errorf("failed to create authenticator: %w", err)
358-
}
359-
360-
authorizerConfig := authorizerfactory.DelegatingAuthorizerConfig{
361-
SubjectAccessReviewClient: authorizationClient,
362-
AllowCacheTTL: 5 * time.Minute,
363-
DenyCacheTTL: 30 * time.Second,
364-
WebhookRetryBackoff: options.DefaultAuthWebhookRetryBackoff(),
365-
}
366-
delegatingAuthorizer, err := authorizerConfig.New()
367-
if err != nil {
368-
return nil, fmt.Errorf("failed to create authorizer: %w", err)
369-
}
370-
371-
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
372-
if req.Method != http.MethodGet {
373-
http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed)
374-
return
375-
}
376-
377-
ctx := req.Context()
378-
379-
res, ok, err := delegatingAuthenticator.AuthenticateRequest(req)
380-
if err != nil {
381-
log.Error(err, "Authentication failed", err)
382-
http.Error(w, "Authentication failed", http.StatusInternalServerError)
383-
return
384-
}
385-
if !ok {
386-
log.V(4).Info("Authentication failed")
387-
http.Error(w, "Unauthorized", http.StatusUnauthorized)
388-
return
389-
}
390-
391-
attributes := authorizer.AttributesRecord{
392-
User: res.User,
393-
Verb: "get",
394-
Path: req.URL.Path,
395-
}
396-
397-
authorized, reason, err := delegatingAuthorizer.Authorize(ctx, attributes)
398-
if err != nil {
399-
msg := fmt.Sprintf("Authorization for user %s failed", attributes.User.GetName())
400-
log.Error(err, msg)
401-
http.Error(w, msg, http.StatusInternalServerError)
402-
return
403-
}
404-
if authorized != authorizer.DecisionAllow {
405-
msg := fmt.Sprintf("Authorization denied for user %s", attributes.User.GetName())
406-
log.V(4).Info(fmt.Sprintf("%s: %s", msg, reason))
407-
http.Error(w, msg, http.StatusForbidden)
408-
return
409-
}
410-
411-
handler.ServeHTTP(w, req)
412-
}), nil
413-
}
414-
415336
func (cm *controllerManager) serveHealthProbes() {
416337
mux := http.NewServeMux()
417338
server := httpserver.New(mux)

pkg/manager/manager.go

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ import (
2929
"k8s.io/apimachinery/pkg/api/meta"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/apimachinery/pkg/runtime"
32-
"k8s.io/client-go/kubernetes"
33-
authenticationv1 "k8s.io/client-go/kubernetes/typed/authentication/v1"
34-
authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
3532
"k8s.io/client-go/rest"
3633
"k8s.io/client-go/tools/leaderelection/resourcelock"
3734
"k8s.io/client-go/tools/record"
@@ -263,37 +260,19 @@ type Options struct {
263260
// for serving prometheus metrics.
264261
// It can be set to "0" to disable the metrics serving.
265262
//
266-
// Per default metrics will be served via http and without authentication and authorization.
267-
// If MetricsSecureServing is enabled metrics will be served via https and authenticated (via TokenReviews)
268-
// and authorized (via SubjectAccessReviews) with the kube-apiserver.
263+
// Per default metrics will be served via http. If MetricsSecureServing is enabled
264+
// metrics will be served via https.
269265
MetricsBindAddress string
270266

271-
// MetricsSecureServing enables secure metrics serving.
272-
// This means metrics will be served via https and authenticated (via TokenReviews)
273-
// and authorized (via SubjectAccessReviews) with the kube-apiserver.
274-
//
275-
// For the authentication and authorization the controller needs a ClusterRole
276-
// with the following rules:
277-
// - apiGroups:
278-
// - authentication.k8s.io
279-
// resources:
280-
// - tokenreviews
281-
// verbs:
282-
// - create
283-
// - apiGroups:
284-
// - authorization.k8s.io
285-
// resources:
286-
// - subjectaccessreviews
287-
// verbs:
288-
// - create
289-
// To scrape metrics e.g. via Prometheus the client needs a ClusterRole
290-
// with the following rule:
291-
// - nonResourceURLs:
292-
// - "/metrics"
293-
// verbs:
294-
// - get
267+
// MetricsSecureServing enables serving metrics via https.
295268
MetricsSecureServing bool
296269

270+
// MetricsFilterProvider provides a metrics filter which is a func that is added before
271+
// the metrics handler on the metrics server.
272+
// This can be e.g. used to enforce authentication and authorization on the metrics
273+
// endpoint by setting this field to filters.WithAuthenticationAndAuthorization.
274+
MetricsFilterProvider func(c *rest.Config, httpClient *http.Client) (metrics.Filter, error)
275+
297276
// HealthProbeBindAddress is the TCP address that the controller should bind to
298277
// for serving health probes
299278
// It can be set to "0" or "" to disable serving the health probe.
@@ -483,15 +462,12 @@ func New(config *rest.Config, options Options) (Manager, error) {
483462
}
484463
}
485464

486-
var metricsAuthenticationClient authenticationv1.AuthenticationV1Interface
487-
var metricsAuthorizationClient authorizationv1.AuthorizationV1Interface
488-
if options.MetricsSecureServing {
489-
metricsKubeClient, err := kubernetes.NewForConfigAndClient(config, cluster.GetHTTPClient())
465+
var metricsFilter metrics.Filter
466+
if options.MetricsFilterProvider != nil {
467+
metricsFilter, err = options.MetricsFilterProvider(config, cluster.GetHTTPClient())
490468
if err != nil {
491469
return nil, err
492470
}
493-
metricsAuthenticationClient = metricsKubeClient.AuthenticationV1()
494-
metricsAuthorizationClient = metricsKubeClient.AuthorizationV1()
495471
}
496472

497473
// Create the metrics listener. This will throw an error if the metrics bind
@@ -529,10 +505,8 @@ func New(config *rest.Config, options Options) (Manager, error) {
529505
recorderProvider: recorderProvider,
530506
resourceLock: resourceLock,
531507
metricsListener: metricsListener,
532-
metricsSecureServing: options.MetricsSecureServing,
508+
metricsFilter: metricsFilter,
533509
metricsExtraHandlers: metricsExtraHandlers,
534-
metricsAuthenticationClient: metricsAuthenticationClient,
535-
metricsAuthorizationClient: metricsAuthorizationClient,
536510
controllerConfig: options.Controller,
537511
logger: options.Logger,
538512
elected: make(chan struct{}),

pkg/manager/manager_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import (
5555
"sigs.k8s.io/controller-runtime/pkg/leaderelection"
5656
fakeleaderelection "sigs.k8s.io/controller-runtime/pkg/leaderelection/fake"
5757
"sigs.k8s.io/controller-runtime/pkg/metrics"
58+
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
5859
"sigs.k8s.io/controller-runtime/pkg/recorder"
5960
"sigs.k8s.io/controller-runtime/pkg/webhook"
6061
)
@@ -1380,7 +1381,8 @@ var _ = Describe("manger.Manager", func() {
13801381
BeforeEach(func() {
13811382
listener = nil
13821383
opts = Options{
1383-
MetricsSecureServing: true,
1384+
MetricsSecureServing: true,
1385+
MetricsFilterProvider: filters.WithAuthenticationAndAuthorization,
13841386
newMetricsListener: func(addr string, secureServing bool) (net.Listener, error) {
13851387
var err error
13861388
listener, err = metrics.NewListener(addr, secureServing)
@@ -1456,6 +1458,7 @@ var _ = Describe("manger.Manager", func() {
14561458
resp, err := httpClient.Do(req)
14571459
Expect(err).NotTo(HaveOccurred())
14581460
defer resp.Body.Close()
1461+
// This is expected as the token has rights for /metrics.
14591462
Expect(resp.StatusCode).To(Equal(200))
14601463
body, err := io.ReadAll(resp.Body)
14611464
Expect(err).NotTo(HaveOccurred())
@@ -1516,6 +1519,7 @@ var _ = Describe("manger.Manager", func() {
15161519
resp, err := httpClient.Do(req)
15171520
Expect(err).NotTo(HaveOccurred())
15181521
defer resp.Body.Close()
1522+
// This is expected as the token has rights for /metrics.
15191523
Expect(resp.StatusCode).To(Equal(200))
15201524
data, err := io.ReadAll(resp.Body)
15211525
Expect(err).NotTo(HaveOccurred())
@@ -1567,6 +1571,7 @@ var _ = Describe("manger.Manager", func() {
15671571
resp, err := httpClient.Do(req)
15681572
Expect(err).NotTo(HaveOccurred())
15691573
defer resp.Body.Close()
1574+
// This is expected as the token has rights for /debug.
15701575
Expect(resp.StatusCode).To(Equal(200))
15711576
body, err := io.ReadAll(resp.Body)
15721577
Expect(err).NotTo(HaveOccurred())

pkg/metrics/filters.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
/*
18+
Package metrics contains controller related metrics utilities
19+
*/
20+
package metrics
21+
22+
import (
23+
"net/http"
24+
25+
"github.com/go-logr/logr"
26+
)
27+
28+
// Filter is a func that is added before the metrics handler on the metrics server.
29+
// This can be e.g. used to enforce authentication and authorization on the metrics
30+
// endpoint by setting this field to filters.WithAuthenticationAndAuthorization.
31+
type Filter func(log logr.Logger, handler http.Handler) (http.Handler, error)

0 commit comments

Comments
 (0)