Add --unsetenv option to buildah commit and build

[rhatdan]

This option will allow users to remove environment variables from the
final image.

Fixes: #3512

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[dilyanpalauzov]

Isn’t $PATH the only use case? Thus thereis no need for a parameter to the --unsetenv command line option.

[dilyanpalauzov]

I still think the PATH variable is the only use case and therefore this switch shall be called --unsetpathenv without parameters.

[dilyanpalauzov]

The Containerfile specification, as far as I remember does not say that there is an implicit PATH variable, which this new command line parameter is supposed to disable.

[rhatdan]

No, I believe if you base on top of a lower image, the new image will inherit the environment from the base image, If you want to remove those environment variables, this would come in handy.

[TomSweeneyRedHat]

It would be nice to have an example in the EXAMPLE section, ditto elsewhere.

[TomSweeneyRedHat]

Tests aren't hip and it looks like you need a rebase. In the tests, it would be good to define an ENVAR and then unset it rather than relying on PATH.

[dilyanpalauzov]

No, I believe if you base on top of a lower image, the new image will inherit the environment from the base image, If you want to remove those environment variables, this would come in handy.

Has somebody ever come to the idea to remove variables, set by lower images?

I think the only use case for this change is to suppress the implicit PATH when interpreting a Containerfile. This implicit PATH is nowhere documented. The tests shall have the PATH variables in mind.

[rhatdan]

$ cat /tmp/Containerfile 
from ubi8-init
run echo hi
$ ./bin/buildah build --tag dan /tmp
STEP 1/2: FROM ubi8-init
STEP 2/2: run echo hi
hi
COMMIT dan
Getting image source signatures
Copying blob 5bc03dec6239 skipped: already exists  
Copying blob 525ed45dbdb1 skipped: already exists  
Copying blob a363948ee32d skipped: already exists  
Copying blob 0faab747557f done  
Copying config 67cdfe9d6a done  
Writing manifest to image destination
Storing signatures
--> 67cdfe9d6a7
Successfully tagged localhost/dan:latest
67cdfe9d6a7886d17efd6f4c40aee6454281bc6d5298a23a2341e8c08ded2f07
$ buildah inspect --format '{{ .OCIv1.Config.Env }}' dan
[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin container=oci]

Since ubi8-init defines PATH and container, you see both defined in the new image.
If I don't want container defined, in my new image I can do the following command.

 $ ./bin/buildah build --unsetenv container --tag dan /tmp
STEP 1/2: FROM ubi8-init
STEP 2/2: run echo hi
hi
COMMIT dan
Getting image source signatures
Copying blob 5bc03dec6239 skipped: already exists  
Copying blob 525ed45dbdb1 skipped: already exists  
Copying blob a363948ee32d skipped: already exists  
Copying blob 593493bf944c done  
Copying config 3f23c8b6e4 done  
Writing manifest to image destination
Storing signatures
--> 3f23c8b6e4c
Successfully tagged localhost/dan:latest
3f23c8b6e4cbf1236185f960038aa04a8f3ab2798e7cb51464bd76d1d275bdba
$ buildah inspect --format '{{ .OCIv1.Config.Env }}' dan
[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin]

Images can have multiple environment variables defined within them, This way a user can base an image off of a lower image, and remove environment variables.

[dilyanpalauzov]

How exactly does Containerfile’s FROM (non-scratch) work? Does it unfold the base image, add the current Container-file instruction and create a new images, so that the new image is self-contained and when moved to a new system, the inital base image does not have to be downloaded?

Or creating FROM base image creates a new layer and a reference to the base image? Thus when the new image is used, the newest layer is downloaded and the base image is downloaded, too?

[dilyanpalauzov]

Ah, I see there is a "org.opencontainers.image.base.name" field in the "schemaVersion":2,"config":{"mediaType":"application/vnd.oci.image.config.v1+json" object.

I have:

$ podman images
docker.io/library/alpine  latest      14119a10abf4  2 months ago        5.87 MB

$ mkdir L
$ skopeo copy containers-storage:1411 oci:L
$ grep -r path L/

L/blobs/sha256/696d33ca1510966c426bdcc0daf05f75990d68c4eb820f615edccf7b971935e7:{"created":"2021-08-27T17:19:45.758611523Z","architecture":"amd64","os":"linux","config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"],"Cmd":["/bin/sh"]},"rootfs":{"type":"layers","diff_ids":["sha256:e2eb06d8af8218cfec8210147357a68b7e13f7c485b991c288c2d01dc228bb68"]},"history":[{"created":"2021-08-27T17:19:45.553092363Z","created_by":"/bin/sh -c #(nop) ADD file:aad4290d27580cc1a094ffaf98c3ca2fc5d699fe695dfb8e6e9fac20f1129450 in / "},{"created":"2021-08-27T17:19:45.758611523Z","created_by":"/bin/sh -c #(nop)  CMD [\"/bin/sh\"]","empty_layer":true}]}

$ cat Containerfile
FROM alpine
CMD /bin/echo "[$PATH]"

$ buildah bud --unsetenv PATH
STEP 1/2: FROM alpine
STEP 2/2: CMD /bin/echo "[$PATH]"
COMMIT
Getting image source signatures
Copying blob e2eb06d8af82 skipped: already exists  
Copying blob 5f70bf18a086 skipped: already exists  
Copying config e8b4d08029 done  
Writing manifest to image destination
Storing signatures
--> e8b4d080294
e8b4d08029434d503a1917fca8e8f5014833b61c48221bc927c0ee8881064837

$ podman run e8b4d080294
[/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin]

$ skopeo copy containers-storage:e8b4 oci:F

so the Alpine image (L/ directory) definition contains config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"]}. The derived image (F/ directory) does not contain it, but printing echo $PATH does print the $PATH from the base image. So not setting PATH in the newest layer, keeps PATH from the lower layer. Is there an OCI conformat way to remove a variable from a lower layer? It can certainly be set to the empty string, but this is just changing the variable value, which anyway can be done, it is not removing variable from the container.

I have built buildah on the rhatdan:unsetenv branch.

[rhatdan]

Podman will add the PATH environment itself, if the image does not contain it. (Docker will too, I believe.)

[rhatdan]

@flouthoc @TomSweeneyRedHat @nalind @ashley-cui @umohnani8 @vrothberg @giuseppe PTAL

[giuseppe]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [giuseppe,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ashley-cui]

/lgtm