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.

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

Remove deprecated extensions API group in document #32909

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

Sea-n
Copy link
Member

@Sea-n Sea-n commented Apr 13, 2022

The extensions API group is deprecated.

Changes in ABAC page

Changes in RBAC page

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. language/en Issues or PRs related to English language labels Apr 13, 2022
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Apr 13, 2022
@netlify
Copy link

netlify bot commented Apr 13, 2022

Deploy Preview for kubernetes-io-main-staging ready!

Name Link
🔨 Latest commit 613bb08
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/625acb2b7fbd85000948ec94
😎 Deploy Preview https://deploy-preview-32909--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -407,7 +407,7 @@ rules:
# objects is "pods"
resources: ["pods"]
verbs: ["get", "list", "watch"]
- apiGroups: ["batch", "extensions"]
- apiGroups: ["batch", "networking.k8s.io"]
Copy link
Member

Choose a reason for hiding this comment

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

jobs don't exist in the networking.k8s.io group... if you want to use a multi-group example, "events" resources in the core "" and "events.k8s.io" API groups would be a current example

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! Thanks very much for pointing it out and providing a great example!

I have pushed a new commit, could you please check the correctness again? 🙇‍♂️

@Sea-n Sea-n force-pushed the deprecate-ext branch 2 times, most recently from 1d30612 to d508d28 Compare April 14, 2022 03:23
@Sea-n Sea-n changed the title Use networking.k8s.io instead of extensions in document Remove deprecated extensions API group in document Apr 14, 2022
@liggitt
Copy link
Member

liggitt commented Apr 14, 2022

/lgtm

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

LGTM label has been added.

Git tree hash: 640be8ca25b8876bfcbe27fc249b7c7fde78356a

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks for proposing the update.

One question, one recommended tweak.

Comment on lines 386 to 387
Allow reading/writing Events (at the HTTP level: objects with `"events"` in the
resource part of their URL) in both the core `""` and `"events.k8s.io"` API groups:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use Deployment, in its GA API group (apps). People rarely patch Events.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for letting me know that events is not a common resource. 😅

I think this section (Role examples) is about how to write rules instead of most used resources:

  1. Reading pods in the core API group
    • The most simple one
  2. R/W deployments in extensions and apps API group
    • Multiple API groups
  3. Reading pods in core API group and R/W jobs in batch and extensions
    • Multiple rules
  4. Allow reading a ConfigMap named my-config
    • Must be bound with a RoleBinding to limit to a single ConfigMap in a single namespace.
  5. Allow reading the resource "nodes" in the core group
    • Because a Node is cluster-scoped, this must be in a ClusterRole bound with a ClusterRoleBinding to be effective.
  6. Allow GET and POST requests to the non-resource endpoint
    • Example for nonResourceURLs

Since extensions is deprecated, I think it's good to replace it with another (although not that common) example for multiple API groups.

There are only three resources that appeared in multiple API groups:

NAME      APIVERSION                NAMESPACED    KIND
events    v1                        true          Event
events    events.k8s.io/v1          true          Event
nodes     v1                        false         Node
nodes     metrics.k8s.io/v1beta1    false         NodeMetrics
pods      v1                        true          Pod
pods      metrics.k8s.io/v1beta1    true          PodMetrics

I am still a beginner at Kubernetes, could you give me some advice about which could be a better example for this page?

Copy link
Member

Choose a reason for hiding this comment

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

The nodes and pods in v1 and metrics API groups are not the same resource, so I wouldn't expect to grant access to both in the same rule.

events are the only remaining built-in resource that appears in multiple API groups (we had a lot in the extensions API group but those got migrated to dedicated groups and the extensions API group was deprecated and disabled)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for clarifying! Now I think it's better to change events example back to deployments. 😊

- Wildcard: `*` matches all API groups.
- `namespace`, type string; a namespace.
- Ex: `kube-system`
- Wildcard: `*` matches all resource requests.
- `resource`, type string; a resource type
- Ex: `pods`
- Ex: `pods`, `events`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be better to provide multiple examples. Just like you said that it's not common to see events, could you give me some advice on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've change it to deployments, hope it will be more useful for users.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 15, 2022
@sftim
Copy link
Contributor

sftim commented Apr 16, 2022

@kubernetes/sig-auth-pr-reviews does this look right?

@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Apr 16, 2022
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Markdown LGTM

#
# at the HTTP level, the name of the resource for accessing Deployment
# at the HTTP level, the name of the resource for accessing Deployments
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
# at the HTTP level, the name of the resource for accessing Deployments
# at the HTTP level, the name of the resource for accessing Deployment

In the URL it's /deployments/; in text, it's “Deployments” or “Deployment objects”.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh no, it's a typo when I manually revert the change. 😢

How could I miss that, thanks for pointing it out! 🙇‍♂️

@Sea-n
Copy link
Member Author

Sea-n commented Apr 20, 2022

Hi @liggitt,

I've force-pushed the changes according to your suggestions, could you take a look? 🙇‍♂️

@liggitt
Copy link
Member

liggitt commented Apr 20, 2022

/lgtm

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

LGTM label has been added.

Git tree hash: e259f32eb38a33a1d6ac1ddecae0747fab41efb6

@sftim
Copy link
Contributor

sftim commented Apr 20, 2022

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2022
@k8s-ci-robot k8s-ci-robot merged commit c62c9e9 into kubernetes:main Apr 20, 2022
@Sea-n Sea-n deleted the deprecate-ext branch May 24, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants