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.

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

fix: update NGINX Plus Dockerfile #274

Merged
merged 4 commits into from
Jul 10, 2024
Merged

fix: update NGINX Plus Dockerfile #274

merged 4 commits into from
Jul 10, 2024

Conversation

alessfg
Copy link
Collaborator

@alessfg alessfg commented Jun 26, 2024

Proposed changes

This PR refactors the NGINX Plus Dockerfile by using the NGINX Plus Dockerfile found here gist.github.com/nginx-gists/36e97fc87efb5cf0039978c8e41a34b5 as a starting point.

This PR also refactors the various ENV instructions in both the NGINX OSS and NGINX Plus Dockerfiles to use the new guidelines where the key=value pair is separated by "=" instead of a whitespace.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

@alessfg alessfg requested a review from 4141done June 26, 2024 16:58
@alessfg alessfg self-assigned this Jun 26, 2024
@alessfg alessfg added bug Something isn't working enhancement New feature or request labels Jun 26, 2024
Copy link
Collaborator

@4141done 4141done left a comment

Choose a reason for hiding this comment

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

This all makes sense. One question and one small suggestion. I'm going to run some tests with this then approve.


ENTRYPOINT ["/docker-entrypoint.sh"]
# Download your NGINX license certificate and key from the F5 customer portal (https://account.f5.com) and copy it to the build context
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't be sure what where I need to put my files based on this comment. I know we supply build context as the last argument to docker build so does that mean that if I supply . then this file expects my license files to be in the current dir?

Copy link
Collaborator Author

@alessfg alessfg Jun 26, 2024

Choose a reason for hiding this comment

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

You would pass the location via the src parameter in --secret id=nginx-crt,src=plus/etc/ssl/nginx/nginx-repo.crt.

gpg1 --export "$NGINX_GPGKEY" > "$NGINX_GPGKEY_PATH" ; \
rm -rf "$GNUPGHOME"; \
apt-get remove --purge --auto-remove -y gnupg1 && rm -rf /var/lib/apt/lists/*; \
echo "deb [signed-by=/etc/apt/keyrings/nginx-archive-keyring.gpg] https://nginx.org/packages/mainline/debian/ $(echo $PKG_RELEASE | cut -f2 -d~) nginx" >> /etc/apt/sources.list.d/nginx.list; \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason we're removing this? Did the official nginx image GPG keys get updated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They did! All three keys (including the new two ones) are now part of the official image and the two new keys do not expire (in theory).

Copy link
Collaborator

@4141done 4141done left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! Manually tested! Ship it!

@alessfg alessfg merged commit 1e6eca5 into main Jul 10, 2024
9 checks passed
@alessfg alessfg deleted the dockerfile-update-simple branch July 10, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants