https://pantsbuild.org/ logo
#development
Title
# development
h

hundreds-father-404

09/15/2022, 12:17 AM
@fast-nail-55400 @witty-crayon-22786 I didn't totally follow the conversation from yesterday. What's the current thinking for whether Docker should be stripping all env vars?
f

fast-nail-55400

09/15/2022, 12:25 AM
What do you mean by "stripping all env vars"? My point was that Docker does not offer a way to wipe out all environment variables that the image would set.
It offers a way to unset variables set in an image, but that requires a user know in advance what env vars they want to unset.
h

hundreds-father-404

09/15/2022, 12:26 AM
Right, and we discussed the workaround of running every process via a script that unsets every env var, then resets the opted-into ones. Are we thinking we should do that?
f

fast-nail-55400

09/15/2022, 12:28 AM
I don't believe we should. Users may want env vars that come from the image.
h

hundreds-father-404

09/15/2022, 12:33 AM
don't we break caching hermiticity without stripping? I don't see why Docker would fundamentally be different than local environments. W/ local, users often indeed also want env vars from the host
f

fast-nail-55400

09/15/2022, 12:37 AM
if every user uses the same image, then the env vars would be the same for all of the users, right?
👍 1
w

witty-crayon-22786

09/15/2022, 12:39 AM
Extraction will pull a complete list of env vars from the image.
Various contexts will then set or reset some of those env vars
f

fast-nail-55400

09/15/2022, 12:39 AM
for example, take an official Rust image (
rust:1.63.0
) which sets
CARGO_HOME
and
RUSTUP_HOME
. Why should Pants supply those values?
w

witty-crayon-22786

09/15/2022, 12:39 AM
Various contexts will then set or reset some of those env vars
...which might override the vars set inside the image.
It's possible that we will additionally want to do something to purge other env vars inside the image to align with local. But the fact that local purges all env vars will mean that in most cases, the fact that we don't purge other env vars, and only override / reset ones which are known relevant should be sufficient for now.
i.e. any sort of additional purging isn't necessary in a first version I think.
h

happy-kitchen-89482

09/15/2022, 1:32 AM
Isn’t the image identity part of the cache key?
So whatever env vars it sets are accounted for?
h

hundreds-father-404

09/15/2022, 3:01 AM
if every user uses the same image, then the env vars would be the same for all of the users, right?
Alright that now makes sense to me to not strip. https://github.com/pantsbuild/pants/pull/16876 and https://github.com/pantsbuild/pants/issues/16877 only downside imo is not having parity with local environments. But that seems like a weak reason to force users to add a bunch of boilerplate to enable env vars they would reasonably expect to be enabled
f

fast-nail-55400

09/15/2022, 3:06 AM
Isn’t the image identity part of the cache key?
Unclear to me how we account for it currently. It is easy enough to retrieve the SHA256 hash of the image and use that value somewhere.
w

witty-crayon-22786

09/15/2022, 3:21 AM
Relatedly: that sha256 for a particular version of the image is likely what we should be including in the Process struct, rather than the original input name.