From f9bef825d68fc1b85a314a7c42db829a9391cae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Wed, 4 Dec 2024 15:44:45 +0100 Subject: [PATCH 1/2] featuregate UID in RequestHeader authenticator Kubernetes-commit: a051b067cdffc92fbe40bcc5a8e8f1bf974348c4 --- pkg/apiserver/handler_proxy.go | 9 ++- pkg/apiserver/handler_proxy_test.go | 99 ++++++++++++++++++++++++++--- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/pkg/apiserver/handler_proxy.go b/pkg/apiserver/handler_proxy.go index 5292ec86..d95a271a 100644 --- a/pkg/apiserver/handler_proxy.go +++ b/pkg/apiserver/handler_proxy.go @@ -159,7 +159,12 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { proxyRoundTripper := handlingInfo.proxyRoundTripper upgrade := httpstream.IsUpgradeRequest(req) - proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), user.GetUID(), user.GetGroups(), user.GetExtra(), proxyRoundTripper) + var userUID string + if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.RemoteRequestHeaderUID) { + userUID = user.GetUID() + } + + proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), userUID, user.GetGroups(), user.GetExtra(), proxyRoundTripper) if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.APIServerTracing) && !upgrade { tracingWrapper := tracing.WrapperFor(r.tracerProvider) @@ -170,7 +175,7 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // NOT use the proxyRoundTripper. It's a direct dial that bypasses the proxyRoundTripper. This means that we have to // attach the "correct" user headers to the request ahead of time. if upgrade { - transport.SetAuthProxyHeaders(newReq, user.GetName(), user.GetUID(), user.GetGroups(), user.GetExtra()) + transport.SetAuthProxyHeaders(newReq, user.GetName(), userUID, user.GetGroups(), user.GetExtra()) } handler := proxy.NewUpgradeAwareHandler(location, proxyRoundTripper, true, upgrade, &responder{w: w}) diff --git a/pkg/apiserver/handler_proxy_test.go b/pkg/apiserver/handler_proxy_test.go index 63272701..ecc85c7e 100644 --- a/pkg/apiserver/handler_proxy_test.go +++ b/pkg/apiserver/handler_proxy_test.go @@ -34,7 +34,9 @@ import ( "sync/atomic" "testing" + "github.com/google/go-cmp/cmp" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/client-go/transport" @@ -53,8 +55,11 @@ import ( "k8s.io/apiserver/pkg/endpoints/filters" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/server/egressselector" + utilfeature "k8s.io/apiserver/pkg/util/feature" utilflowcontrol "k8s.io/apiserver/pkg/util/flowcontrol" apiserverproxyutil "k8s.io/apiserver/pkg/util/proxy" + "k8s.io/component-base/featuregate" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" apiregistration "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" @@ -130,6 +135,8 @@ func TestProxyHandler(t *testing.T) { expectedBody string expectedCalled bool expectedHeaders map[string][]string + + enableFeatureGates []featuregate.Feature }{ "no target": { expectedStatusCode: http.StatusNotFound, @@ -174,6 +181,40 @@ func TestProxyHandler(t *testing.T) { }, expectedStatusCode: http.StatusOK, expectedCalled: true, + expectedHeaders: map[string][]string{ + "X-Forwarded-Proto": {"https"}, + "X-Forwarded-Uri": {"/request/path"}, + "X-Forwarded-For": {"127.0.0.1"}, + "X-Remote-User": {"username"}, + "User-Agent": {"Go-http-client/1.1"}, + "Accept-Encoding": {"gzip"}, + "X-Remote-Group": {"one", "two"}, + }, + }, + "[RemoteRequestHeaderUID] proxy with user, insecure": { + user: &user.DefaultInfo{ + Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", + Groups: []string{"one", "two"}, + }, + path: "/request/path", + apiService: &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, + Spec: apiregistration.APIServiceSpec{ + Service: &apiregistration.ServiceReference{Port: pointer.Int32Ptr(443)}, + Group: "foo", + Version: "v1", + InsecureSkipTLSVerify: true, + }, + Status: apiregistration.APIServiceStatus{ + Conditions: []apiregistration.APIServiceCondition{ + {Type: apiregistration.Available, Status: apiregistration.ConditionTrue}, + }, + }, + }, + enableFeatureGates: []featuregate.Feature{features.RemoteRequestHeaderUID}, + expectedStatusCode: http.StatusOK, + expectedCalled: true, expectedHeaders: map[string][]string{ "X-Forwarded-Proto": {"https"}, "X-Forwarded-Uri": {"/request/path"}, @@ -208,6 +249,40 @@ func TestProxyHandler(t *testing.T) { }, expectedStatusCode: http.StatusOK, expectedCalled: true, + expectedHeaders: map[string][]string{ + "X-Forwarded-Proto": {"https"}, + "X-Forwarded-Uri": {"/request/path"}, + "X-Forwarded-For": {"127.0.0.1"}, + "X-Remote-User": {"username"}, + "User-Agent": {"Go-http-client/1.1"}, + "Accept-Encoding": {"gzip"}, + "X-Remote-Group": {"one", "two"}, + }, + }, + "[RemoteRequestHeaderUID] proxy with user, cabundle": { + user: &user.DefaultInfo{ + Name: "username", + UID: "6b60d791-1af9-4513-92e5-e4252a1e0a78", + Groups: []string{"one", "two"}, + }, + path: "/request/path", + apiService: &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, + Spec: apiregistration.APIServiceSpec{ + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Group: "foo", + Version: "v1", + CABundle: testCACrt, + }, + Status: apiregistration.APIServiceStatus{ + Conditions: []apiregistration.APIServiceCondition{ + {Type: apiregistration.Available, Status: apiregistration.ConditionTrue}, + }, + }, + }, + enableFeatureGates: []featuregate.Feature{features.RemoteRequestHeaderUID}, + expectedStatusCode: http.StatusOK, + expectedCalled: true, expectedHeaders: map[string][]string{ "X-Forwarded-Proto": {"https"}, "X-Forwarded-Uri": {"/request/path"}, @@ -320,7 +395,11 @@ func TestProxyHandler(t *testing.T) { target.Reset() legacyregistry.Reset() - func() { + t.Run(name, func(t *testing.T) { + for _, f := range tc.enableFeatureGates { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true) + } + targetServer := httptest.NewUnstartedServer(target) serviceCert := tc.serviceCertOverride if serviceCert == nil { @@ -354,37 +433,37 @@ func TestProxyHandler(t *testing.T) { resp, err := http.Get(server.URL + tc.path) if err != nil { - t.Errorf("%s: %v", name, err) + t.Errorf("%v", err) return } if e, a := tc.expectedStatusCode, resp.StatusCode; e != a { body, _ := httputil.DumpResponse(resp, true) - t.Logf("%s: %v", name, string(body)) - t.Errorf("%s: expected %v, got %v", name, e, a) + t.Logf("%v", string(body)) + t.Errorf("expected %v, got %v", e, a) return } bytes, err := io.ReadAll(resp.Body) if err != nil { - t.Errorf("%s: %v", name, err) + t.Errorf("%v", err) return } if !strings.Contains(string(bytes), tc.expectedBody) { - t.Errorf("%s: expected %q, got %q", name, tc.expectedBody, string(bytes)) + t.Errorf("expected %q, got %q", tc.expectedBody, string(bytes)) return } if e, a := tc.expectedCalled, target.called; e != a { - t.Errorf("%s: expected %v, got %v", name, e, a) + t.Errorf("expected %v, got %v", e, a) return } // this varies every test delete(target.headers, "X-Forwarded-Host") if e, a := tc.expectedHeaders, target.headers; !reflect.DeepEqual(e, a) { - t.Errorf("%s: expected %v, got %v", name, e, a) + t.Errorf("expected != got %v", cmp.Diff(e, a)) return } if e, a := targetServer.Listener.Addr().String(), target.host; tc.expectedCalled && !reflect.DeepEqual(e, a) { - t.Errorf("%s: expected %v, got %v", name, e, a) + t.Errorf("expected %v, got %v", e, a) return } @@ -397,7 +476,7 @@ func TestProxyHandler(t *testing.T) { t.Errorf("expected the x509_missing_san_total to be 1, but it's %d", errorCounter) } } - }() + }) } } From d2837ca2994f9a9239dd9e38ecb786ae3818fda6 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 4 Dec 2024 16:04:36 -0500 Subject: [PATCH 2/2] Update tests to handle RemoteRequestHeaderUID Signed-off-by: Monis Khan Kubernetes-commit: 779d76176afe96f7a8b83c14cb6370650c9464a4 --- pkg/apiserver/handler_proxy_test.go | 46 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/apiserver/handler_proxy_test.go b/pkg/apiserver/handler_proxy_test.go index ecc85c7e..a19cdd22 100644 --- a/pkg/apiserver/handler_proxy_test.go +++ b/pkg/apiserver/handler_proxy_test.go @@ -35,35 +35,33 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "k8s.io/apiserver/pkg/audit" - "k8s.io/apiserver/pkg/features" - "k8s.io/apiserver/pkg/server/dynamiccertificates" - "k8s.io/client-go/transport" - - "golang.org/x/net/websocket" - "go.opentelemetry.io/otel/propagation" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" + "golang.org/x/net/websocket" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/proxy" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/filters" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/features" + "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/apiserver/pkg/server/egressselector" utilfeature "k8s.io/apiserver/pkg/util/feature" utilflowcontrol "k8s.io/apiserver/pkg/util/flowcontrol" apiserverproxyutil "k8s.io/apiserver/pkg/util/proxy" + "k8s.io/client-go/transport" "k8s.io/component-base/featuregate" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" apiregistration "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" - "k8s.io/utils/pointer" "k8s.io/utils/ptr" ) @@ -145,7 +143,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", }, @@ -168,7 +166,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", InsecureSkipTLSVerify: true, @@ -201,7 +199,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", InsecureSkipTLSVerify: true, @@ -236,7 +234,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: testCACrt, @@ -269,7 +267,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: testCACrt, @@ -304,7 +302,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: testCACrt, @@ -328,7 +326,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "bad-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "bad-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: testCACrt, @@ -351,7 +349,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", }, @@ -373,7 +371,7 @@ func TestProxyHandler(t *testing.T) { apiService: &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: testCACrt, @@ -511,6 +509,8 @@ func newBrokenDialerAndSelector() (*mockEgressDialer, *egressselector.EgressSele } func TestProxyUpgrade(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoteRequestHeaderUID, true) + upgradeUser := "upgradeUser" upgradeUID := "upgradeUser-UID" testcases := map[string]struct { @@ -525,7 +525,7 @@ func TestProxyUpgrade(t *testing.T) { CABundle: testCACrt, Group: "mygroup", Version: "v1", - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -542,7 +542,7 @@ func TestProxyUpgrade(t *testing.T) { InsecureSkipTLSVerify: true, Group: "mygroup", Version: "v1", - Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns", Port: ptr.To[int32](443)}, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -559,7 +559,7 @@ func TestProxyUpgrade(t *testing.T) { CABundle: testCACrt, Group: "mygroup", Version: "v1", - Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns", Port: ptr.To[int32](443)}, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -576,7 +576,7 @@ func TestProxyUpgrade(t *testing.T) { CABundle: testCACrt, Group: "mygroup", Version: "v1", - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -594,7 +594,7 @@ func TestProxyUpgrade(t *testing.T) { CABundle: testCACrt, Group: "mygroup", Version: "v1", - Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns", Port: ptr.To[int32](443)}, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -1075,7 +1075,7 @@ func TestProxyCertReload(t *testing.T) { apiService := &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, Spec: apiregistration.APIServiceSpec{ - Service: &apiregistration.ServiceReference{Name: "test-service2", Namespace: "test-ns", Port: pointer.Int32Ptr(443)}, + Service: &apiregistration.ServiceReference{Name: "test-service2", Namespace: "test-ns", Port: ptr.To[int32](443)}, Group: "foo", Version: "v1", CABundle: backendCaCertificate(), // used to validate backendCertificate()