Skip to content

Conversation

@shaun-nx
Copy link
Contributor

@shaun-nx shaun-nx commented Sep 22, 2025

Proposed changes

This change adds three new Dockerfiles that can be used to build NGF, NGINX Open Source, and NGINX Plus with ub9-minimal as the base image.

Docker files are now organized into folders by the base image they use (e.g. alpine, ubi, etc...)

This allows us to using the BUILD_OS arg in our Makefile to build imaged from different base images.

To build both NGf and NGINX Opnesource using the UBI based Dockerfiles:

make build-images BUILD_OS=ubi

BUILD_OS defaults to alpine

Closes #3906

Checklist

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

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Add Dockerfiles to build images based on RedHat's UBI 9 Minimal image

@codecov
Copy link

codecov bot commented Sep 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat/openshift-support@0a624fd). Learn more about missing BASE report.

Additional details and impacted files
@@                    Coverage Diff                    @@
##             feat/openshift-support    #3941   +/-   ##
=========================================================
  Coverage                          ?   86.81%           
=========================================================
  Files                             ?      128           
  Lines                             ?    16602           
  Branches                          ?       62           
=========================================================
  Hits                              ?    14413           
  Misses                            ?     2007           
  Partials                          ?      182           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shaun-nx
Copy link
Contributor Author

shaun-nx commented Sep 24, 2025

@ciarams87 @sjberman

In the most recent commit, /tmp was added as an emptyDir to our deployments.
There was a number of permission errors when trying to write to /tmp.

The build also uses chmod 1777 on /tmp as ps command expects /tmp to be world-writable (1777), not just writable by the current user.

Without that change, we saw this error occur when running ps -ef or any other process that needed to write to /tmp

assertion failed [!result.is_error]: Failed to create temporary file
(ThreadContextFcntl.cpp:85 create_tempfile)
 command terminated with exit code 133

Also, I understand it's probably not desirable from a security standpoint to have any dir with full 777 permissions.
What from I found (with the help of Cline), the sticky bit (t in drwxrwxrwt) ensures that only the owner of a file in /tmp can delete it, even though the directory is world-writable. This is the standard and secure way to allow multiple processes to use /tmp safely.

@sjberman
Copy link
Collaborator

sjberman commented Sep 24, 2025

@shaun-nx What else is writing to /tmp? If you updated the entrypoint to the new method from agent, you aren't running ps -ef anymore, right? I definitely don't want an extra volume mount for this, especially with those permissions if it is avoidable and we can adjust the startup script.

@shaun-nx shaun-nx changed the base branch from main to feat/openshift-support September 24, 2025 14:16
@shaun-nx
Copy link
Contributor Author

@shaun-nx What else is writing to /tmp? If you updated the entrypoint to the new method from agent, you aren't running ps -ef anymore, right? I definitely don't want an extra volume mount for this, especially with those permissions if it is avoidable and we can adjust the startup script.

That's a good point. Agent itself was throwing the same error though, not just ps -ef. I'll test that out again though and see what happens

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Sep 24, 2025
@shaun-nx shaun-nx marked this pull request as ready for review September 25, 2025 08:47
@shaun-nx shaun-nx requested a review from a team as a code owner September 25, 2025 08:47
@shaun-nx shaun-nx requested a review from ciarams87 September 25, 2025 13:29
@shaun-nx shaun-nx requested a review from sjberman September 26, 2025 10:06
@shaun-nx shaun-nx requested a review from ciarams87 September 26, 2025 16:02
@shaun-nx shaun-nx requested a review from salonichf5 September 29, 2025 09:30
@shaun-nx shaun-nx merged commit b0cf702 into feat/openshift-support Sep 29, 2025
38 checks passed
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Sep 29, 2025
@shaun-nx shaun-nx deleted the feat/ubi-base-image branch September 29, 2025 15:52
@shaun-nx shaun-nx linked an issue Oct 20, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request release-notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add UBI base image support to Dockerfiles

6 participants