Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

document /watch prefix deprecation #65147

Merged
merged 1 commit into from
Aug 7, 2018
Merged

document /watch prefix deprecation #65147

merged 1 commit into from
Aug 7, 2018

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jun 15, 2018 •
edited
Loading

closes #65133

these have been marked as deprecated in code for many releases, and all client accesses have switched to using the ?watch=true access method, but documentation was never updated

@kubernetes/sig-api-machinery-api-reviews @kubernetes/api-reviewers

The watch API endpoints prefixed with `/watch` are deprecated and will be removed in a future release. These standard method for watching resources (supported since v1.0) is to use the list API endpoints with a `?watch=true` parameter. All client-go clients have used the parameter method since v1.6.0.

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 15, 2018
@liggitt liggitt mentioned this pull request Jun 15, 2018
Closed
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 15, 2018
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 15, 2018
@liggitt
Copy link
Member Author

liggitt commented Jun 15, 2018 •
edited
Loading

The /proxy prefix paths (deprecated since 1.2) were removed in 1.10 in #59884

I'd like to get deprecation of the /watch prefix paths on the record for 1.11.

hzxuzhonghu
hzxuzhonghu reviewed Jun 20, 2018
View reviewed changes
staging/src/k8s.io/apiserver/pkg/endpoints/installer.go
@@ -382,7 +382,7 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't been deprecated long ago? Can we remove now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it wasn't formally announced, so not yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ic

@lavalamp
Copy link
Member

I was just saying on a api coverage spreadsheet that all these should be just deleted at this point...

cc @mbohlool to double check the open api spec

@liggitt
Copy link
Member Author

liggitt commented Jun 21, 2018 •
edited
Loading

I was just saying on a api coverage spreadsheet that all these should be just deleted at this point...

cc @mbohlool to double check the open api spec

unfortunately, the endpoints are listed throughout the openapi spec, with no doc-visible deprecation notices. this PR updates it to note the deprecation - https://github.com/kubernetes/kubernetes/pull/65147/files#diff-9ce7ea8441086bf1902b4f936f4601d0

@liggitt
Copy link
Member Author

liggitt commented Jun 25, 2018

@lavalamp @deads2k should we include this deprecation announcement in the 1.11 release notes?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2018
@liggitt
Copy link
Member Author

liggitt commented Jul 13, 2018

/assign @mbohlool @deads2k @lavalamp

@spiffxp
Copy link
Member

spiffxp commented Aug 1, 2018

@liggitt needs-rebase (also, I'm happy to lgtm after rebase unless you specifically need approval from the aformentioned reviewers)

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2018
@liggitt
Copy link
Member Author

liggitt commented Aug 6, 2018

rebased. would like an ack from @lavalamp / @deads2k

@lavalamp
Copy link
Member

lavalamp commented Aug 6, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spiffxp
Copy link
Member

spiffxp commented Aug 6, 2018 

/go/src/k8s.io/kubernetes/api/openapi-spec is out of date. Please run hack/update-openapi-spec.sh
/go/src/k8s.io/kubernetes/api/swagger-spec is out of date. Please run hack/update-swagger-spec.sh

@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2018
@liggitt
Copy link
Member Author

liggitt commented Aug 7, 2018

regenerated, retagging

@liggitt liggitt added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2018
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@spiffxp
Copy link
Member

spiffxp commented Aug 7, 2018 

/go/src/k8s.io/kubernetes/docs/api-reference is out of date. Please run hack/update-api-reference-docs.sh

... sure would have been nice if it said this last time

@spiffxp
Copy link
Member

spiffxp commented Aug 7, 2018

/lgtm cancel
to stop fejta-bot from /retest spamming

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2018
@liggitt
Copy link
Member Author

liggitt commented Aug 7, 2018

third time's the charm

@liggitt liggitt added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2018
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 6bea053 into kubernetes:master Aug 7, 2018
@liggitt liggitt deleted the watch-prefix-deprecation branch August 10, 2018 01:07
@spiffxp spiffxp mentioned this pull request Aug 23, 2018
Closed
@stealthybox stealthybox mentioned this pull request Sep 29, 2018
Closed
@jethrogb
Copy link

Why is this deprecated? This is a regression in terms of RBAC

@liggitt
Copy link
Member Author

liggitt commented Dec 28, 2018

Why is this deprecated?

The top-level path segment access patterns (for proxy and watch) were deprecated in favor of the proxy subresource and the watch query parameter around 1.2

This is a regression in terms of RBAC

That is incorrect, watch requests are still separately permissioned using the watch verb. Prior to this, there were two URLs that would make a watch request (/watch/… and …?watch=true), and this announces deprecation of the first

@jethrogb
Copy link

jethrogb commented Dec 28, 2018 •
edited
Loading

That is incorrect, watch requests are still separately permissioned using the watch verb.

No, it is a regression. With the Watch API, you can set watch permissions on a single named resource. With WatchList, you need to give permissions on all resources of a particular type. (Related kubernetes/kubectl#305)

@liggitt
Copy link
Member Author

liggitt commented Dec 28, 2018 •
edited
Loading

you can still set watch permissions on a single named item, watched with ?watch=true&fieldSelector=metadata.name=<name>

see #63469

@corruptmem corruptmem mentioned this pull request Apr 1, 2019
Closed
@dims dims mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzxuzhonghu hzxuzhonghu hzxuzhonghu left review comments

@erictune erictune Awaiting requested review from erictune

Assignees

@spiffxp spiffxp

@lavalamp lavalamp

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ambiguity to find pod specific watch api endpoints in e2e test suite
8 participants
@liggitt @lavalamp @spiffxp @k8s-ci-robot @fejta-bot @k8s-github-robot @jethrogb @hzxuzhonghu