Skip to content

Use official checksums to verify Tini #55491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 24, 2020
Merged

Use official checksums to verify Tini #55491

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
067fb59
Merge pull request #1 from elastic/master
Apr 13, 2020
9875a82
Merge pull request #3 from elastic/master
Apr 20, 2020
57fc364
Removed generated tini SHAs and now pull from SHAs uploaded to releas…
Apr 20, 2020
73af316
Utilize switch case for arch info and bring in changes suggested by p…
Apr 22, 2020
8909ebc
Merge branch 'master' into tini_sha
elasticmachine Apr 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions distribution/docker/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ ext.expansions = { architecture, oss, local ->
final String classifier = "aarch64".equals(architecture) ? "linux-aarch64" : "linux-x86_64"
final String elasticsearch = oss ? "elasticsearch-oss-${VersionProperties.elasticsearch}-${classifier}.tar.gz" : "elasticsearch-${VersionProperties.elasticsearch}-${classifier}.tar.gz"
return [
'base_image' : "aarch64".equals(architecture) ? "arm64v8/centos:7" : "centos:7",
'base_image' : "centos:7",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasontedor Question for you, since centos uses manifests for their Docker builds pulling from the specific arch is unneeded. Was there another reason to directly pull from arm64v8/centos:7? Or would pulling from centos:7 work? You can see the manifests/archs that Centos supports here, https://hub.docker.com/_/centos?tab=tags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thinking here is that using an explicit base image makes it possible to cross-build the ARM image, for example using Docker for Mac. It's entirely possible that we'll revert to just specifying centos:7 in the future however.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thinking here is that using an explicit base image makes it possible to cross-build the ARM image, for example using Docker for Mac.

Makes perfect sense! I went a head a made the amd64 image explicit too in case you’re not on a x86 system.

It's entirely possible that we'll revert to just specifying centos:7 in the future however.

Using manifests for this would help simplify things but all depends on your CI/CD. How are you currently building and publishing your images? Does Elastic have access to arm64 hardware that you will build the arm docker images on? Or are you planning on using qemu and binfmt_misc to emulate the architectures so you can build all the images on x86 hardware?

I am happy to lend a hand if you need when it comes to building multi-arch containers and building out manifests. Being at IBM, I do that quite a lot! 😄

Are you planning on producing manifests for your arm64 and amd64 images? That would be really cool to see!!

One quick note with using qemu and binfmt_misc is that there are some bugs you might run into. Images might seem like they built correctly, but when ran on the specific hardware you will run into errors. Hence why I encourage people to build on platform if they can. I am currently helping the Jenkins Docker team to build on platform for each of their images. AWS does offer arm64 resources, in case you guys were interested.

'build_date' : BuildParams.buildDate,
'elasticsearch' : elasticsearch,
'git_revision' : BuildParams.gitRevision,
'license' : oss ? 'Apache-2.0' : 'Elastic-License',
'source_elasticsearch': local ? "COPY $elasticsearch /opt/" : "RUN cd /opt && curl --retry 8 -s -L -O https://artifacts.elastic.co/downloads/elasticsearch/${elasticsearch} && cd -",
'tini_suffix' : "aarch64".equals(architecture) ? "-arm64" : "",
'version' : VersionProperties.elasticsearch
]
}
Expand Down
14 changes: 8 additions & 6 deletions distribution/docker/src/docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
FROM ${base_image} AS builder

RUN for iter in {1..10}; do yum update --setopt=tsflags=nodocs -y && \
yum install --setopt=tsflags=nodocs -y gzip shadow-utils tar && \
yum install --setopt=tsflags=nodocs -y epel-release && yum install --setopt=tsflags=nodocs -y dpkg wget gzip shadow-utils tar && \
yum clean all && exit_code=0 && break || exit_code=\$? && echo "yum error: retry \$iter in 10s" && sleep 10; done; \
(exit \$exit_code)

Expand All @@ -40,11 +40,13 @@ RUN chmod 0660 config/elasticsearch.yml config/log4j2.properties
#
# The tini GitHub page gives instructions for verifying the binary using
# gpg, but the keyservers are slow to return the key and this can fail the
# build. Instead, we check the binary against a checksum that we have
# computed.
ADD https://github.com/krallin/tini/releases/download/v0.18.0/tini${tini_suffix} /tini
COPY config/tini${tini_suffix}.sha512 /tini.sha512
RUN sha512sum -c /tini.sha512 && chmod +x /tini
# build. Instead, we check the binary against a checksum that they provide.
RUN wget --no-check-certificate --no-cookies --quiet https://github.com/krallin/tini/releases/download/v0.19.0/tini-\$(dpkg --print-architecture) \
&& wget --no-check-certificate --no-cookies --quiet https://github.com/krallin/tini/releases/download/v0.19.0/tini-\$(dpkg --print-architecture).sha256sum \
&& echo "\$(cat tini-\$(dpkg --print-architecture).sha256sum)" | sha256sum -c \
&& rm -f tini-\$(dpkg --print-architecture).sha256sum \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to remove the checksum, since this is just the builder image.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I remove it was were still under the working dir of /usr/share/elasticsearch, which gets copied into the main image. Thus the checksum will be copied to the second stage.

What would be easier is to move the whole Tini "block" of code up before you change the WORKDIR to /usr/share/elasticsearch, around line 20. Do you mind if I move the Tini code "block" to line 20? I did not want to move to much stuff around, but if you think is a okay solution I will!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my latest commit, I moved the Tini code "block" up in the Dockerfile before you change WORKDIR, which eliminates the need to delete the checksum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that we removed the checksum file even if it's currently unneeded, because it means if there's a later refactoring that would for example, accidentally revert executing this in /usr/share/elasticsearch, then we don't have to "remember" to now delete the checksum file. It can be done as part of this RUN command, so not introducing a new layer (which doesn't even matter since it's in the builder image).

Copy link
Author

@james-crowley james-crowley Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasontedor @pugnascotia I am flexiable either way.

It can be done as part of this RUN command, so not introducing a new layer (which doesn't even matter since it's in the builder image).

I think this is key. If there is no new layer were not adding to the build time or build size. Plus, like @jasontedor, its just the builder image.

&& mv tini-\$(dpkg --print-architecture) /tini \
&& chmod +x /tini

################################################################################
# Build stage 1 (the actual elasticsearch image):
Expand Down
1 change: 0 additions & 1 deletion distribution/docker/src/docker/config/tini-arm64.sha512
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion distribution/docker/src/docker/config/tini.sha512
View file
Open in desktop

This file was deleted.