Skip to content

Commit

Permalink
Merge pull request #129081 from stlaz/fg_remote_uid
Browse files Browse the repository at this point in the history 
featuregate UID in RequestHeader authenticator

Kubernetes-commit: 1504f10e7946f95a8b1da35e28e4c7453ff62775
  • Loading branch information
@k8s-publishing-bot
k8s-publishing-bot committed Dec 5, 2024
2 parents 029aa2f + d2837ca commit a701c6a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 38 deletions.
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ require (
golang.org/x/net v0.30.0
k8s.io/api v0.0.0-20241205022319-ea815d578ce0
k8s.io/apimachinery v0.0.0-20241205021851-220d7c35f6db
k8s.io/apiserver v0.0.0-20241205025540-b3c0cb68b591
k8s.io/apiserver v0.0.0-20241205044541-adee2595b30a
k8s.io/client-go v0.0.0-20241205022958-9df509924771
k8s.io/code-generator v0.0.0-20241205023959-906f6b3626c4
k8s.io/component-base v0.0.0-20241205024250-af0d53b80315
k8s.io/component-base v0.0.0-20241205044541-c7b5643e57ed
k8s.io/klog/v2 v2.130.1
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,14 +276,14 @@ k8s.io/api v0.0.0-20241205022319-ea815d578ce0 h1:aiiQwgKSiYU3xmGuF6y/qdZPRB0s3L4
k8s.io/api v0.0.0-20241205022319-ea815d578ce0/go.mod h1:GEs2lxLS/XwjQlAUWC9pZYPt+ywPxC46guJkjHDEG84=
k8s.io/apimachinery v0.0.0-20241205021851-220d7c35f6db h1:53SOJ+NqfBfvLxPkHUs0TbQjW9txj/09lxtN4G61zxY=
k8s.io/apimachinery v0.0.0-20241205021851-220d7c35f6db/go.mod h1:pfmi1Ug6+bq/azoo9WveGhYBCQ0b+Wm4IgxWGFZ7wRc=
k8s.io/apiserver v0.0.0-20241205025540-b3c0cb68b591 h1:WJB3WsuRvm0+jSOJd4r2/nHJBlJBzrGkOXKLH3Q1yhA=
k8s.io/apiserver v0.0.0-20241205025540-b3c0cb68b591/go.mod h1:L4U+0DUd4CzpJJxSw/XlmGF1oWlU7te4Cvu86Qnefmc=
k8s.io/apiserver v0.0.0-20241205044541-adee2595b30a h1:z79OOMe0wUpBPYRldL+EwSUlQft7MfQnxm6fpCbYJMw=
k8s.io/apiserver v0.0.0-20241205044541-adee2595b30a/go.mod h1:AIxyiuTTK2pLSZqIOPeuCjrVD8McWJUasYaCa+uaVkY=
k8s.io/client-go v0.0.0-20241205022958-9df509924771 h1:xs0ClyErB354D9YgUd9Xl/GmmFD/6z8L6h+OzukN1e4=
k8s.io/client-go v0.0.0-20241205022958-9df509924771/go.mod h1:pjwARkIwQo/lbYknNh+zS807MiUmLmzkdiqcjCkc63o=
k8s.io/code-generator v0.0.0-20241205023959-906f6b3626c4 h1:7pJgzJy17Y8OspQI0JPB1kxEB0i6+nOfX+JYuIxx0Tg=
k8s.io/code-generator v0.0.0-20241205023959-906f6b3626c4/go.mod h1:Z23VrD0O7AdI2IpPPGpmKb4MTvoO/1OMrxGpLowjgQA=
k8s.io/component-base v0.0.0-20241205024250-af0d53b80315 h1:2tZbvHAYLULZ3feYnn1T4jyR3Zy59NvG9SbiWWQMFdE=
k8s.io/component-base v0.0.0-20241205024250-af0d53b80315/go.mod h1:Pq5/A75ZXlNNOr0S1Wk8ZqoXk9QTr+yxwgJty4wfkxY=
k8s.io/component-base v0.0.0-20241205044541-c7b5643e57ed h1:yuM5yr0hAukRSVkzR3QEaKg/89dBYNeOo4s5ypINVn8=
k8s.io/component-base v0.0.0-20241205044541-c7b5643e57ed/go.mod h1:Pq5/A75ZXlNNOr0S1Wk8ZqoXk9QTr+yxwgJty4wfkxY=
k8s.io/gengo/v2 v2.0.0-20240911193312-2b36238f13e9 h1:si3PfKm8dDYxgfbeA6orqrtLkvvIeH8UqffFJDl0bz4=
k8s.io/gengo/v2 v2.0.0-20240911193312-2b36238f13e9/go.mod h1:EJykeLsmFC60UQbYJezXkEsG2FLrt0GPNkU5iK5GWxU=
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
Expand Down
9 changes: 7 additions & 2 deletions pkg/apiserver/handler_proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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})
Expand Down
139 changes: 109 additions & 30 deletions pkg/apiserver/handler_proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,31 +34,34 @@ import (
"sync/atomic"
"testing"

"k8s.io/apiserver/pkg/audit"
"k8s.io/apiserver/pkg/server/dynamiccertificates"
"k8s.io/client-go/transport"

"golang.org/x/net/websocket"

"github.com/google/go-cmp/cmp"
"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"
)

Expand Down Expand Up @@ -130,6 +133,8 @@ func TestProxyHandler(t *testing.T) {
expectedBody string
expectedCalled bool
expectedHeaders map[string][]string

enableFeatureGates []featuregate.Feature
}{
"no target": {
expectedStatusCode: http.StatusNotFound,
Expand All @@ -138,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",
},
Expand All @@ -161,7 +166,40 @@ 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,
},
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
},
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: ptr.To[int32](443)},
Group: "foo",
Version: "v1",
InsecureSkipTLSVerify: true,
Expand All @@ -172,6 +210,7 @@ func TestProxyHandler(t *testing.T) {
},
},
},
enableFeatureGates: []featuregate.Feature{features.RemoteRequestHeaderUID},
expectedStatusCode: http.StatusOK,
expectedCalled: true,
expectedHeaders: map[string][]string{
Expand All @@ -195,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,
Expand All @@ -208,6 +247,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: ptr.To[int32](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"},
Expand All @@ -229,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,
Expand All @@ -253,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,
Expand All @@ -276,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",
},
Expand All @@ -298,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,
Expand All @@ -320,7 +393,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 {
Expand Down Expand Up @@ -354,37 +431,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
}

Expand All @@ -397,7 +474,7 @@ func TestProxyHandler(t *testing.T) {
t.Errorf("expected the x509_missing_san_total to be 1, but it's %d", errorCounter)
}
}
}()
})
}
}

Expand Down Expand Up @@ -432,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 {
Expand All @@ -446,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{
Expand All @@ -463,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{
Expand All @@ -480,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{
Expand All @@ -497,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{
Expand All @@ -515,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{
Expand Down Expand Up @@ -996,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()
Expand Down

0 comments on commit a701c6a

Please sign in to comment.