Skip to content

Commit 46d2a1c

Browse files
authored
Merge pull request #4258 from mjudeikis/fp.metrics
Adjust FP metrics endpoint
2 parents 60b2406 + f7e2689 commit 46d2a1c

4 files changed

Lines changed: 150 additions & 9 deletions

File tree

hack/tools.checksums

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ controller-gen|GOARCH=amd64;GOOS=linux|21e5f3239666fc0c5e2d23c2a3a83fd655af40a96
22
controller-gen|GOARCH=arm64;GOOS=darwin|2ca28be7185d9279ed82e3355529b0543938f392cb812add3f25a62196ed7441
33
controller-gen|GOARCH=arm64;GOOS=linux|a1a1f758435d05933c4b2f8c292f8ab2448e81a02c45f14dbd81c10e87ec4b20
44
golangci-lint|GOARCH=amd64;GOOS=linux|e26335d9bd381a60e5769a13b0ccc7967db5b6fb9c39a896a1f6fd0befe0a661
5-
golangci-lint|GOARCH=arm64;GOOS=darwin|584ac152c81dbd2325cbf576e2274cee2e608888f4e9e5a8be42712965f866bc
5+
golangci-lint|GOARCH=arm64;GOOS=darwin|691b9100ce968ff0009b6b7757ef6a585e31ae9ab11dfe0340ebb6e8e21fdc3d
66
golangci-lint|GOARCH=arm64;GOOS=linux|66dbbe60d3e1fc6b07807be7cbe58fe9ed45afe190018fba2112870e2e30b737
77
gotestsum|GOARCH=amd64;GOOS=linux|2e505a9368568aa7422e0a90ef77acc8807c0d3272ab81c7a69e3e8688d1cf65
88
gotestsum|GOARCH=arm64;GOOS=darwin|020be8d14358c7ac4155e296436057cf4b1f1232f8f8f3d71f22a0e7a5504340

pkg/proxy/mapping.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626

2727
"sigs.k8s.io/yaml"
2828

29-
"k8s.io/component-base/metrics/legacyregistry"
3029
"k8s.io/klog/v2"
3130

3231
"github.com/kcp-dev/kcp/pkg/proxy/metrics"
@@ -54,13 +53,7 @@ func isShardMapping(m types.PathMapping) bool {
5453

5554
func NewHandler(ctx context.Context, mappings []types.PathMapping) (http.Handler, error) {
5655
handlers := proxy.HttpHandler{
57-
Mappings: types.HttpHandlerMappings{
58-
{
59-
Weight: 0,
60-
Path: "/metrics",
61-
Handler: legacyregistry.Handler(),
62-
},
63-
},
56+
Mappings: types.HttpHandlerMappings{},
6457
}
6558

6659
logger := klog.FromContext(ctx)

pkg/proxy/metrics_authz_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
Copyright 2026 The kcp 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+
package proxy
18+
19+
import (
20+
"context"
21+
"net/http"
22+
"net/http/httptest"
23+
"testing"
24+
25+
"k8s.io/apiserver/pkg/authentication/authenticator"
26+
"k8s.io/apiserver/pkg/authentication/user"
27+
"k8s.io/apiserver/pkg/authorization/authorizer"
28+
29+
"github.com/kcp-dev/kcp/pkg/server/requestinfo"
30+
)
31+
32+
// authFunc adapts a function to authenticator.Request.
33+
type authFunc func(req *http.Request) (*authenticator.Response, bool, error)
34+
35+
func (f authFunc) AuthenticateRequest(req *http.Request) (*authenticator.Response, bool, error) {
36+
return f(req)
37+
}
38+
39+
// authzFunc adapts a function to authorizer.Authorizer.
40+
type authzFunc func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error)
41+
42+
func (f authzFunc) Authorize(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
43+
return f(ctx, a)
44+
}
45+
46+
// TestWithMetricsAuthorization verifies that the /metrics guard rejects
47+
// anonymous callers (401) and authenticated-but-unauthorized callers (403), and
48+
// only invokes the wrapped handler for an authenticated, authorized caller.
49+
func TestWithMetricsAuthorization(t *testing.T) {
50+
t.Parallel()
51+
authenticated := authFunc(func(req *http.Request) (*authenticator.Response, bool, error) {
52+
return &authenticator.Response{User: &user.DefaultInfo{Name: "scraper"}}, true, nil
53+
})
54+
anonymous := authFunc(func(req *http.Request) (*authenticator.Response, bool, error) {
55+
return nil, false, nil
56+
})
57+
allow := authzFunc(func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
58+
// The guard must authorize the /metrics non-resource URL.
59+
if a.IsResourceRequest() || a.GetPath() != "/metrics" || a.GetVerb() != "get" {
60+
return authorizer.DecisionDeny, "unexpected attributes", nil
61+
}
62+
return authorizer.DecisionAllow, "", nil
63+
})
64+
deny := authzFunc(func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
65+
return authorizer.DecisionNoOpinion, "nope", nil
66+
})
67+
68+
for _, tc := range []struct {
69+
name string
70+
auth authenticator.Request
71+
authz authorizer.Authorizer
72+
wantStatus int
73+
wantServed bool
74+
}{
75+
{name: "anonymous is rejected", auth: anonymous, authz: allow, wantStatus: http.StatusUnauthorized, wantServed: false},
76+
{name: "authenticated but unauthorized is forbidden", auth: authenticated, authz: deny, wantStatus: http.StatusForbidden, wantServed: false},
77+
{name: "authenticated and authorized is served", auth: authenticated, authz: allow, wantStatus: http.StatusOK, wantServed: true},
78+
} {
79+
t.Run(tc.name, func(t *testing.T) {
80+
t.Parallel()
81+
served := false
82+
inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
83+
served = true
84+
w.WriteHeader(http.StatusOK)
85+
})
86+
87+
h := withMetricsAuthorization(inner, tc.auth, tc.authz, requestinfo.NewFactory())
88+
89+
rec := httptest.NewRecorder()
90+
h.ServeHTTP(rec, httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/metrics", http.NoBody))
91+
92+
if rec.Code != tc.wantStatus {
93+
t.Errorf("status = %d, want %d (body: %s)", rec.Code, tc.wantStatus, rec.Body.String())
94+
}
95+
if served != tc.wantServed {
96+
t.Errorf("wrapped handler served = %v, want %v", served, tc.wantServed)
97+
}
98+
})
99+
}
100+
}

