Skip to content

Commit

Permalink
Remove "WATCH" and "WATCHLIST" deprecated in 1.11
Browse files Browse the repository at this point in the history 
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
  • Loading branch information
@dims
dims committed Feb 22, 2024
1 parent 9fa043e commit 335bcd5
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 109 deletions.
9 changes: 0 additions & 9 deletions staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,6 @@ func TestNotFound(t *testing.T) {
"groupless root DELETE with extra segment": {"DELETE", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
"groupless root PUT without extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots", http.StatusMethodNotAllowed},
"groupless root PUT with extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
"groupless root watch missing storage": {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusInternalServerError},

"groupless namespaced PATCH method": {"PATCH", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
"groupless namespaced GET long prefix": {"GET", "/" + grouplessPrefix + "/", http.StatusNotFound},
Expand All @@ -765,9 +764,6 @@ func TestNotFound(t *testing.T) {
"groupless namespaced DELETE with extra segment": {"DELETE", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
"groupless namespaced PUT without extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
"groupless namespaced PUT with extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
"groupless namespaced watch missing storage": {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusInternalServerError},
"groupless namespaced watch with bad method": {"POST", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/namespaces/ns/simples/bar", http.StatusMethodNotAllowed},
"groupless namespaced watch param with bad method": {"POST", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/ns/simples/bar?watch=true", http.StatusMethodNotAllowed},

// Positive checks to make sure everything is wired correctly
"GET root": {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots", http.StatusOK},
Expand All @@ -785,8 +781,6 @@ func TestNotFound(t *testing.T) {
"root DELETE with extra segment": {"DELETE", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
"root PUT without extra segment": {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots", http.StatusMethodNotAllowed},
"root PUT with extra segment": {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound},
"root watch missing storage": {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusInternalServerError},
// TODO: JTL: "root watch with bad method": {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/simpleroot/bar", http.StatusMethodNotAllowed},

"namespaced PATCH method": {"PATCH", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
"namespaced GET long prefix": {"GET", "/" + prefix + "/", http.StatusNotFound},
Expand All @@ -797,9 +791,6 @@ func TestNotFound(t *testing.T) {
"namespaced DELETE with extra segment": {"DELETE", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
"namespaced PUT without extra segment": {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples", http.StatusMethodNotAllowed},
"namespaced PUT with extra segment": {"PUT", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar/baz", http.StatusNotFound},
"namespaced watch missing storage": {"GET", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/", http.StatusInternalServerError},
"namespaced watch with bad method": {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/namespaces/ns/simples/bar", http.StatusMethodNotAllowed},
"namespaced watch param with bad method": {"POST", "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/ns/simples/bar?watch=true", http.StatusMethodNotAllowed},
}
handler := handle(map[string]rest.Storage{
"simples": &SimpleRESTStorage{},
Expand Down
63 changes: 1 addition & 62 deletions staging/src/k8s.io/apiserver/pkg/endpoints/installer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ var toDiscoveryKubeVerb = map[string]string{
"POST": "create",
"PROXY": "proxy",
"PUT": "update",
"WATCH": "watch",
"WATCHLIST": "watch",
}

// Install handlers for API resources.
Expand Down Expand Up @@ -425,15 +423,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
isGetter = true
}

var versionedWatchEvent interface{}
if isWatcher {
versionedWatchEventPtr, err := a.group.Creater.New(a.group.GroupVersion.WithKind("WatchEvent"))
if err != nil {
return nil, nil, err
}
versionedWatchEvent = indirectArbitraryPointer(versionedWatchEventPtr)
}

var (
connectOptions runtime.Object
versionedConnectOptions runtime.Object
Expand All @@ -459,7 +448,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
}
}

allowWatchList := isWatcher && isLister // watching on lists is allowed only for kinds that support both watch and list.
nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string")
pathParam := ws.PathParameter("path", "path to the resource").DataType("string")

Expand Down Expand Up @@ -521,8 +509,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer, false}, isLister)
actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer, false}, isCreater)
actions = appendIf(actions, action{"DELETECOLLECTION", resourcePath, resourceParams, namer, false}, isCollectionDeleter)
// DEPRECATED in 1.11
actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer, false}, allowWatchList)

// Add actions at the item path: /api/apiVersion/resource/{name}
actions = appendIf(actions, action{"GET", itemPath, nameParams, namer, false}, isGetter)
Expand Down Expand Up @@ -566,8 +552,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer, false}, isLister)
actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer, false}, isCreater)
actions = appendIf(actions, action{"DELETECOLLECTION", resourcePath, resourceParams, namer, false}, isCollectionDeleter)
// DEPRECATED in 1.11
actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer, false}, allowWatchList)

actions = appendIf(actions, action{"GET", itemPath, nameParams, namer, false}, isGetter)
if getSubpath {
Expand All @@ -576,8 +560,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer, false}, isUpdater)
actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer, false}, isPatcher)
actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer, false}, isGracefulDeleter)
// DEPRECATED in 1.11
actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer, false}, isWatcher)

actions = appendIf(actions, action{"CONNECT", itemPath, nameParams, namer, false}, isConnecter)
actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", proxyParams, namer, false}, isConnecter && connectSubpath)

Expand All @@ -586,8 +569,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
// TODO: more strongly type whether a resource allows these actions on "all namespaces" (bulk delete)
if !isSubresource {
actions = appendIf(actions, action{"LIST", resource, params, namer, true}, isLister)
// DEPRECATED in 1.11
actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer, true}, allowWatchList)
}
}

