-
Notifications
You must be signed in to change notification settings - Fork 154
feat: update nginx plus image builds to use nginx plus base images #449
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
base: main
Are you sure you want to change the base?
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.
Pull Request Overview
This PR migrates NGINX Plus container builds from using a public Debian base image to the official NGINX Plus Docker base image, improving compliance and support. It also removes legacy entrypoint scripts that are now redundant with the official base image.
- Updates both Dockerfiles to use the official NGINX Plus base image from the private registry
- Removes legacy entrypoint scripts (
10-listen-on-ipv6-by-default.sh
and20-envsubst-on-templates.sh
) - Updates environment variables and module installation logic to use explicit Plus and OSS versions
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
Dockerfile.plus | Updates base image to official NGINX Plus and removes redundant user creation and container configuration |
Dockerfile.buildkit.plus | Same base image and configuration updates as standard Dockerfile |
plus/docker-entrypoint.d/10-listen-on-ipv6-by-default.sh | Removes legacy IPv6 configuration script |
plus/docker-entrypoint.d/20-envsubst-on-templates.sh | Removes legacy template substitution script |
docs/getting_started.md | Updates build instructions to clarify NGINX Plus image repository setup requirements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Changes look good to me - the copilot suggestion around chmod commands on directories that are not explicitly copied in any long are worth looking at but perhaps we are not just dealing with those directories as they are inherited from the new base image?
3fb3c55
to
62504e6
Compare
nginx plus has entrypoint scripts copied from docker-nginx of OSS images. Are we sure we want to do any changes to entrypoint.sh? |
e928452
to
8c80096
Compare
@oxpa I've incorporated your suggestions and redone this PR. Thank you for them. It helped me clarify what was needed. |
@@ -1,9 +1,5 @@ | |||
FROM nginx:1.29.0@sha256:f5c017fb33c6db484545793ffb67db51cdd7daebee472104612f73a85063f889 | |||
|
|||
# NJS env vars |
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.
We no longer hard-code NJS versions.
COPY common/etc /etc | ||
COPY common/docker-entrypoint.sh /docker-entrypoint.sh |
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.
We no longer need a custom entry point script.
COPY common/docker-entrypoint.d /docker-entrypoint.d/ | ||
|
||
RUN set -x \ | ||
&& mkdir -p /var/cache/nginx/s3_proxy \ | ||
&& chown nginx:nginx /var/cache/nginx/s3_proxy \ | ||
&& chmod -R -v +x /docker-entrypoint.sh /docker-entrypoint.d/*.sh; | ||
&& find /docker-entrypoint.d -type f \( -name '*.sh' -or -name '*.envsh' \) -exec chmod -v +x {} \; |
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.
This find command ensures all the files in the entry point directory are executable.
Dockerfile.plus
Outdated
|
||
ENTRYPOINT ["/docker-entrypoint.sh"] | ||
RUN set -eux \ |
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.
We now append the commented out default E+V blocks into the nginx.conf, so that the entry point substitution script can replace values from it.
Dockerfile.plus
Outdated
# Copy files from the OSS NGINX Docker container such that the container | ||
# startup is the same. | ||
COPY --from=build /etc/nginx/modules/ngx_http_xslt_filter_module*.so /etc/nginx/modules/ | ||
COPY --from=build /etc/nginx/modules/ngx_http_js_module*.s /etc/nginx/modules/ | ||
COPY --from=build /lib/aarch64-linux-gnu/libxslt.so.1 /lib/aarch64-linux-gnu/ |
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.
It looks like I hard coded architecture here, so this will need to be fixed.
This change does the following: * Migrates to using the official NGINX Plus Docker images as base images * Removes the distinction between BuildKit and non-BuildKit builds for Plus images (OSS never had this) * Adds support for license validation for Plus images * Introduces a multi-stage build for Plus images Signed-off-by: Elijah Zupancic <[email protected]>
Signed-off-by: Elijah Zupancic <[email protected]>
By using the version reported by NGINX rather than the environment variable it allows for a more reliable setting and less complexity. Signed-off-by: Elijah Zupancic <[email protected]>
8c80096
to
ce18e42
Compare
This change migrates to using the official NGINX Plus Docker images as base images.
Proposed changes
This pull request refactors the Dockerfiles and entrypoint scripts for both the OSS and Plus NGINX S3 Gateway images. The main goals are to modernize the build process, improve maintainability, and enhance compatibility with best practices for multi-stage Docker builds. The changes include switching to official NGINX Plus base images, restructuring build steps, simplifying environment variable handling, and improving script robustness.
Dockerfile and Build Process Refactoring:
Dockerfile.oss
) and Plus (Dockerfile.plus
) images to use official NGINX and NGINX Plus base images, removing custom build logic and simplifying module installation. The Plus image now uses a multi-stage build to avoid embedding sensitive license files in the final image. [1] [2] [3]Dockerfile.buildkit.plus
as its logic is replaced by the new multi-stage approach.Entrypoint Script Improvements:
docker-entrypoint.sh
Other Notable Changes:
These changes collectively modernize the build and runtime environment for the NGINX S3 Gateway images, making them easier to maintain and more secure.
Base Image and Versioning Updates:
Dockerfile.plus
now uses the official NGINX Plus base image from the private registry instead of the public Debian image, ensuring compliance and improved support. [1] [2]Entrypoint and Startup Script Cleanup:
10-listen-on-ipv6-by-default.sh
and20-envsubst-on-templates.sh
have been removed fromplus/docker-entrypoint.d
, simplifying container startup and reducing maintenance overhead. [1] [2]Documentation Improvements:
docs/getting_started.md
have been updated to clarify the process for building the NGINX Plus image, including the need to set up access to the official Plus Docker image repository and the steps for handling repository keys.Checklist
Before creating a pull request (PR), run through this checklist and mark each as complete:
README.md
).