pkg/proxy/server.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,29 @@ import (
2323
"slices"
2424
"time"
2525

26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/runtime"
28+
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/apimachinery/pkg/runtime/serializer"
2630
"k8s.io/apimachinery/pkg/util/wait"
31+
"k8s.io/apiserver/pkg/authentication/authenticator"
32+
"k8s.io/apiserver/pkg/authorization/authorizer"
2733
genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
34+
"k8s.io/apiserver/pkg/endpoints/request"
2835
genericfilters "k8s.io/apiserver/pkg/server/filters"
2936
"k8s.io/apiserver/pkg/server/healthz"
3037
restclient "k8s.io/client-go/rest"
38+
"k8s.io/component-base/metrics/legacyregistry"
3139
"k8s.io/klog/v2"
3240

41+
kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes"
3342
"github.com/kcp-dev/sdk/apis/core"
3443
corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
3544
kcpclientset "github.com/kcp-dev/sdk/client/clientset/versioned/cluster"
3645
kcpinformers "github.com/kcp-dev/sdk/client/informers/externalversions"
3746

3847
"github.com/kcp-dev/kcp/pkg/authentication"
48+
"github.com/kcp-dev/kcp/pkg/authorization/delegated"
3949
kcpfeatures "github.com/kcp-dev/kcp/pkg/features"
4050
frontproxyfilters "github.com/kcp-dev/kcp/pkg/proxy/filters"
4151
"github.com/kcp-dev/kcp/pkg/proxy/index"
@@ -149,12 +159,50 @@ func NewServer(ctx context.Context, c CompletedConfig) (*Server, error) {
149159
healthz.InstallReadyzHandler(mux, healthz.NewInformerSyncHealthz(s.KcpSharedInformerFactory))
150160
healthz.InstallLivezHandler(mux, healthz.PingHealthz)
151161

162+
// /metrics is served locally by the front proxy (it is not proxied to a
163+
// shard), so it needs its own authentication and authorization: without it
164+
// the endpoint is world-readable and leaks shard-wide operational data
165+
// (logical-cluster counts, client-cert expiry, request patterns). Access is
166+
// delegated to :root via SubjectAccessReview, so the same
167+
// system:kcp:metrics-reader binding that authorizes scraping on the shards
168+
// authorizes it here too. Registered before "/" so it takes precedence over
169+
// the proxy handler for this exact path.
170+
metricsHandler, err := newProtectedMetricsHandler(c.AuthenticationInfo.Authenticator, c.RootShardConfig, requestInfoFactory)
171+
if err != nil {
172+
return nil, fmt.Errorf("failed to create protected metrics handler: %w", err)
173+
}
174+
mux.Handle("/metrics", metricsHandler)
175+
152176
mux.Handle("/", handler)
153177
s.Handler = mux
154178

155179
return s, nil
156180
}
157181

182+
func newProtectedMetricsHandler(auth authenticator.Request, rootShardConfig *restclient.Config, requestInfoResolver request.RequestInfoResolver) (http.Handler, error) {
183+
kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(rootShardConfig)
184+
if err != nil {
185+
return nil, fmt.Errorf("failed to create root shard client for metrics authorization: %w", err)
186+
}
187+
authz, err := delegated.NewDelegatedAuthorizer(core.RootCluster, kubeClusterClient, delegated.Options{})
188+
if err != nil {
189+
return nil, fmt.Errorf("failed to create metrics authorizer: %w", err)
190+
}
191+
192+
return withMetricsAuthorization(legacyregistry.Handler(), auth, authz, requestInfoResolver), nil
193+
}
194+
195+
func withMetricsAuthorization(h http.Handler, auth authenticator.Request, authz authorizer.Authorizer, requestInfoResolver request.RequestInfoResolver) http.Handler {
196+
scheme := runtime.NewScheme()
197+
metav1.AddToGroupVersion(scheme, schema.GroupVersion{Version: "v1"})
198+
codecs := serializer.NewCodecFactory(scheme)
199+
200+
h = genericapifilters.WithAuthorization(h, authz, codecs)
201+
h = genericapifilters.WithAuthentication(h, auth, frontproxyfilters.NewUnauthorizedHandler(), nil, nil)
202+
h = genericapifilters.WithRequestInfo(h, requestInfoResolver)
203+
return h
204+
}
205+
158206
// preparedServer is a private wrapper that enforces a call of PrepareRun() before Run can be invoked.
159207
type preparedServer struct {
160208
*Server

0 commit comments

Comments
 (0)