-
Notifications
You must be signed in to change notification settings - Fork 130
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
Use Alpine Linux, multistage build #37
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
37fd5d4
use Alpine as base, latest nginx, multistage build
0b33368
make init script compatible with Alpine, use new docker compose plugin
839d8b6
allow for non-interactive installs
ad4f682
use sh for maximum compatibility
fead432
support both docker-compose and docker compose
9315994
fix if statement syntax
10a119c
Merge branch 'signalapp:main' into main
adduty 897ab1b
create dependabot.yml
adduty e9c758b
Merge pull request #1 from adduty/dependabot
adduty 7dd5007
specify directories
adduty 3d34a1d
Merge pull request #2 from adduty/dependabot
adduty File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
version: 2 | ||
updates: | ||
- package-ecosystem: "docker" | ||
directory: "/nginx-relay" | ||
schedule: | ||
interval: "weekly" | ||
- package-ecosystem: "docker" | ||
directory: "/nginx-terminate" | ||
schedule: | ||
interval: "weekly" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,44 @@ | ||
FROM ubuntu:20.04 | ||
FROM alpine:3.17 as build | ||
|
||
RUN apt-get update && apt-get -y upgrade && \ | ||
apt-get install -y wget libpcre3-dev build-essential libssl-dev zlib1g-dev && \ | ||
rm -rf /var/lib/apt/lists/* | ||
ARG NGINX_VER='1.22.1' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
|
||
RUN apk add --no-cache \ | ||
build-base \ | ||
libressl-dev \ | ||
pcre-dev \ | ||
zlib-dev | ||
|
||
WORKDIR /opt | ||
|
||
RUN wget https://nginx.org/download/nginx-1.18.0.tar.gz && \ | ||
tar -zxvf nginx-1.*.tar.gz && \ | ||
cd nginx-1.* && \ | ||
./configure --prefix=/opt/nginx --user=nginx --group=nginx --with-http_ssl_module --with-ipv6 --with-threads --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module && \ | ||
make && make install && \ | ||
cd .. && rm -rf nginx-1.* | ||
RUN wget "https://nginx.org/download/nginx-${NGINX_VER}.tar.gz" \ | ||
&& tar -zxvf "nginx-${NGINX_VER}.tar.gz" \ | ||
&& cd "nginx-${NGINX_VER}" \ | ||
&& ./configure \ | ||
--prefix=/opt/nginx \ | ||
--user=nginx \ | ||
--group=nginx \ | ||
--with-http_ssl_module \ | ||
--with-ipv6 \ | ||
--with-threads \ | ||
--with-stream \ | ||
--with-stream_ssl_module \ | ||
--with-stream_ssl_preread_module \ | ||
&& make && make install \ | ||
&& cd .. && rm -rf "nginx-${NGINX_VER}*" | ||
|
||
FROM alpine:3.17 as final | ||
|
||
RUN apk upgrade --update-cache \ | ||
&& apk add \ | ||
libressl3.6-libcrypto \ | ||
libressl3.6-libssl \ | ||
pcre \ | ||
&& rm -rf /var/cache/apk | ||
|
||
RUN adduser --system --no-create-home --disabled-login --disabled-password --group nginx | ||
COPY --from=build /opt/ /opt/ | ||
|
||
WORKDIR / | ||
RUN addgroup -S nginx \ | ||
&& adduser -SDHG nginx nginx | ||
|
||
EXPOSE 443 | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Having to maintain the latest nginx version is what led to the currently outdated nginx version in the current code. Having the version hardcoded in a variable is not a big improvement for maintainability over the current state of having it in the URL below. In both cases, the repo here has to be updated for every nginx release.
Could the official
nginx:latest
docker image be used as base, instead of having the specific version in the source code here?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.
Compare #23
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.
The reason I moved the nginx version to an ARG is because it is referenced in multiple places, so declaring it as a variable will make it less likely to be missed somewhere when it is changed. This also resulted in more general commands. For instance,
rm -rf nginx-1.*
was used to clean up, but this would fail once nginx 2.X was used, whereasrm -rf "nginx-${NGINX_VER}*"
will have the intended effect regardless of nginx version.As for using
nginx:latest
, there are several reasons not to do this. First, it is considered best practice to pin a specific version, or at least a major version. Otherwise, builds could start failing for users when a new major version is released. Second, it looks like this project is using a build option (--with-ipv6
) for nginx that is not used in the official image, and the official image includes many build options that are not needed by this project. Finally, LibreSSL has advantages over OpenSSL, and using the official image would confine the project to using OpenSSL.You are correct that pinning the version requires manual changes for updates, but this is usually preferable to using the latest tag, and it would be possible to use something like GitHub Actions to automatically make a PR when a new version of nginx is released. That way we could get the best of both worlds.