-
Notifications
You must be signed in to change notification settings - Fork 57
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.
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
Verify checksums of downloaded files in Dockerfile + remove curl -k flag in jq download #71
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use plain variables for sha and don't store them as env variables in the final image. Do you plan to use the env variables later on?
Ok yes thanks for the comment this makes sense. But I guess same applies to the versions right? I prefer to not separate version and sha to keep it clear that both must be updated when upgrading to a higher version |
I've pushed the change, but cannot build the image because of #72 . Any ideas how to solve this ? |
Dockerfile
Outdated
@@ -135,15 +142,17 @@ RUN echo " ... adding Openresty, NGINX, and PCRE" \ | |||
--without-http_scgi_module \ | |||
-j${NPROC} \ | |||
&& make -j${NPROC} \ | |||
&& make install \ | |||
&& make install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to complete this docker layer here, I'd clean it up too:
&& make install | |
&& apk del g++ gcc make \ | |
&& rm -rf /var/cache/apk/* \ | |
&& rm -rf /tmp/api-gateway |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed the change
ENV OPENRESTY_VERSION 1.13.6.1 | ||
ENV PCRE_VERSION 8.37 | ||
RUN echo " ... adding Openresty and PCRE" \ | ||
&& OPENRESTY_SHA256=d1246e6cfa81098eea56fb88693e980d3e6b8752afae686fab271519b81d696b \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for consistency, should we move OPENRESTY_VERSION
here as well, instead of having it as an ENV
? B/c the SHA and the ENV must be changed together when updating a version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I agree but OPENRESTY_VERSION
is used in other layers.
Hi,
This is a simple PR to verify the integrity of downloaded code and binaries in the Dockerfile, also remove uneeded usage of insecure curl -k flag.