Use ARG instead of ENV for variables only used for labels

[travier]

The variables in https://github.com/sclorg/nginx-container/blob/master/1.22/Dockerfile.fedora#L11..L15 are only used to set LABELS a fe lines below and should not "leak" inside the container images.

Using ARG instead of ENV will make those variables only available during the build and not built into the image default environment.

Note: This also applies to all other container images in this org. I'm only reporting this once.

See https://github.com/containers/docs/pull/15

[pkubatrh]

Thanks for the report.

I can understand the logic behind it but I am very much against this change. Using ARG instead of ENV means that the variables will need to be passed for every image build (unless there is some way to set defaults?) or suffer warnings. Additionally this will also invalidate all layer caches resulting in longer builds, unless the ARG is only set near the end of the Dockerfile. What motivation for this change is there outside of not wanting to have the variables available inside the image for some reason?

[pkubatrh]

This sounds like a change that is aimed at OSBS builds (where these arguments are always available?) so one note from me on that point is, we are no longer building Fedora-based images in Fedora's OSBS system.

[travier]

Using ARG instead of ENV means that the variables will need to be passed for every image build (unless there is some way to set defaults?) or suffer warnings. Additionally this will also invalidate all layer caches resulting in longer builds, unless the ARG is only set near the end of the Dockerfile.

ARG will use the value set by default. No need to pass values for each build: https://docs.docker.com/engine/reference/builder/#default-values

This will not invalidate layers for each builds. Only the first time when we make the change.

The goal of this change is to avoid setting environment variables that are not needed by the application: https://github.com/containers/toolbox/issues/188

I'm trying to look at all Fedora containers to remove those variables from the containers environment as they are not needed to set labels.

This sounds like a change that is aimed at OSBS builds (where these arguments are always available?) so one note from me on that point is, we are no longer building Fedora-based images in Fedora's OSBS system.

This is the reverse: OSBS uses labels to figure out what's in the container, not the ENV or ARG vars. It does not use ARG right now so this change is not for that. OSBS built containers also don't need ENV and can use ARG instead. I'm pushing for that in https://github.com/containers/docs/pull/15

[travier]

If you don't like the ARG usage, you can also directly inline the values in the labels, that should work just fine.

[pkubatrh]

Thanks for the detailed explanation!

Played a bit with ARG in nginx's Fedora-based Dockerfile and it does behave as you described (at least with podman). I guess it really shows it has been a while since I last looked at ARG use. In that case, I am not against this change anymore, as there is basically no difference to current state. Please do note that we will have to check if the variables defined via ENV are really not used anywhere in the image's scripts (as is the case with NGINX_VERSION and NGINX_SHORT_VER), or modify the scripts accordingly so they do not have to be used.

No priority on our side to work on this in the near future, but we welcome PRs if you want to get this change in sooner that we are able to get to it.

[travier]

Great, will make PRs once I have the changes merged upstream.