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

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 and others added 2 commits September 26, 2025 16:48
Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>
@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
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.

5 participants