1255017 - Use vendored tooltool.py in docker images

Status: RESOLVED WORKSFORME

(Taskcluster Graveyard :: Docker Images, defect)

Description

[Selena Deckelmann :selenamarie :selena]

In our ongoing effort to secure our docker images, we're going to checksum-verify the download of tooltool.py (it's already https, and from github.. so this is an "abundance of caution" move).

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

I believe with recent patches this will now be embedded in the docker image based on an in-tree copy of tooltool.py

Comment 2

[Gregory Szorc [:gps]]

Yes, some of this is being done in bug 1247168. But in my working directory, there are still 6 references to the build-tooltool repo on github.

This bug could track the follow-up work to use vendored tooltool from the remaining images.

Comment 3

[Gregory Szorc [:gps]]

I'll crank this out real quick.

Comment 4

[Gregory Szorc [:gps]]

Review series coming up. I'm going to avoid desktop-build in this series because I have another series overhauling that image and I don't want to bit rot.

Comment 5

[Gregory Szorc [:gps]]

Attachment: Bug 1255017 - Use vendored tooltool.py in android-gradle-build;

Review commit: https://reviewboard.mozilla.org/r/66932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66932/

Comment 6

[Gregory Szorc [:gps]]

Attachment: Bug 1255017 - Use vendored tooltool.py in rust-build;

Review commit: https://reviewboard.mozilla.org/r/66934/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66934/

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1255017 - Use vendored tooltool.py in desktop-l10n;

Review commit: https://reviewboard.mozilla.org/r/66936/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66936/

Comment 8

[Gregory Szorc [:gps]]

Attachment: Bug 1255017 - Use vendored tooltool.py in ubuntu1204-test;

Review commit: https://reviewboard.mozilla.org/r/66938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66938/

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8774500 [details]
Bug 1255017 - Use vendored tooltool.py in ubuntu1204-test;

https://reviewboard.mozilla.org/r/66938/#review64212

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8774499 [details]
Bug 1255017 - Use vendored tooltool.py in desktop-l10n;

https://reviewboard.mozilla.org/r/66936/#review64210

::: testing/docker/desktop-l10n/Dockerfile:9
(Diff revision 1)
>  ADD             bin /home/worker/bin
>  ADD		.hgrc /home/worker/.hgrc
>  RUN             chmod +x /home/worker/bin/* 
>  
> +# %include testing/docker/decision/tooltool.py
> +ADD topsrcdir/testing/docker/decision/tooltool.py /builds/tooltool.py

I don't actually see this image referenced anywhere in-tree.  It was created in bug 1248713.  That said, I don't see any harm to this change.

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8774497 [details]
Bug 1255017 - Use vendored tooltool.py in android-gradle-build;

https://reviewboard.mozilla.org/r/66932/#review64204

::: testing/docker/android-gradle-build/Dockerfile:69
(Diff revision 1)
>    && mv /tmp/nexus-${NEXUS_VERSION}/* /opt/sonatype/nexus/ \
>    && rm -rf /tmp/nexus-${NEXUS_VERSION} \
>    && rm -rf /tmp/nexus-${NEXUS_VERSION}-bundle.tar.gz
>  
> -# Install tooltool directly from github.
> -RUN mkdir /build
> +# %include testing/docker/decision/tooltool.py
> +ADD topsrcdir/testing/docker/decision/tooltool.py /build/tooltool.py

Why is it vendored in testing/docker/decision?  Why not somewhere more common-sounding, like `python/tooltool.py` or `build/util/tooltool.py`?

::: testing/docker/android-gradle-build/Dockerfile
(Diff revision 1)
>    && rm -rf /tmp/nexus-${NEXUS_VERSION}-bundle.tar.gz
>  
> -# Install tooltool directly from github.
> -RUN mkdir /build
> +# %include testing/docker/decision/tooltool.py
> +ADD topsrcdir/testing/docker/decision/tooltool.py /build/tooltool.py
> -ADD https://raw.githubusercontent.com/mozilla/build-tooltool/master/tooltool.py /build/tooltool.py
> -RUN chmod +rx /build/tooltool.py

It turns out this path was totally bogus.

    $ grep -r /build/tooltool.py testing/mozharness/ | wc -l
    0
    $ grep -r /builds/tooltool.py testing/mozharness/ | wc -l
    46


Nothing uses `/build/tooltool.py`, and everything uses `/builds/tooltool.py`.  I know, I know, I hate it too.

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8774498 [details]
Bug 1255017 - Use vendored tooltool.py in rust-build;

https://reviewboard.mozilla.org/r/66934/#review64206

::: testing/docker/rust-build/Dockerfile:15
(Diff revision 1)
>  RUN yum clean all
>  
>  # Install tooltool directly from github.
>  RUN mkdir /builds
> -ADD https://raw.githubusercontent.com/mozilla/build-tooltool/master/tooltool.py /build/tooltool.py
> -RUN chmod +rx /build/tooltool.py
> +# %include testing/docker/decision/tooltool.py
> +ADD topsrcdir/testing/docker/decision/tooltool.py /build/tooltool.py

But it looks like rust (which doesn't build from the gecko tree and does not use mozharness) *does* use `/build` -- at least, the entrypoint on line 33 is `/build/fetch_rust.sh`.  So this is fine and doesn't need to be `/builds`.

Comment 13

[Ralph Giles (:rillian)]

Attachment: Use vendored tooltool.py in rust-build

I just bit-rot the rust-build patch in bug 1289605. Here's a rebase. Also fixes the out-of-date comment.

Comment 14

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/66932/#review64204

> Why is it vendored in testing/docker/decision?  Why not somewhere more common-sounding, like `python/tooltool.py` or `build/util/tooltool.py`?

Because the custom Dockerfile processing for custom image build contexts doesn't work with build.sh. I've addressed this in bug 1290531. I'll rebase this series on top of it.

Comment 15

[Chris Cooper [:coop] (he/him)]

Found this in triage today.

gps: is this still required, or even on your radar?

Comment 16

[Gregory Szorc [:gps]]

I no longer see any references to out-of-tree tooltool.py in taskcluster/docker. Pretty sure this was gradually cleaned up along with refactoring and deletion of various images occurring across various bugs.