apiserver aggregation redirect rejection incorrectly fails on 304 Not Modified responses

[brandond]

What happened?

Upgraded from Kubernetes v1.25.0 to v1.25.1. Standard metrics-server deployment now causes the following message to appear in the kube-apiserver logs on a recurring basis:

E0917 00:20:27.674186      54 controller.go:113] loading OpenAPI spec for "v1beta1.metrics.k8s.io" failed with: failed to retrieve openAPI spec, http error: ResponseCode: 502, Body: the backend attempted to redirect this request, which is not permitted, Header: map[]
I0917 00:20:27.674202      54 controller.go:126] OpenAPI AggregationController: action for item v1beta1.metrics.k8s.io: Rate Limited Requeue.

Adding a quick hack debug Infof print shows:

I0917 00:46:32.431308 53 upgradeaware.go:267] Request for https://10.42.0.6:10250/openapi/v2 (&http.Request{Method:"GET", URL:(*url.URL)(0xc00577bef0), Proto:"HTTP/1.1", ProtoMajor:1, ProtoMinor:1, Header:http.Header{"Accept":[]string{"application/json"}, "If-None-Match":[]string{"\"5C20AA8E3041C498FF6A7840B92F71F052BF64D5A4BF31E879A160C22C31BE4BF0289546AEB9EB605F96DF72AEAB19257C2636A7DF2C16DFF9A75BF89EEF3942\""}, "User-Agent":[]string{""}, "X-Forwarded-Host":[]string{"10.42.0.6:10250"}, "X-Forwarded-Proto":[]string{"https"}, "X-Forwarded-Uri":[]string{"/openapi/v2"}, "X-Remote-User":[]string{"system:aggregator"}}, Body:io.ReadCloser(nil), GetBody:(func() (io.ReadCloser, error))(nil), ContentLength:0, TransferEncoding:[]string(nil), Close:false, Host:"10.42.0.6:10250", Form:url.Values(nil), PostForm:url.Values(nil), MultipartForm:(*multipart.Form)(nil), Trailer:http.Header(nil), RemoteAddr:"", RequestURI:"", TLS:(*tls.ConnectionState)(nil), Cancel:(<-chan struct {})(nil), Response:(*http.Response)(nil), ctx:(*context.valueCtx)(0xc013832ba0)}) was redirected with code 304

It appears that the apiserver is sending an If-None-Match cache control header when refreshing the openapi spec, and the metrics-server service is correctly responding with a 304 Not Modified.

What did you expect to happen?

No errors logged; proper refresh of OpenAPI specs

How can we reproduce it (as minimally and precisely as possible)?

Deploy metrics-server, or any other service that uses apiserver aggregation and honors caching headers such as If-None-Match, to a Kubernetes 1.25.1 cluster.

Anything else we need to know?

#112193 made bad assumptions; not all HTTP 3XX responses are redirects. It should probably only reject the response if it is 300-399 AND the location header is set.

Kubernetes version

/ # kubectl version -o yaml
clientVersion:
  buildDate: "2022-09-17T00:34:52Z"
  compiler: gc
  gitCommit: 734ced7c1c988cdd73baf47a74ef972010b13868
  gitTreeState: clean
  gitVersion: v1.25.1+k3s-734ced7c
  goVersion: go1.19.1
  major: "1"
  minor: "25"
  platform: linux/amd64
kustomizeVersion: v4.5.7
serverVersion:
  buildDate: "2022-09-17T00:34:52Z"
  compiler: gc
  gitCommit: 734ced7c1c988cdd73baf47a74ef972010b13868
  gitTreeState: clean
  gitVersion: v1.25.1+k3s-734ced7c
  goVersion: go1.19.1
  major: "1"
  minor: "25"
  platform: linux/amd64

Cloud provider

none

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here

Install tools

k3s

Container runtime (CRI) and version (if applicable)

containerd

Related plugins (CNI, CSI, ...) and versions (if applicable)

flannel

[brandond]

Taking a stab at some tags here...

/cc @jindijamie
/cc @deads2k
/cc @liggitt
/area apiserver
/sig api-machinery

[liggitt]

thanks for the report, looks like this should be

diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
index a3a14241cc6..278ed089d95 100644
--- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
+++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
@@ -263,7 +263,7 @@ func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request
 		oldModifyResponse := proxy.ModifyResponse
 		proxy.ModifyResponse = func(response *http.Response) error {
 			code := response.StatusCode
-			if code >= 300 && code <= 399 {
+			if code >= 300 && code <= 399 && len(response.Header.Get("Location")) > 0 {
 				// close the original response
 				response.Body.Close()
 				msg := "the backend attempted to redirect this request, which is not permitted"

/priority critical-urgent
/kind regression
cc @kubernetes/release-managers since this requires a respin of a final 1.22 patch release :-/

[liggitt]

Fix open in #112526, will get cherry-picks open to start greening up CI

[leilajal]

/triage accepted

[cybercoder]

I need to make a redirect to the external URL but it's not allowed :(