feat: k8s-sa-wif provider#1108
Conversation
|
This issue is currently awaiting triage. If the repository mantainers determine this is a relevant issue, they will accept it by applying the The DetailsInstructions 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-sigs/prow repository. |
|
Welcome @ihor-hrytskiv! |
|
Hi @ihor-hrytskiv. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ihor-hrytskiv The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Ihor Hrytskiv <ihor.hrytskiv@macpaw.com>
| ProjectNumber: os.Getenv("GCP_WIF_PROJECT_NUMBER"), | ||
| PoolId: os.Getenv("GCP_WIF_POOL_ID"), | ||
| ProviderId: os.Getenv("GCP_WIF_PROVIDER_ID"), | ||
| }, |
There was a problem hiding this comment.
You should also be able to pass these in via args to the get-credentials command as well. We do use this in GKE and we'd likely pass these in via args in the CredentialProviderConfig passed to the kubelet
| gcrAuthFlow = "gcr" | ||
| dockerConfigAuthFlow = "dockercfg" | ||
| dockerConfigURLAuthFlow = "dockercfg-url" | ||
| k8sSAWIFAuthFlow = "k8s-sa-wif" |
There was a problem hiding this comment.
it's probably fine to have a dedicated authflow, but we should be able to also use the gcr flow and then just check the CredentialProviderRequest to see if it contains the serviceaccount info.
There was a problem hiding this comment.
Hi @mastersingh24, thank you for your comment
Do you mean I should use the same gcrAuthFlow provider with two different logics? If CredentialProviderRequest contains a ServiceAccountToken, use the WIF-based logic; if it doesn’t, fall back to the standard gcrAuthFlow logic using the metadata server?
There was a problem hiding this comment.
@ihor-hrytskiv - that's what I was thinking. I believe that there will be cases in the same cluster where people use both flows so supporting both methods together makes sense.
There was a problem hiding this comment.
@mastersingh24 Could you maybe share an example where both flows would be used?
From what I understand, if the provider is running on a GCP Compute instance, we’d typically use the internal metadata server. And the service account token flow would be more relevant when running outside of GCP, since the metadata server isn’t available there.
There was a problem hiding this comment.
@mastersingh24 I’ve implemented new logic within the existing gcrAuthFlow. If that looks good, I’ll add tests.
Signed-off-by: Ihor Hrytskiv <ihor.hrytskiv@macpaw.com>
| klog.Errorf("while reading access token endpoint: %v", err) | ||
| return cfg | ||
| entry := credentialconfig.DockerConfigEntry{ | ||
| Username: "_token", |
There was a problem hiding this comment.
Can we confirm that Artifact Registry accepts _token as the username when the password is a Google access token? I see the existing provider uses _token, while the public Artifact Registry docs show oauth2accesstoken.
https://docs.cloud.google.com/artifact-registry/docs/docker/authentication#token
|
Since Container Registry does not support identity federation, should the WIF path return credentials only for Artifact Registry (*.pkg.dev)? |
|
PR needs rebase. DetailsInstructions 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-sigs/prow repository. |
This PR implements Service Account Token Integration for Image Pulls (GA), enabling kubelet to use Workload Identity Federation via GCP STS when retrieving credentials for pulling images. This change also resolves the issue described in #864