-
Notifications
You must be signed in to change notification settings - Fork 481
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
bake: support compose build secrets #1069
Conversation
Signed-off-by: CrazyMax <[email protected]>
4ded8e6
to
52d4cb9
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.
LGTM
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 guess in the future we can discuss how this could be changed to a typed structure. But hacky to encode csv so it can be parsed again. (There is also a plan to allow defining the secret keys in json/hcl directly instead of csv).
bake/compose.go
Outdated
func composeToBuildkitSecret(inp compose.ServiceSecretConfig, projectSecrets compose.Secrets) (string, error) { | ||
psecret := projectSecrets[inp.Source] | ||
if psecret.External.External { | ||
return "", fmt.Errorf("unsupported external secret %s", psecret.Name) |
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.
errors.Errorf
bake/compose.go
Outdated
|
||
// composeToBuildkitSecret converts secret from compose format to buildkit's | ||
// csv format. | ||
func composeToBuildkitSecret(inp compose.ServiceSecretConfig, projectSecrets compose.Secrets) (string, error) { |
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.
Why doesn't this just take psecret
as parameter?
52d4cb9
to
f9f763a
Compare
bake/compose.go
Outdated
if psecret.File != "" { | ||
bkattrs = append(bkattrs, "src="+psecret.File) | ||
} | ||
if inp.Target != "" { |
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.
What are these things: targets, uid, gid, mode? These can't be set by the flags but are determined by the definition in Dockerfile.
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.
Oh my bad I confused the secret mount in the Dockerfile syntax with the secret flag!
Yes typed structure would be nice and also expose a contract in BuildKit so we don't have hardcoded attributes that could potentially drift. |
Signed-off-by: CrazyMax <[email protected]>
f9f763a
to
c0f8a83
Compare
@glours Looking at this, adding these fields in the compose-spec might have been out of scope for buildx. |
fixes #1060
Adds support for compose build secrets with bake. Needs to vendor compose-go compose-spec/compose-go@v1.2.1...v1.2.4. See spec https://github.com/compose-spec/compose-spec/blob/master/build.md#secrets.
We can keep the
x-bake
secret field for now imo.Also env secret type is supported with the following format:
I will review our documentation about bake and create dedicated guides for compose and hcl formats for this kind of use cases in a follow-up.
cc @glours @ciaranmcnulty