set GIT_AUTH_TOKEN secret if Git context used #284
Conversation
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
crazy-max
force-pushed
the
fix-git-auth-token
branch
from
2bbc921 to
a23a0ce
Compare
| // request: https://github.com/docker/buildx/pull/2905 | ||
| const gitAuthToken = getGitAuthToken(inputs); | ||
| if (gitAuthToken && !Bake.hasGitAuthTokenSecret(definition) && inputs.source.startsWith(Context.gitContext())) { | ||
| args.push('--set', `*.secrets=${Build.resolveSecretString(`GIT_AUTH_TOKEN=${gitAuthToken}`)}`); |
There was a problem hiding this comment.
Just to be sure, this does not overwrite existing secrets, right?
How does this looks as a raw value? The actual secret value should not be argument for --set.
There was a problem hiding this comment.
Just to be sure, this does not overwrite existing secrets, right?
Yes it doesn't, !Bake.hasGitAuthTokenSecret(definition) checks if GIT_AUTH_TOKEN is set first.
How does this looks as a raw value? The actual secret value should not be argument for
--set.
It creates a temp file setting the secret value: https://github.com/docker/bake-action/actions/runs/12707842078/job/35423580416#step:4:231
And is removed when job is completed: https://github.com/docker/bake-action/actions/runs/12707842078/job/35423580416#step:6:35
This is the same behavior in build-push-action repo.
There was a problem hiding this comment.
Yes it doesn't, !Bake.hasGitAuthTokenSecret(definition) checks if GIT_AUTH_TOKEN is set first.
I meant that if user has own secrets (not git) defined in HCL then this doesn't overwrite it. I don't remember how the merge logic was in this case.
It creates a temp file setting the secret value:
Temp file shouldn't be needed as buildx can just read the value from env, but this doesn't need to be part of this PR.
There was a problem hiding this comment.
I meant that if user has own secrets (not git) defined in HCL then this doesn't overwrite it. I don't remember how the merge logic was in this case.
Yes we check and skip if defined either through override or within the bake definition because we solve the definition first and look at existing secrets from there: https://github.com/docker/actions-toolkit/blob/54bdcf6c08a9b43d37df4156506648c978b372fe/src/buildx/bake.ts#L411-L423
Temp file shouldn't be needed as buildx can just read the value from
env, but this doesn't need to be part of this PR.
Yes we could improve that agree, env was not a thing before. Will open a follow-up.
This reverts commit 5a1b7c9, reversing changes made to ded8f8f. Signed-off-by: CrazyMax <[email protected]>
relates to docker/buildx#2905
Tested here: https://github.com/docker/bake-action/actions/runs/12707842078/job/35423580416#step:4:231