feat: SAVE IMAGE --without-earthly-labels #4084
Conversation
This commit adds the `--disable-earthly-labels` for the SAVE IMAGE instruction for not impacting on the digest of the final artifact. Certain custom registries may require strict checking of the image contents, so this option disables build information.
|
|
| myimagewithlabels: | ||
| FROM scratch | ||
| COPY file / | ||
| SAVE IMAGE myimagewithlabels:test |
There was a problem hiding this comment.
this will fail the +lint target; please make sure all files end with a newline.
ast/commandflag/flags.go
Outdated
| Insecure bool `long:"insecure" description:"Use unencrypted connection for the push"` | ||
| NoManifestList bool `long:"no-manifest-list" description:"Do not include a manifest list (specifying the platform) in the creation of the image"` | ||
| CacheFrom []string `long:"cache-from" description:"Declare additional cache import as a Docker tag"` | ||
| DisableEarthlyLabels bool `long:"disable-earthly-labels" description:"Disable build information dev.earthly labels to reduce the chance of changing images digsest."` |
There was a problem hiding this comment.
a little bit of a nit pick might be more consistent with other flags to call this :
no-earthly-labels maybe?
CC @alexcb
There was a problem hiding this comment.
That's a good suggestion, another idea is --without-earthly-labels.
There was a problem hiding this comment.
I like both, I would probably go with the shortest.
There was a problem hiding this comment.
The problem with --no-earthly-labels is that the feature attribute would be AllowNoEarthlyLabels, which is kinda confusing. without would probably don't have this issue, if we want to keep the consistency between feature and command flags.
tests/save-images/test.sh
Outdated
| @@ -63,4 +63,27 @@ echo "=== ($LINENO): Test New Behaviour referencing remote via build ===" | |||
| "$frontend" inspect myimage:buildtest >/dev/null | |||
| "$frontend" inspect earthly-test-saveimage:test >/dev/null | |||
|
|
|||
| echo "=== ($LINENO): Test Disable Earthly Labels ===" | |||
There was a problem hiding this comment.
Instead of having this test here, can you do it in an Earthfile instead?
Perhaps something along the lines of:
$ cat ./tests/with-docker-validate-labels/Earthfile
VERSION --allow-disable-earthly-labels 0.8 0.8
ARG --global DIND_IMAGE=earthly/dind:alpine-3.19-docker-25.0.5-r0
FROM $DIND_IMAGE
all:
BUILD +test-with-labels
BUILD +test-without-labels
myimage-with-labels:
FROM alpine
COPY file /
SAVE IMAGE myimage:test
myimage-without-labels:
FROM alpine
COPY file /
SAVE IMAGE myimage:test
test-with-labels:
WITH DOCKER --load=+myimage-with-labels
RUN docker inspect myimage:test | jq -r '.[].Config.Labels' # ....
END
test-without-labels:
...
then under tests/Earthfile, you can call it by adding it to
ga-no-qemu-group12:
...
BUILD --pass-args ./with-docker-validate-labels+all
features/features.go
Outdated
| @@ -74,6 +74,7 @@ type Features struct { | |||
| WildcardCopy bool `long:"wildcard-copy" description:"allow for the expansion of wildcard (glob) paths for COPY commands"` | |||
| RawOutput bool `long:"raw-output" description:"allow for --raw-output on RUN commands"` | |||
| GitAuthorEmailNameArgs bool `long:"git-author-email-name-args" description:"includes EARTHLY_GIT_AUTHOR_EMAIL and EARTHLY_GIT_AUTHOR_NAME builtin ARGs"` | |||
| AllowDisableEarthlyLabels bool `long:"allow-disable-earthly-labels" description:"Allow the usage of --disable-earthly-labels in SAVE IMAGE"` | |||
There was a problem hiding this comment.
I like that you didn't call it --enable-disable-earthly-labels, the --allow-* prefix was a good call 😄
There was a problem hiding this comment.
If we change the SAVE IMAGE to --no-earthly-labels, the feature attribute might need to be changed to AllowNoEarthlyLabels. The "AllowNo" prefix sounds confusing, probably it makes more sense AllowWithoutEarhtlyLabels.
earthfile2llb/interpreter.go
Outdated
| err = i.converter.Label(ctx, labels) | ||
| if err != nil { | ||
| return i.wrapError(err, cmd.SourceLocation, "failed to create dev.earthly.* labels during SAVE IMAGE") | ||
| if !opts.DisableEarthlyLabels { |
There was a problem hiding this comment.
there should be a check to make sure the AllowDisableEarthlyLabels feature was enabled.
e.g.
if opts.DisableEarthlyLabels {
if !i.converter.ftrs.AllowDisableEarthlyLabels {
return i.errorf(cmd.SourceLocation, "the SAVE IMAGE --disable-earthly-labels flag must be enabled with the VERSION --allow-disable-earthly-labels feature flag.")
}
// deliberately don't add any labels (i.e. do nothing here)
} else {
...
}
- Renamed feature to --allow-without-eartly-labels - Tests corrected and dockerized
|
New commit with changes in the review pushed. cc /@alexcb |
Co-authored-by: Alex Couture-Beil <[email protected]>
alexcb
added
approved-for-tests
alexcb
changed the title
This commit adds the
--without-earthly-labelsfor the SAVE IMAGE instruction to not modify the digest of the final artifact if none of its contents changes. Enabled--allow-without-earthly-labelsfeature.Certain custom registries may require strict checking of the image contents, so this option disables build information.