From 2d397e853018d7234d63e376bcb13b6701862c90 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 28 Sep 2022 09:34:51 -0400 Subject: [PATCH] Avoid following redirects in aggregator availability controller --- .../status/available_controller.go | 3 + .../status/available_controller_test.go | 67 ++++++++++++++----- test/integration/examples/apiserver_test.go | 2 +- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go index 9c89b313167..ba69c4dc369 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go @@ -264,6 +264,9 @@ func (c *AvailableConditionController) sync(key string) error { Transport: restTransport, // the request should happen quickly. Timeout: 5 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, } apiService := originalAPIService.DeepCopy() diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go index 46dc74877ff..d4a23118897 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller_test.go @@ -258,11 +258,12 @@ func TestSync(t *testing.T) { tests := []struct { name string - apiServiceName string - apiServices []*apiregistration.APIService - services []*v1.Service - endpoints []*v1.Endpoints - forceDiscoveryFail bool + apiServiceName string + apiServices []*apiregistration.APIService + services []*v1.Service + endpoints []*v1.Endpoints + backendStatus int + backendLocation string expectedAvailability apiregistration.APIServiceCondition }{ @@ -270,6 +271,7 @@ func TestSync(t *testing.T) { name: "local", apiServiceName: "local.group", apiServices: []*apiregistration.APIService{newLocalAPIService("local.group")}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionTrue, @@ -282,6 +284,7 @@ func TestSync(t *testing.T) { apiServiceName: "remote.group", apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, services: []*v1.Service{newService("foo", "not-bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -302,7 +305,8 @@ func TestSync(t *testing.T) { }, }, }}, - endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -315,6 +319,7 @@ func TestSync(t *testing.T) { apiServiceName: "remote.group", apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -328,6 +333,7 @@ func TestSync(t *testing.T) { apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, endpoints: []*v1.Endpoints{newEndpoints("foo", "bar")}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -341,6 +347,7 @@ func TestSync(t *testing.T) { apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, "wrongName")}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -354,6 +361,7 @@ func TestSync(t *testing.T) { apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusOK, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionTrue, @@ -362,12 +370,41 @@ func TestSync(t *testing.T) { }, }, { - name: "remote-bad-return", - apiServiceName: "remote.group", - apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, - services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, - endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, - forceDiscoveryFail: true, + name: "remote-bad-return", + apiServiceName: "remote.group", + apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, + services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, + endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusForbidden, + expectedAvailability: apiregistration.APIServiceCondition{ + Type: apiregistration.Available, + Status: apiregistration.ConditionFalse, + Reason: "FailedDiscoveryCheck", + Message: `failing or missing response from`, + }, + }, + { + name: "remote-redirect", + apiServiceName: "remote.group", + apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, + services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, + endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusFound, + backendLocation: "/test", + expectedAvailability: apiregistration.APIServiceCondition{ + Type: apiregistration.Available, + Status: apiregistration.ConditionFalse, + Reason: "FailedDiscoveryCheck", + Message: `failing or missing response from`, + }, + }, + { + name: "remote-304", + apiServiceName: "remote.group", + apiServices: []*apiregistration.APIService{newRemoteAPIService("remote.group")}, + services: []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}, + endpoints: []*v1.Endpoints{newEndpointsWithAddress("foo", "bar", testServicePort, testServicePortName)}, + backendStatus: http.StatusNotModified, expectedAvailability: apiregistration.APIServiceCondition{ Type: apiregistration.Available, Status: apiregistration.ConditionFalse, @@ -394,10 +431,10 @@ func TestSync(t *testing.T) { } testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !tc.forceDiscoveryFail { - w.WriteHeader(http.StatusOK) + if tc.backendLocation != "" { + w.Header().Set("Location", tc.backendLocation) } - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(tc.backendStatus) })) defer testServer.Close() diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index 1cbba275844..0ce4944f65e 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -392,7 +392,7 @@ func TestAggregatedAPIServerRejectRedirectResponse(t *testing.T) { if strings.HasSuffix(r.URL.Path, "tryRedirect") { http.Redirect(w, r, redirectedURL+"/redirectTarget", http.StatusMovedPermanently) } else { - http.Redirect(w, r, redirectedURL, http.StatusMovedPermanently) + w.WriteHeader(http.StatusOK) } })) defer redirectServer.Close()