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

Add a KEP for Node-Scoped Daemonset #944

Closed

Conversation

haiyanmeng
Copy link
Contributor

This KEP describes a mechanism restricting a DaemonSet pod to only managing the resources that reside on the same node as the DaemonSet pod.

Tracking issue: the 4.c part of kubernetes/kubernetes#62747

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 8, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @haiyanmeng. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 8, 2019
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 8, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: haiyanmeng
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: liggitt

If they are not already assigned, you can assign the PR to them by writing /assign @liggitt in a comment when ready.

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 kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Apr 8, 2019
@haiyanmeng
Copy link
Contributor Author

/cc @tallclair

@janetkuo
Copy link
Member

janetkuo commented Apr 8, 2019

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 8, 2019
@haiyanmeng
Copy link
Contributor Author

/cc @janetkuo

@janetkuo
Copy link
Member

janetkuo commented Apr 8, 2019

/sig apps

@k8s-ci-robot k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Apr 8, 2019
Copy link
Member

@tallclair tallclair left a comment

Choose a reason for hiding this comment

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

Thanks @haiyanmeng ! Awesome job for your first ever KEP. Looking forward to discussing at next sig-auth

keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
node-scoped info into the Extra field of user.DefaultInfo;
* NodeRestriction gets the pod info and the node-scoped info from the Extra
field of user.DefaultInfo through GetExtra; using the pod info,
NodeRestriction can gets the node information of P1 by checking
Copy link
Member

Choose a reason for hiding this comment

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

I thought it could just fetch it directly from the graph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the resource graph used in NodeAuthorizer?

Since NodeRestriction can inspect the request objects, there is no need to check the resource graph.

keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
keps/sig-auth/20190408-node-scoped-daemonset.md Outdated Show resolved Hide resolved
@haiyanmeng haiyanmeng force-pushed the node-scoped-daemonset branch 9 times, most recently from 6033312 to 4fd0cb4 Compare April 10, 2019 19:28
@haiyanmeng haiyanmeng force-pushed the node-scoped-daemonset branch 2 times, most recently from 097bff5 to 4bb5285 Compare April 22, 2019 18:40
@tallclair
Copy link
Member

Nodes from 2019-04-17 sig-auth:

  • Consider separating the authorization & admission piece. It looks like the admission piece doesn't need the graph, and is more node specific. Admission could be handled with policy controls (e.g. gatekeeper)
  • Intersecting permissions has been previously discussed, and might make sense here - would intersecting service account & kubelet permissions be sufficient?
  • Some questions about whether this is sufficiently generic, and how it applies to non-DaemonSet workloads.
    • Note(tallclair): I don't see the use case for non-daemonset workloads. IMO this only makes sense when the workload is node-aware

Anything else I missed?

@haiyanmeng haiyanmeng force-pushed the node-scoped-daemonset branch 9 times, most recently from f97c3c7 to b2b3cc7 Compare April 22, 2019 22:26
Signed-off-by: Haiyan Meng <haiyanmeng@google.com>
@haiyanmeng
Copy link
Contributor Author

/cc @deads2k @smarterclayton

@haiyanmeng
Copy link
Contributor Author

Based on the feedback from last week's sig-auth meeting, I updated the KEP. The main changes are:

  1. added a Use Cases section and Auternatives section;
  2. updated the Proposal section. In short, the new solution allows the DaemonSet owners to specify a list of resource types whose access permissions should be node-scoped; during the authorization step, the requests from node-scoped daemonset pods will go through both NodeAuthorizer and RBAC, and the request must be allowed by the NodeAuthorizer first before RBAC authorizes the request. If NodeAuthorizer does not explicitly allow the request, the request will be denied directly.

Please let me know if you have any suggestions.


Node-scoped info can be defined in two places:

* Option 1: in
Copy link
Member

Choose a reason for hiding this comment

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

I am still trying to grasp all of the pieces here, but adding a new field to SA seems wrong to me. Could this be handled via the token request API?


### The Origin of Node-Scoped Resource Spec

Node-scoped info can be defined in two places:
Copy link
Member

@mikedanese mikedanese Apr 23, 2019

Choose a reason for hiding this comment

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

Both these options feel like authorization bleeding out of authorization. We unfortunately do this in various places already but it hurts our ability to build authorization features, e.g. can-i explain, delegated authz.

@haiyanmeng
Copy link
Contributor Author

As discussed in the sig-auth meeting, the short-term plan to achieve node-scoped daemonsets is to run daemonsets with kubelet credentials, while we work on a long term plan, which will avoid tangling authorizers together and may conditionalize authorization.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 5, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 4, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cheyang
Copy link
Contributor

cheyang commented Mar 30, 2023

@haiyanmeng May I ask if we'd like to use Node-Scoped Daemonset. What's the recommended way? Still with kubelet credentials?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants