Skip to content
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

Jump to bottom

Update nginx to latest stable #725

Merged
merged 8 commits into from
Feb 18, 2020
Merged

Update nginx to latest stable #725

merged 8 commits into from
Feb 18, 2020

Conversation

jschaul
Copy link
Member

@jschaul jschaul commented Apr 23, 2019

No description provided.

tiago-loureiro
tiago-loureiro approved these changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@tiago-loureiro tiago-loureiro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Assuming all our config options still work 👌

(As in, hope they work, otherwise we should just fix them not avoid the upgrade)

@jschaul
Copy link
Member Author

jschaul commented Apr 24, 2019

Actually, perhaps we should follow the upstream changes from these commits for the docker build:

@fisx fisx mentioned this pull request May 3, 2019
Merged
@jschaul jschaul changed the title update nginx to latest stable [WIP] update nginx to latest stable May 6, 2019
@jschaul jschaul changed the title [WIP] update nginx to latest stable Update nginx to latest stable Feb 18, 2020
@jschaul
Copy link
Member Author

jschaul commented Feb 18, 2020

Updated this PR, can be re-reviewed.

I tried to incorporate the changes from the upstream docker image building; however that does not work to compile the nginx-zauth-module and nginx-module-vts modules that are needed. Even if in theory they can be dynamically loaded (which also requires us to change config files), they need to still be compiled and available on the filesystem as .so files. The vts module appears unmaintained and doesn't provide details on usage with nginx 1.16 and dynamic loading.
Switching to dynamic modules is a harder task than I anticipated, and should be done on a separate PR.

arianvp
arianvp reviewed Feb 18, 2020
View reviewed changes
services/nginz/Dockerfile Outdated
@@ -8,7 +8,7 @@ RUN cd /src/libzauth/libzauth-c \
&& make install

# Nginz container
FROM alpine:3.11
FROM alpine:3.10
Copy link
Contributor

@arianvp arianvp Feb 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

What does "not supported" mean? Perhaps you could leave a comment for the next person who feels like bumping this number? What went wrong?

arianvp
arianvp reviewed Feb 18, 2020
View reviewed changes
services/nginz/Dockerfile
@@ -1,5 +1,5 @@
# Requires docker >= 17.05 (requires support for multi-stage builds)
FROM alpine:3.10 as libzauth-builder
FROM alpine:3.11 as libzauth-builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind the change got reverted here again. I was reading this from commit to commit. Ignore previous comment :)

@jschaul jschaul merged commit 153fc94 into develop Feb 18, 2020
@jschaul jschaul deleted the feature/update-nginx branch February 18, 2020 18:44
@fisx fisx mentioned this pull request Feb 18, 2020
Merged
@fisx fisx mentioned this pull request Mar 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arianvp arianvp arianvp approved these changes

@tiago-loureiro tiago-loureiro Awaiting requested review from tiago-loureiro

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jschaul @arianvp @tiago-loureiro