From 43e924a5b613850f43c02ac5a74d3e81c1292f3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= <wojtekt@google.com>
Date: Thu, 25 Nov 2021 08:55:45 +0100
Subject: [PATCH] Fix logging resource-scoped watch requests as GET requests.

---
 .../pkg/endpoints/metrics/metrics.go          | 22 ++++++-------------
 .../pkg/endpoints/metrics/metrics_test.go     | 12 ++++++++--
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
index 7dd40a373c3..445dfb1c297 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
@@ -562,7 +562,7 @@ func CanonicalVerb(verb string, scope string) string {
 // LIST and APPLY from PATCH.
 func CleanVerb(verb string, request *http.Request) string {
 	reportedVerb := verb
-	if verb == "LIST" && checkIfWatch(request) {
+	if suggestedVerb := getVerbIfWatch(request); suggestedVerb == "WATCH" {
 		reportedVerb = "WATCH"
 	}
 	// normalize the legacy WATCHLIST to WATCH to ensure users aren't surprised by metrics
@@ -593,27 +593,19 @@ func cleanVerb(verb, suggestedVerb string, request *http.Request) string {
 	return OtherRequestMethod
 }
 
-//getVerbIfWatch additionally ensures that GET or List would be transformed to WATCH
+// getVerbIfWatch additionally ensures that GET or List would be transformed to WATCH
 func getVerbIfWatch(req *http.Request) string {
 	if strings.ToUpper(req.Method) == "GET" || strings.ToUpper(req.Method) == "LIST" {
-		if checkIfWatch(req) {
-			return "WATCH"
+		// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
+		if values := req.URL.Query()["watch"]; len(values) > 0 {
+			if value := strings.ToLower(values[0]); value != "0" && value != "false" {
+				return "WATCH"
+			}
 		}
 	}
 	return ""
 }
 
-//checkIfWatch check request is watch
-func checkIfWatch(req *http.Request) bool {
-	// see apimachinery/pkg/runtime/conversion.go Convert_Slice_string_To_bool
-	if values := req.URL.Query()["watch"]; len(values) > 0 {
-		if value := strings.ToLower(values[0]); value != "0" && value != "false" {
-			return true
-		}
-	}
-	return false
-}
-
 func cleanDryRun(u *url.URL) string {
 	// avoid allocating when we don't see dryRun in the query
 	if !strings.Contains(u.RawQuery, "dryRun") {
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go
index a1fa6ba95ca..5da5a850047 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go
@@ -49,6 +49,7 @@ func TestCleanVerb(t *testing.T) {
 			desc:        "LIST should be transformed to WATCH if we have the right query param on the request",
 			initialVerb: "LIST",
 			request: &http.Request{
+				Method: "GET",
 				URL: &url.URL{
 					RawQuery: "watch=true",
 				},
@@ -59,6 +60,7 @@ func TestCleanVerb(t *testing.T) {
 			desc:        "LIST isn't transformed to WATCH if we have query params that do not include watch",
 			initialVerb: "LIST",
 			request: &http.Request{
+				Method: "GET",
 				URL: &url.URL{
 					RawQuery: "blah=asdf&something=else",
 				},
@@ -66,20 +68,25 @@ func TestCleanVerb(t *testing.T) {
 			expectedVerb: "LIST",
 		},
 		{
-			desc:        "GET isn't be transformed to WATCH if we have the right query param on the request",
+			// The above may seem counter-intuitive, but it actually is needed for cases like
+			// watching a single item, e.g.:
+			//  /api/v1/namespaces/foo/pods/bar?fieldSelector=metadata.name=baz&watch=true
+			desc:        "GET is transformed to WATCH if we have the right query param on the request",
 			initialVerb: "GET",
 			request: &http.Request{
+				Method: "GET",
 				URL: &url.URL{
 					RawQuery: "watch=true",
 				},
 			},
-			expectedVerb: "GET",
+			expectedVerb: "WATCH",
 		},
 		{
 			desc:          "LIST is transformed to WATCH for the old pattern watch",
 			initialVerb:   "LIST",
 			suggestedVerb: "WATCH",
 			request: &http.Request{
+				Method: "GET",
 				URL: &url.URL{
 					RawQuery: "/api/v1/watch/pods",
 				},
@@ -91,6 +98,7 @@ func TestCleanVerb(t *testing.T) {
 			initialVerb:   "LIST",
 			suggestedVerb: "WATCHLIST",
 			request: &http.Request{
+				Method: "GET",
 				URL: &url.URL{
 					RawQuery: "/api/v1/watch/pods",
 				},