<@U0N6C2Q9F> <@U06A03HV1> I didn't totally follow ...
# developmenth
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 AMWhat 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.
fast-nail-55400
09/15/2022, 12:26 AMIt 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 AMRight, 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 AMI don't believe we should. Users may want env vars that come from the image.
h
hundreds-father-404
09/15/2022, 12:33 AMdon'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 AMif 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 AMExtraction will pull a complete list of env vars from the image.
witty-crayon-22786
09/15/2022, 12:39 AMVarious contexts will then set or reset some of those env vars
f
fast-nail-55400
09/15/2022, 12:39 AMfor example, take an official Rust image (
rust:1.63.0CARGO_HOMERUSTUP_HOMEw
witty-crayon-22786
09/15/2022, 12:39 AMVarious contexts will then set or reset some of those env vars
...which might override the vars set inside the image.
witty-crayon-22786
09/15/2022, 12:41 AMIt'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.
witty-crayon-22786
09/15/2022, 12:41 AMi.e. any sort of additional purging isn't necessary in a first version I think.
h
happy-kitchen-89482
09/15/2022, 1:32 AMIsn’t the image identity part of the cache key?
happy-kitchen-89482
09/15/2022, 1:32 AMSo whatever env vars it sets are accounted for?
h
hundreds-father-404
09/15/2022, 3:01 AMif 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 AMIsn’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 AMRelatedly: 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.