Skip to content

📖 Single controller multitenancy proposal v3 #1713

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
Merged
merged 3 commits into from
Jun 1, 2020
Merged

📖 Single controller multitenancy proposal v3 #1713

merged 3 commits into from
Jun 1, 2020

Conversation

randomvariable
Copy link
Member

@randomvariable randomvariable commented May 6, 2020 •
edited
Loading

What this PR does / why we need it:
Succeeds #1674

Updated multi-tenancy proposal after discussion on 2020-05-01. Principals have been moved to being cluster scoped, and follow the model of GatewayClass within https://github.com/kubernetes-sigs/service-apis . Seems to be the best compromise given current Kubernetes RBAC.

CC:

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 #

Additionally relabel all the old proposals with dates.

Naadir Jeewa added 2 commits May 6, 2020 14:04
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 6, 2020
@randomvariable randomvariable force-pushed the credential-provider branch 2 times, most recently from 03aa62a to 3656fb8 Compare May 6, 2020 15:06
@randomvariable
Copy link
Member Author

https://github.com/randomvariable/cluster-api-provider-aws/blob/credential-provider/docs/proposal/20200506-single-controller-multitenancy.md is the file you need

@randomvariable randomvariable force-pushed the credential-provider branch from 3656fb8 to b438245 Compare May 6, 2020 15:14
This was referenced May 7, 2020
Closed
Closed
@bagnaram bagnaram mentioned this pull request May 7, 2020
Closed
@fabriziopandini fabriziopandini mentioned this pull request May 11, 2020
Closed
@randomvariable
Copy link
Member Author

/hold
for approvals

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2020
@sethp-nr
Copy link
Contributor

I am a big +1 on the design & can commit at least to doing some reviews if not implementation.

@vincepri
Copy link
Member

/assign

going to review next week

vincepri
vincepri reviewed May 19, 2020
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

Only reviewed half of it, going to continue tomorrow for the rest

@andrewmyhre
Copy link
Contributor

We're beginning an implementation today. It looks as though the principal types are no longer intended to be stored as secrets. That's an intentional change, correct?

@andrewmyhre
Copy link
Contributor

Also, I'd like to see PrincipalRef able to be overridden by the AWSMachineTemplate type.

detiber
detiber reviewed May 20, 2020
View reviewed changes
@randomvariable
Copy link
Member Author

Also, I'd like to see PrincipalRef able to be overridden by the AWSMachineTemplate type.

I really want to keep cross-VPC cluster support out of scope for now

@randomvariable randomvariable force-pushed the credential-provider branch 2 times, most recently from a9b03ad to 8ad5c23 Compare May 21, 2020 12:33
@randomvariable
Copy link
Member Author

randomvariable commented May 21, 2020 •
edited
Loading

Added a small section on adding cluster-scoped ownerReference support to clusterctl's object graph bits.

@randomvariable randomvariable force-pushed the credential-provider branch 2 times, most recently from cb761b8 to 14f4cbb Compare May 21, 2020 13:07
@randomvariable randomvariable force-pushed the credential-provider branch 3 times, most recently from 5606f54 to c5384be Compare May 21, 2020 13:26
@randomvariable randomvariable mentioned this pull request May 21, 2020
Closed
@detiber
Copy link
Contributor

detiber commented May 29, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber, randomvariable

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 May 29, 2020
@rudoi
Copy link
Contributor

rudoi commented May 29, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 29, 2020
@devigned
Copy link

Looks great! We'll queue up similar work in CAPZ. Thanks for leading the way on this.

@randomvariable
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 1, 2020
@k8s-ci-robot k8s-ci-robot merged commit 00885d9 into kubernetes-sigs:master Jun 1, 2020
@randomvariable randomvariable deleted the credential-provider branch June 1, 2020 13:29
@andrewmyhre andrewmyhre mentioned this pull request Jun 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ashish-amarnath ashish-amarnath Awaiting requested review from ashish-amarnath

@justinsb justinsb Awaiting requested review from justinsb

3 more reviewers

@andrewmyhre andrewmyhre andrewmyhre left review comments

@detiber detiber detiber left review comments

@vincepri vincepri vincepri approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@devigned devigned

@vincepri vincepri

@rudoi rudoi

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@randomvariable @sethp-nr @vincepri @andrewmyhre @detiber @k8s-ci-robot @rudoi @devigned