Move cloud-specific roles out of RBAC bootstrap #66635

wgliang · 2018-07-26T00:25:27Z

What this PR does / why we need it:
The system:aws-cloud-provider ClusterRole(Binding)s should be removed from the bootstrap policy, and instead configured by the provisioning tools.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #66628

Special notes for your reviewer:
@tallclair

/sig aws
/sig cloud-provider
/sig auth

Release note:

The `system:aws-cloud-provider` cluster role, deprecated in v1.13, is no longer auto-created. Deployments using the AWS cloud provider should grant required permissions to the `aws-cloud-provider` service account in the `kube-system` namespace as part of deployment.

tallclair · 2018-07-26T00:31:41Z

Deleting the roles is the easy part. The hard part is coordinating with the whole provisioning ecosystem to avoid unexpected breakage. How do we know when this is safe to merge?

micahhausler · 2018-07-26T18:07:55Z

@tallclair I'm not familiar with where what component this gets run in, is it part of the APIServer?

tallclair · 2018-07-26T18:28:53Z

@micahhausler yeah, it's part of the RBAC plugin, so whichever apiserver that's loaded in. Typically it's the kube-aggregator server, with extension servers using delegation, but I think it's also possible to run RBAC in each extension apiserver.

duglin · 2018-09-07T17:34:07Z

Does this line need to be removed too? https://github.com/kubernetes/kubernetes/pull/66635/files#diff-eee450e334a11e0b683ce965f584c3c4R531

duglin · 2018-09-07T17:37:30Z

/retest

duglin · 2018-09-09T16:43:13Z

/test pull-kubernetes-bazel-test

duglin · 2018-09-09T16:44:08Z

@wgliang I think the expected output needs to be updated.

Signed-off-by: Doug Davis <[email protected]>

duglin · 2018-09-10T18:57:08Z

@wgliang will you have time to finish this one?

wgliang · 2018-09-10T22:44:17Z

Yeah, I'll will update it.

duglin · 2018-09-11T00:00:42Z

@wgliang thanks. If it helps, I created this: master...duglin:pr66635 if case you didn't have time. There's another "aws" line I think might need to be removed too.

wgliang · 2018-09-12T00:08:27Z

@duglin
Thanks for your review. I have updated it.

duglin · 2018-09-12T02:53:17Z

/lgtm

duglin · 2018-09-25T13:17:51Z

with the thaw... any chance of moving this one along now?

tallclair · 2018-09-25T19:57:56Z

This LGTM, but I'd like to get approval from someone from @kubernetes/sig-aws-misc
Maybe @justinsb ?

duglin · 2018-10-05T17:31:13Z

ping @justinsb @kubernetes/sig-aws-misc

tallclair · 2018-10-10T20:45:24Z

/cc @lavalamp - FYI

lavalamp · 2018-10-19T16:31:44Z

How did this get in there in the first place? We really shouldn't have any cloud provider specific code like this that's not hidden behind an interface in a cloud provider package.

gnufied · 2018-10-19T16:40:07Z

@lavalamp it got in here via #56709 and at that time there used to be a generic system:cloud-provider service account which had similar access to modify resources. There is a long history of comments how it got there(if you read the PR). but GCE cloudprovider has similar access. I originally proposed AWS cloud-provider to use same system:cloud-provider service account that GCE uses.

gnufied · 2018-10-19T18:37:16Z

BTW - is there any reason we can't deprecate this RBAC policy first and remove it in next release? That at least will give time for installation tools to catch up and will be consistent with how other cloudprovider policies are being removed. cc @justinsb

liggitt · 2018-10-19T18:54:23Z

agree with announcing removal prior to removing

liggitt · 2018-10-26T14:18:14Z

@wgliang can you make sure a deprecation notice makes it into the 1.13 release notes, and is announced to the relevant sigs (looks like aws, in this case), along with the required action for deployers? do we want to copy this role out to a yaml manifest that deployers can use as they transition to granting this permission externally?

wgliang · 2018-10-27T00:48:46Z

@liggitt No problem. I will follow up with 1.13 release note and communicate with the relevant personnel.

liggitt · 2018-11-27T20:37:51Z

added to 1.13 release notes as deprecated

/hold
for 1.15

liggitt · 2019-04-02T02:41:33Z

/hold cancel

master is open for 1.15 dev now, can you rebase this?

wgliang · 2019-04-02T11:19:01Z

@liggitt Done.

liggitt · 2019-04-02T12:53:40Z

/retest
/lgtm
/approve

