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

WorkloadIdentityBinding operator #3456

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

justinsb
Copy link
Contributor

WIP, for discussion cc @mortent

@johnbelamaric
Copy link
Contributor

A few high-level comments, more-or-less documenting what we discussed in person:

  • A README or some description of what this is for would be helpful in understanding the code. Also maybe some examples?
  • This is a GCP only controller, I would expect that to be reflected in the naming or something. Or perhaps if we expect to have a number of these, we bundle them into a separate GCP-specific controller-manager.
  • Why does this need to be a controller? Could it be a function instead?
  • Does this only apply to WI for Porch itself? What about configuring WI for workloads running in other clusters for which Porch is just pushing packagerevisions to their deployment repos? This is more the scenario that would be useful for my use cases - a function I think could satisfy both of these (not sure though).

@justinsb justinsb force-pushed the workloadidentitybinding_operator branch 2 times, most recently from 373ec2a to 148beef Compare August 15, 2022 22:24
Setting up a pattern for more controllers, and adding a simple one to
help with setting up WorkloadIdentityBinding.
@justinsb justinsb force-pushed the workloadidentitybinding_operator branch from 148beef to e71c5ce Compare August 15, 2022 23:03
@justinsb justinsb changed the title WIP: WorkloadIdentityBinding operator WorkloadIdentityBinding operator Aug 16, 2022
@justinsb
Copy link
Contributor Author

justinsb commented Aug 16, 2022

Thanks @johnbelamaric!

A README or some description of what this is for would be helpful in understanding the code. Also maybe some examples?

Good idea - I'll add in a follow-on PR, as we don't want this to hold up the other PRs that build on it.

This is a GCP only controller, I would expect that to be reflected in the naming or something. Or perhaps if we expect to have a number of these, we bundle them into a separate GCP-specific controller-manager.

Good plan also, although WorkloadIdentity is a actually a GCP name, other clouds use different terms. It's alpha so I think we can iterate here.

Why does this need to be a controller? Could it be a function instead?

In this case, we need the identity-namespace/project-id that we're running in. That's not a piece of information that is really part of the package, it's more part of the deployment environment. I'm experimenting with building these packages, and this one really helps us out. Elsewhere I've been able to use a function. If we find we need to plumb through the identity namespace for other reasons, then I think this could well be a function. I actually suspect that this ends up in KCC (or a KCC-adjacent project), because really we're trying to make the KRM schema a lot more regular, vs encoding a lot of information in a "magic" pseudo-ServiceAccount name.

Does this only apply to WI for Porch itself? What about configuring WI for workloads running in other clusters for which Porch is just pushing packagerevisions to their deployment repos? This is more the scenario that would be useful for my use cases - a function I think could satisfy both of these (not sure though).

It is generic in terms of binding Kubernetes ServiceAccounts to GCP ServiceAccounts. It should run wherever the KSA is configured. I suspect in most cases KCC is running in the same cluster, configuring the GCP ServiceAccount, but this isn't required. (But, if something else is provisioning the GCP SA, probably that thing is a good candidate for configuring the KSA -> GCP SA IAM association).

@johnbelamaric
Copy link
Contributor

johnbelamaric commented Aug 18, 2022

That's not a piece of information that is really part of the package, it's more part of the deployment environment.

The deployment environment is known at the time of package configuration. For example we could annotate the deployment repo with this project ID. Or associate a “ClusterDemographics” or “ClusterGCPContext” resource with a repo. This is effectively what I did in the Nephio controller PoC with a CluaterScaleProfile.

@rajatchowdhary
Copy link

Any ETA for the Porch release that will include this operator?

@johnbelamaric
Copy link
Contributor

johnbelamaric commented Aug 18, 2022

The deployment environment is known at the time of package configuration.

Follow up. This is the point of this bullet in #3396:

We've identified the need for a deployment target "context", such as cluster targets #3387, along the lines of kubeconfig, #2715, terraform provider config, etc.

This function would have to run in the package that owns the KSA of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants