dockerTools.streamLayeredImage: resolve duplicate env vars

[lbpdt]

Motivation for this change

For images running on Kubernetes, there is no guarantee on how duplicate
environment variables in the image config will be handled. This seems
to be different from Docker, where the last environment variable value
is consistently selected.

The current code for streamLayeredImage was exploiting that assumption
to easily propagate environment variables from the base image, leaving
duplicates unchecked. It should rather resolve these duplicates to
ensure consistent behavior on Docker and Kubernetes.

Current tests for base image environment propagation should keep passing.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[lbpdt]

@GrahamcOfBorg test docker-tools

[roberth]

Could you add a test case?

[lbpdt]

I briefly touched upon that in the PR description, but I don't think a test case is doable here. This behavior is only triggered when running images on Kubernetes and not Docker, so I don't see how the NixOS Docker-based tests can help here.

Here's what happens when you load an image with duplicate config.Env variables:

• On Docker, the last value will always be selected when duplicate variable names are listed in config.Env.
• On Kubernetes, the image config is loaded and Kubernetes computes the full set of environment variables it wants to inject in the running container. When duplicate variable names appear in config.Env, it selected the first occurrence in my tests. So even if you run on top of Docker, Kubernetes overrides the default environment variables that Docker would have computed.

I don't know if the OCI image spec is clear on how duplicate config.Env keys should be handled, but I wouldn't be surprised if it was left as undefined behavior. This patch resolves the ambiguity in the image config directly, preserving the intended outcome where the last value wins if there are duplicates (i.e. you override an env var from base image).

As we already have tests checking that env var inheritance works as intended, I think it is enough to confirm that this patch did not break them.

What do you think?

[roberth]

You could untar the image and look at the config json, to confirm that it only has the expected entries, so no duplicates.

I think it is enough to confirm that this patch did not break them.

Which is exactly why we need to grow the test suite when we discover new requirements like this.

[lbpdt]

You could untar the image and look at the config json, to confirm that it only has the expected entries, so no duplicates.

I added a test that does that.

Which is exactly why we need to grow the test suite when we discover new requirements like this.

Agreed - I wasn't sure if we wanted this test suite to also cover non-Docker requirements but glad that's the ambition.

[roberth]

Thank you!