k8s-ci-robot · 2019-04-02T12:54:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, liggitt, wgliang

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~plugin/pkg/auth/authorizer/OWNERS~~ [liggitt]

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

We consider it part of the storage configuration for AWS now. Upstream change: kubernetes/kubernetes#66635

* Update kubeadm init and join configs with new versions and options. * This includes switching to cgroupfs cgroup driver in kubelet due to kubernetes/kubernetes#76531. * Add AWS RBAC to keights-system Ansible role since kubernetes/kubernetes#66635 removes those permissions by default. * Update CI assets and README build status badges.

k8s-ci-robot requested review from ericchiang and erictune July 26, 2018 00:25

duglin pushed a commit to duglin/kubernetes that referenced this pull request Sep 10, 2018

Missed some files in kubernetes#66635

288716b

Signed-off-by: Doug Davis <[email protected]>

wgliang force-pushed the feature/remove-aws-cloud-provider branch from f329c87 to c5875e7 Compare September 11, 2018 23:55

k8s-ci-robot assigned duglin Sep 12, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 12, 2018

k8s-ci-robot requested a review from lavalamp October 10, 2018 20:45

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2018

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 29, 2018

tallclair added this to the v1.15 milestone Jan 2, 2019

tallclair added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jan 2, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 2, 2019

Move cloud-specific roles out of RBAC bootstrap

128fd88

wgliang force-pushed the feature/remove-aws-cloud-provider branch from c5875e7 to 128fd88 Compare April 2, 2019 11:18

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 2, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 2, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2019

k8s-ci-robot merged commit 4e397d9 into kubernetes:master Apr 2, 2019

justinsb added a commit to justinsb/kops that referenced this pull request May 9, 2019

Include aws-cloud-provider roles in 1.15

491e657

We consider it part of the storage configuration for AWS now. Upstream change: kubernetes/kubernetes#66635

justinsb mentioned this pull request May 9, 2019

Include aws-cloud-provider roles in 1.15 kubernetes/kops#6899

Merged

justinsb added a commit to justinsb/kops that referenced this pull request May 9, 2019

Include aws-cloud-provider roles in 1.15

9f2b671

We consider it part of the storage configuration for AWS now. Upstream change: kubernetes/kubernetes#66635

zhan849 mentioned this pull request May 21, 2019

Automated cherry pick of #76976: get node zone info from k8s, added tests #78101

Closed

zhan849 mentioned this pull request Jul 1, 2019

Cloud provider AWS library should query instance by ID when possible #78140

Merged

nckturner mentioned this pull request Apr 22, 2021

Understand (and rename?) RBAC and service account for AWS storage addon kubernetes/kops#11283

Closed

Move cloud-specific roles out of RBAC bootstrap #66635

Move cloud-specific roles out of RBAC bootstrap #66635

Conversation

wgliang commented Jul 26, 2018 • edited by liggitt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tallclair commented Jul 26, 2018

Uh oh!

micahhausler commented Jul 26, 2018

Uh oh!

tallclair commented Jul 26, 2018

Uh oh!

duglin commented Sep 7, 2018

Uh oh!

duglin commented Sep 7, 2018

Uh oh!

duglin commented Sep 9, 2018

Uh oh!

duglin commented Sep 9, 2018

Uh oh!

duglin commented Sep 10, 2018

Uh oh!

wgliang commented Sep 10, 2018

Uh oh!

duglin commented Sep 11, 2018

Uh oh!

wgliang commented Sep 12, 2018

Uh oh!

duglin commented Sep 12, 2018

Uh oh!

duglin commented Sep 25, 2018

Uh oh!

tallclair commented Sep 25, 2018

Uh oh!

duglin commented Oct 5, 2018

Uh oh!

tallclair commented Oct 10, 2018

Uh oh!

lavalamp commented Oct 19, 2018

Uh oh!

gnufied commented Oct 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnufied commented Oct 19, 2018

Uh oh!

liggitt commented Oct 19, 2018

Uh oh!

liggitt commented Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wgliang commented Oct 27, 2018

Uh oh!

liggitt commented Nov 27, 2018

Uh oh!

liggitt commented Apr 2, 2019

Uh oh!

wgliang commented Apr 2, 2019

Uh oh!

liggitt commented Apr 2, 2019

Uh oh!

k8s-ci-robot commented Apr 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

wgliang commented Jul 26, 2018 •

edited by liggitt

Loading

gnufied commented Oct 19, 2018 •

edited

Loading

liggitt commented Oct 26, 2018 •

edited

Loading