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
implement GCP passthrough mode (and add service API checks) #86
implement GCP passthrough mode (and add service API checks) #86
Conversation
/retest |
1c4c238
to
b3a53bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great just a few thoughts
pkg/controller/utils/gcp/utils.go
Outdated
@@ -110,14 +115,31 @@ func checkServicesAndPermissions(gcpClient ccgcp.Client, permissionsList []strin | |||
return servicesEnabled, nil | |||
} | |||
|
|||
func filterPermissionsList(list []string) []string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function a bit unclear, needs either a rename, godoc, and/or a rename of the "list" var. i.e. what are we filtering for and why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i made it more generic and have now defined a list of permissions we know fail the TestIamPermissions() call when done in project scope
LGTM, lets squash, thanks! |
- [x] handle the case when the root creds are in passthrough mode. - [x] add checks for passthrough and mint mode where we check that the service APIs are enabled/available before proceeding to process the CredentialsRequest. Note that passthrough mode is handled differently than in AWS. In AWS we work with a static list of permissions that are needed for the cluster to run with. For GCP passthrough mode, the decision on whether we can satisfy a CredentialsRequest is dynamic. All we absolutely need for passthrough mode is the ability to list service APIs (to determine whether any particular service API is enabled), the ability to get the details for a specific role (so we can determine whether the role exists and the permissions attached to that role), and the ability to get the details of a project (so we can get the project number for a given project name). All the other permissions attached to the root creds in passthrough mode would pass/fail a permissions check during the TestIamPermissions() call.
78cb476
to
9e1d794
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, joelddiaz 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
Note that passthrough mode is handled differently than in AWS. In AWS we work with a static list of permissions that are needed for the cluster to run with. For GCP passthrough mode, the decision on whether we can satisfy a CredentialsRequest is dynamic. All we absolutely need for passthrough mode is the ability to list service APIs (to determine whether any particular service API is enabled), the ability to get the details for a specific role (so we can determine whether the role exists and the permissions attached to that role), and the ability to get the details of a project (so we can get the project number for a given project name). All the other permissions attached to the root creds in passthrough mode would pass/fail a permissions check during the TestIamPermissions() calls.