Expand Down Expand Up @@ -979,48 +960,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
}
addParams(route, action.Params)
routes = append(routes, route)
// deprecated in 1.11
case "WATCH": // Watch a resource.
doc := "watch changes to an object of kind " + kind
if isSubresource {
doc = "watch changes to " + subresource + " of an object of kind " + kind
}
doc += ". deprecated: use the 'watch' parameter with a list operation instead, filtered to a single item with the 'fieldSelector' parameter."
handler := metrics.InstrumentRouteFunc(action.Verb, group, version, resource, subresource, requestScope, metrics.APIServerComponent, deprecated, removedRelease, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout))
handler = utilwarning.AddWarningsHandler(handler, warnings)
route := ws.GET(action.Path).To(handler).
Doc(doc).
Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed. Defaults to 'false' unless the user-agent indicates a browser or command-line HTTP tool (curl and wget).")).
Operation("watch"+namespaced+kind+strings.Title(subresource)+operationSuffix).
Produces(allMediaTypes...).
Returns(http.StatusOK, "OK", versionedWatchEvent).
Writes(versionedWatchEvent)
if err := AddObjectParams(ws, route, versionedListOptions); err != nil {
return nil, nil, err
}
addParams(route, action.Params)
routes = append(routes, route)
// deprecated in 1.11
case "WATCHLIST": // Watch all resources of a kind.
doc := "watch individual changes to a list of " + kind
if isSubresource {
doc = "watch individual changes to a list of " + subresource + " of " + kind
}
doc += ". deprecated: use the 'watch' parameter with a list operation instead."
handler := metrics.InstrumentRouteFunc(action.Verb, group, version, resource, subresource, requestScope, metrics.APIServerComponent, deprecated, removedRelease, restfulListResource(lister, watcher, reqScope, true, a.minRequestTimeout))
handler = utilwarning.AddWarningsHandler(handler, warnings)
route := ws.GET(action.Path).To(handler).
Doc(doc).
Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed. Defaults to 'false' unless the user-agent indicates a browser or command-line HTTP tool (curl and wget).")).
Operation("watch"+namespaced+kind+strings.Title(subresource)+"List"+operationSuffix).
Produces(allMediaTypes...).
Returns(http.StatusOK, "OK", versionedWatchEvent).
Writes(versionedWatchEvent)
if err := AddObjectParams(ws, route, versionedListOptions); err != nil {
return nil, nil, err
}
addParams(route, action.Params)
routes = append(routes, route)
case "CONNECT":
for _, method := range connecter.ConnectMethods() {
connectProducedObject := storageMeta.ProducesObject(method)
Expand Down
15 changes: 1 addition & 14 deletions staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,7 @@ var (
"POST",
"PROXY",
"PUT",
"UPDATE",
"WATCH",
"WATCHLIST")
"UPDATE")

// These are the valid connect requests which we report in our metrics.
validConnectRequests = utilsets.NewString(
Expand Down Expand Up @@ -691,10 +689,6 @@ func CleanVerb(verb string, request *http.Request, requestInfo *request.RequestI
if suggestedVerb := getVerbIfWatch(request); suggestedVerb == "WATCH" {
reportedVerb = "WATCH"
}
// normalize the legacy WATCHLIST to WATCH to ensure users aren't surprised by metrics
if verb == "WATCHLIST" {
reportedVerb = "WATCH"
}
if verb == "PATCH" && request.Header.Get("Content-Type") == string(types.ApplyPatchType) {
reportedVerb = "APPLY"
}
Expand Down Expand Up @@ -730,13 +724,6 @@ func determineRequestNamespaceAndName(ctx context.Context, opts *metainternalver

// cleanVerb additionally ensures that unknown verbs don't clog up the metrics.
func cleanVerb(verb, suggestedVerb string, request *http.Request, requestInfo *request.RequestInfo) string {
// CanonicalVerb (being an input for this function) doesn't handle correctly the
// deprecated path pattern for watch of:
// GET /api/{version}/watch/{resource}
// We correct it manually based on the pass verb from the installer.
if suggestedVerb == "WATCH" || suggestedVerb == "WATCHLIST" {
return "WATCH"
}
reportedVerb := CleanVerb(verb, request, requestInfo)
if validRequestMethods.Has(reportedVerb) {
return reportedVerb
Expand Down
24 changes: 0 additions & 24 deletions staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,6 @@ func TestCleanVerb(t *testing.T) {
},
expectedVerb: "WATCH",
},
{
desc: "LIST is transformed to WATCH for the old pattern watchlist",
initialVerb: "LIST",
suggestedVerb: "WATCHLIST",
request: &http.Request{
Method: "GET",
URL: &url.URL{
RawQuery: "/api/v1/watch/pods",
},
},
expectedVerb: "WATCH",
},
{
desc: "WATCHLIST should be transformed to WATCH",
initialVerb: "WATCHLIST",
request: nil,
expectedVerb: "WATCH",
},
{
desc: "PATCH should be transformed to APPLY with the right content type",
initialVerb: "PATCH",
Expand All @@ -134,12 +116,6 @@ func TestCleanVerb(t *testing.T) {
request: nil,
expectedVerb: "PATCH",
},
{
desc: "WATCHLIST should be transformed to WATCH",
initialVerb: "WATCHLIST",
request: nil,
expectedVerb: "WATCH",
},
{
desc: "unexpected verbs should be designated as unknown",
initialVerb: "notValid",
Expand Down

0 comments on commit 335bcd5

Please sign in to comment.