Bug: Makisu invokes GCR credential helper with environment variables defined in Dockerfile

[geekflyer]

Hi,

I noticed a very strange bug, where Makisu invokes the GCR credential helper with environment variables set in ENV sections in the Dockerfile instead of Makisu's own environment variables.

This breaks builds where a Dockerfile sets an environment variable that is also used by the credential helper, like GOOGLE_APPLICATION_CREDENTIALS, effectively creating a conflict.

Consider this example:

1. Create this dockerfile:

Dockerfile:

FROM alpine:latest

ENV GOOGLE_APPLICATION_CREDENTIALS=/path/inside/dockerfile/key.json

RUN echo bla

2. Create a GCP service account key with roles/storage.admin access to the GCR GCS bucket and save it under key.json in the same working directory as the Dockerfile

3. Run this command to attempt building and pushing the image to GCR

docker run -i --rm --net host \
        -v /var/run/docker.sock:/docker.sock \
        -e DOCKER_HOST=unix:///docker.sock \
        -e SSL_CERT_DIR=/makisu-internal/certs \
        -e GOOGLE_APPLICATION_CREDENTIALS=/makisu-context/key.json \
        -v $(pwd):/makisu-context \
        -v /tmp/makisu-storage:/makisu-storage \
        gcr.io/makisu-project/makisu-alpine:v0.1.11 build --registry-config='{ "gcr.io": { "*/*": { "push_chunk": -1, "security": { "credsStore": "gcr" } } } }' --modifyfs=true /makisu-context -t=my-project/makisu-bug:latest --push=gcr.io

The build will fail with a message similar to this:

error getting credentials using GOOGLE_APPLICATION_CREDENTIALS environment variable: open /path/inside/dockerfile/key.json: no such file or directory.

As you can see the credential helper receives the environment variable: GOOGLE_APPLICATION_CREDENTIALS=/path/inside/container/key.json but the correct one would be: GOOGLE_APPLICATION_CREDENTIALS=/makisu-context/key.json.

The detailed error message is below:

1.564308369407528e+09   info    * Moving directories [] to /makisu-storage/sandbox/sandbox612244624/stages/MA==
1.5643083694252641e+09  error   Failed to push cache: push layer sha256:0503825856099e6adb39c8297af09547f69684b7016b7f3680ed801aa310baaa: check layer exists: gcr.io/my-project/makisu-bug (sha256:0503825856099e6adb39c8297af09547f69684b7016b7f3680ed801aa310baaa): get security opt: get credentials: get credentials from helper gcr: error getting credentials - err: exit status 1, out: `docker-credential-gcr/helper: could not retrieve GCR's access token: google: error getting credentials using GOOGLE_APPLICATION_CREDENTIALS environment variable: open /path/inside/dockerfile/key.json: no such file or directory`; push layer sha256:41d4e8690e798dc7d7edfda8a6619ba8d22285cec292f61034c954c64a0a672a: check layer exists: gcr.io/my-project/makisu-bug (sha256:41d4e8690e798dc7d7edfda8a6619ba8d22285cec292f61034c954c64a0a672a): get security opt: get credentials: get credentials from helper gcr: error getting credentials - err: exit status 1, out: `docker-credential-gcr/helper: could not retrieve GCR's access token: google: error getting credentials using GOOGLE_APPLICATION_CREDENTIALS environment variable: open /path/inside/dockerfile/key.json: no such file or directory`
1.5643083694460058e+09  info    Computed total image size 5644457       {"total_image_size": 5644457}
1.5643083694460492e+09  info    Successfully built image my-project/makisu-bug:latest
1.5643083694461188e+09  info    * Started pushing image gcr.io/my-project/makisu-bug:latest
1.5643083694710586e+09  error   failed to push image: failed to push image: check manifest exists for image gcr.io/my-project/makisu-bug:latest: get security opt: get credentials: get credentials from helper gcr: error getting credentials - err: exit status 1, out: `docker-credential-gcr/helper: could not retrieve GCR's access token: google: error getting credentials using GOOGLE_APPLICATION_CREDENTIALS environment variable: open /path/inside/dockerfile/key.json: no such file or directory`

If I remove the line

ENV GOOGLE_APPLICATION_CREDENTIALS=/path/inside/dockerfile/key.json

from the Dockerfile, the build and push succeeds.

Summary

As you can see from the example Makisu is somehow invoking the docker-credential-gcr/helper with environment variables defined in the Dockerfile, which is almost certainly a bug and not how it should work.

The docker-credential-gcr helper should only inherit the environment variables being passed to makisu and not the ones defined in the Dockerfile.

We have some apps which set the GOOGLE_APPLICATION_CREDENTIALS environment variable in their Dockerfile and if they do so it completely breaks the makisu build.

[geekflyer]

@yiranwang52 @evelynl94 Any thoughts on this one? This is blocking some of our repos from adopting makisu unfortunately.

[evelynl94]

This is tricky a question. Makisu treats the envs passed in via flag and envs in Dockerfile the same, and hence a step in Dockerfile can overwrite the value. I'm not sure if we want to make them distinguishable because Makisu should not make too many assumptions about the environment imo.

For your users, should /path/inside/dockerfile/key.json be included in the final image? Is it possible to use mount as a workaround?

[geekflyer]

yes, for our use case the key.json should be included in the image (we know that this is generally not best practice, but this is a special case and the key has very few privileges).

As for the general env behaviour of makisu: Do the makisu build steps always automatically inherit the environment of the makisu caller? Isn't this a bit of a security and separation concern?

In docker we always have to explicitly use -e to propagate environment vars into the builder layers.

I think if makisu (intentional or not) merges the caller with the layer environment this design decision and the behaviour should be documented somewhere as this will otherwise lead to a lot of weird surprises of some users (like me) down the road.

[geekflyer]

For the short term I think it would be definately better if things like the credential helpers use an immutable "copy" of the environment at the beginning of the makisu invocation and not allow this environment to get mutated by the docker layers. It's fine to have the layers inherit the makisu environment, but not allow overriding each other.

[evelynl94]

As for the general env behaviour of makisu: Do the makisu build steps always automatically inherit the environment of the makisu caller?

I believe so.

For the short term I think it would be definately better if things like the credential helpers use an immutable "copy" of the environment at the beginning of the makisu invocation and not allow this environment to get mutated by the docker layers.

Yea we actually have discussed splitting fetch-build-push into separated and isolated phases, because we have concerns in including all different credHelpers in our images and setting up environments for them. But no work has been done on that front yet.