Minimization#376
Conversation
📝 WalkthroughWalkthroughAdds a new Changesnginx 1.26 Minimal Image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 YAMLlint (1.37.1)1.26-minimal/test/test-openshift.yaml[Errno 2] No such file or directory: '1.26-minimal/test/test-openshift.yaml' 🔧 Checkov (3.3.1)1.26-minimal/test/test-openshift.yaml2026-06-24 10:57:12,733 [MainThread ] [ERROR] Template file not found: 1.26-minimal/test/test-openshift.yaml ... [truncated 9105 characters] ... directory: '1.26-minimal/test/test-openshift.yaml' Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request validationFailed🔴 Approval - missing or changes were requested Success🟢 CI - All checks have passed Triggered by Workflow Run |
phracek
left a comment
phracek
left a comment
There was a problem hiding this comment.
.exclude-rhel8 is not needed at all as we would like to have only C11S and RHEL11.
The rest is fine. Thanks. Do you have any results? How the image is minimized?
|
[test] |
|
[test] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
1.26-minimal/Dockerfile.c11s (1)
22-23: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSame duplicate
io.openshift.expose-serviceslabel asDockerfile.rhel11.The second declaration overrides the first, dropping
8080:http. Merge intoio.openshift.expose-services="8080:http,8443:https".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@1.26-minimal/Dockerfile.c11s` around lines 22 - 23, The Dockerfile has a duplicated io.openshift.expose-services label declaration, which causes the earlier service mapping to be overwritten. Update the metadata in the Dockerfile.c11s label block so both ports are combined into a single io.openshift.expose-services entry, matching the intended exposure for 8080:http and 8443:https.
🧹 Nitpick comments (4)
1.26-minimal/root/opt/app-root/nginxconf-rhscl.sed (1)
1-10: 📐 Maintainability & Code Quality | 🔵 TrivialRemove the duplicate
nginxconf-rhscl.sed
1.26-minimal/Dockerfile.c11sand1.26-minimal/Dockerfile.rhel11both usenginxconf.sed;nginxconf-rhscl.sedhas no references and duplicates the same rules. Dropping it would avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@1.26-minimal/root/opt/app-root/nginxconf-rhscl.sed` around lines 1 - 10, Remove the unused duplicate nginxconf-rhscl.sed script since the Dockerfile.c11s and Dockerfile.rhel11 builds already reference nginxconf.sed and this file is not referenced anywhere else. Delete the redundant nginxconf-rhscl.sed asset to prevent config drift and keep the sed rules maintained in a single place.1.26-minimal/s2i/bin/run (1)
11-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
[[ ]]with&&over-ainside[ ].POSIX deprecates the binary
-aoperator inside[ ], and its parsing combined with the bash-only-vtest is fragile. Since this is bash, use[[ ]]:♻️ Proposed fix
-if [ ! -v NGINX_LOG_TO_VOLUME -a -v NGINX_LOG_PATH ]; then +if [[ ! -v NGINX_LOG_TO_VOLUME && -v NGINX_LOG_PATH ]]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@1.26-minimal/s2i/bin/run` around lines 11 - 14, Replace the conditional in run so it uses bash [[ ]] with && instead of [ ] with -a, since the current check mixes a deprecated operator with the bash-only -v test. Update the guard around the NGINX_LOG_TO_VOLUME and NGINX_LOG_PATH checks in the run script, keeping the symlink commands in the same block.1.26-minimal/s2i/bin/assemble (1)
17-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the
lsglob checks consistently.Lines 19, 28, and 45 run
ls -A ./nginx-cfg/*.confetc. without2>/dev/null. When the directory exists but contains no matching*.conf/*.pm, the glob stays literal andlsprints a "No such file or directory" error to stderr (thenginx-startcheck on Line 37 already handles this with2>/dev/null). Align these for clean output and consistent behavior.♻️ Proposed fix
- if [ "$(ls -A ./nginx-cfg/*.conf)" ]; then + if [ "$(ls -A ./nginx-cfg/*.conf 2>/dev/null)" ]; then(apply the same
2>/dev/nullto Lines 28 and 45)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@1.26-minimal/s2i/bin/assemble` around lines 17 - 50, The glob-based existence checks in the assemble script are inconsistent: the `ls -A` tests in the nginx config, default config, and perl module blocks can emit errors when no files match. Update the `ls` checks in the `nginx-cfg`, `nginx-default-cfg`, and `nginx-perl` sections of `assemble` to handle missing glob matches the same way as the `nginx-start` block, using stderr suppression so the script stays quiet and consistent.1.26-minimal/root/usr/share/container-scripts/nginx/common.sh (1)
20-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winQuote variable expansions to avoid word-splitting/globbing.
[ $filename ],$custom_dir/$filename, and$default_dir/$filenameare unquoted. A filename containing spaces or shell metacharacters will break the test and the-f/sourcechecks. Also note the#!/bin/shshebang on Line 1 while the file relies on bash features (function,source,<<<); it works becauserunsources it under bash, but the shebang is misleading.♻️ Proposed fix
while read filename ; do - if [ $filename ]; then + if [ -n "$filename" ]; then echo "=> sourcing $filename ..." # Custom file is prefered - if [ -f $custom_dir/$filename ]; then - source $custom_dir/$filename - elif [ -f $default_dir/$filename ]; then - source $default_dir/$filename + if [ -f "$custom_dir/$filename" ]; then + source "$custom_dir/$filename" + elif [ -f "$default_dir/$filename" ]; then + source "$default_dir/$filename" fi fi done <<<"$(get_matched_files "$custom_dir" "$default_dir" '*.sh' | sort -u)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@1.26-minimal/root/usr/share/container-scripts/nginx/common.sh` around lines 20 - 30, The shell logic in common.sh uses unquoted expansions in the read loop, so update the checks and file sourcing in the loop around read/get_matched_files to quote filename, custom_dir, and default_dir references to prevent word-splitting and globbing. Also review the shebang and bash-specific usage in this script so the header matches the fact that it relies on bash features like source and here-strings, or otherwise make the script consistently POSIX-compliant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@1.26-minimal/Dockerfile.rhel11`:
- Around line 22-23: The Dockerfile RHEL11 image metadata has a duplicate
io.openshift.expose-services label, so the later 8443:https entry overrides the
earlier 8080:http mapping. Update the LABEL block in Dockerfile.rhel11 to use a
single io.openshift.expose-services value that includes both HTTP and HTTPS
ports, preserving both mappings in the final image.
- Around line 51-52: The COPY instructions in the Dockerfile are still pulling
overlay files from the base NGINX version path instead of the minimal variant.
Update the two COPY statements in the Dockerfile build stage so they source the
local minimal overlay directories under the 1.26-minimal variant (the root
overlay and s2i/bin scripts), using the existing COPY setup around the
Dockerfile’s root and STI_SCRIPTS_PATH references.
In `@1.26-minimal/README.md`:
- Around line 1-19: Update the 1.26-minimal README so it matches the
minimal/UBI10 image variant: replace any references to the standard
ubi9/nginx-126 base image with the minimal UBI10/CentOS Stream 10 image names
used by this variant, and add a short note near the introduction explaining that
this directory/documentation is for the minimized image build. Keep the rest of
the usage guidance aligned with the minimal image naming and build/pull
instructions so readers are not pointed at the wrong base image.
- Around line 197-205: The README See also section contains template text and
outdated Dockerfile references. Update the prose in the README’s “See also”
block to refer to the nginx container image instead of “Python environment,” and
correct the variant list so it matches the Dockerfiles actually present for this
image by replacing the stale names with the current ones used in the
1.26-minimal tree, including Dockerfile.rhel11 and Dockerfile.c11s. Use the
README section itself as the target and keep the repository reference aligned
with the nginx-container source.
In `@1.26-minimal/root/README.md`:
- Line 53: The README still refers to the older ubi9/nginx-126 image in a
UBI10-based variant, so update the documented image coordinates to the correct
UBI10 equivalents throughout the affected examples. Fix the stale references in
the README sections that mention the image name and registry path, using the
existing nginx-126 and registry.access.redhat.com references as the targets to
replace.
- Around line 23-44: The minimized-variant README section is inconsistent with
the actual Dockerfile.rhel11 build, so update the description to match the real
single-stage FROM ubi10-minimal:latest setup and its microdnf-based package
installation. Align the listed packages and runtime capabilities with what
Dockerfile.rhel11 actually installs and ships, and remove or revise claims like
the scratch multi-stage build, no package manager, 72% smaller/81 MB figures,
and Perl exclusion if they are not true. Use the README section heading and the
Dockerfile.rhel11 build details as the source of truth when rewriting this
subsection.
- Around line 229-232: The “See also” text in README is stale and refers to the
wrong image type plus nonexistent Dockerfiles. Update the repository note in the
README section to match this nginx image and replace the listed Dockerfile
references with the actual variants present in this directory, using the
existing README wording style and the repository’s current Dockerfile names.
In `@1.26-minimal/test/perl-test-app`:
- Line 1: Add an entry for the gitlink `1.26-minimal/test/perl-test-app` in
`.gitmodules` so it is recognized as a submodule alongside `common`; update the
submodule configuration for `test/perl-test-app` to match the existing submodule
naming/style so `git submodule update --init` can populate it in a fresh
checkout.
In `@1.26-minimal/test/start-hook-test-app`:
- Line 1: The `1.26-minimal/test/start-hook-test-app` path is currently a
gitlink without a matching `.gitmodules` entry, so `git submodule update --init`
cannot resolve it. Update the repository metadata by adding the submodule
registration for this path in `.gitmodules` using the existing submodule setup
patterns (for example, alongside `common`), or change the fixture back to
regular tracked files if it should not be a submodule.
---
Duplicate comments:
In `@1.26-minimal/Dockerfile.c11s`:
- Around line 22-23: The Dockerfile has a duplicated
io.openshift.expose-services label declaration, which causes the earlier service
mapping to be overwritten. Update the metadata in the Dockerfile.c11s label
block so both ports are combined into a single io.openshift.expose-services
entry, matching the intended exposure for 8080:http and 8443:https.
---
Nitpick comments:
In `@1.26-minimal/root/opt/app-root/nginxconf-rhscl.sed`:
- Around line 1-10: Remove the unused duplicate nginxconf-rhscl.sed script since
the Dockerfile.c11s and Dockerfile.rhel11 builds already reference nginxconf.sed
and this file is not referenced anywhere else. Delete the redundant
nginxconf-rhscl.sed asset to prevent config drift and keep the sed rules
maintained in a single place.
In `@1.26-minimal/root/usr/share/container-scripts/nginx/common.sh`:
- Around line 20-30: The shell logic in common.sh uses unquoted expansions in
the read loop, so update the checks and file sourcing in the loop around
read/get_matched_files to quote filename, custom_dir, and default_dir references
to prevent word-splitting and globbing. Also review the shebang and
bash-specific usage in this script so the header matches the fact that it relies
on bash features like source and here-strings, or otherwise make the script
consistently POSIX-compliant.
In `@1.26-minimal/s2i/bin/assemble`:
- Around line 17-50: The glob-based existence checks in the assemble script are
inconsistent: the `ls -A` tests in the nginx config, default config, and perl
module blocks can emit errors when no files match. Update the `ls` checks in the
`nginx-cfg`, `nginx-default-cfg`, and `nginx-perl` sections of `assemble` to
handle missing glob matches the same way as the `nginx-start` block, using
stderr suppression so the script stays quiet and consistent.
In `@1.26-minimal/s2i/bin/run`:
- Around line 11-14: Replace the conditional in run so it uses bash [[ ]] with
&& instead of [ ] with -a, since the current check mixes a deprecated operator
with the bash-only -v test. Update the guard around the NGINX_LOG_TO_VOLUME and
NGINX_LOG_PATH checks in the run script, keeping the symlink commands in the
same block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d832ecda-d2a5-40b2-a462-50ab932eba6f
📒 Files selected for processing (38)
1.26-minimal/.exclude-rhel81.26-minimal/Dockerfile.c11s1.26-minimal/Dockerfile.rhel111.26-minimal/README.md1.26-minimal/root/README.md1.26-minimal/root/opt/app-root/etc/generate_container_user1.26-minimal/root/opt/app-root/etc/passwd.template1.26-minimal/root/opt/app-root/nginxconf-rhscl.sed1.26-minimal/root/opt/app-root/nginxconf.sed1.26-minimal/root/usr/share/container-scripts/nginx/common.sh1.26-minimal/s2i/bin/assemble1.26-minimal/s2i/bin/run1.26-minimal/s2i/bin/usage1.26-minimal/test/__init__.py1.26-minimal/test/conftest.py1.26-minimal/test/examples1.26-minimal/test/imagestreams1.26-minimal/test/perl-test-app1.26-minimal/test/run1.26-minimal/test/run-openshift-remote-cluster1.26-minimal/test/run-pytest1.26-minimal/test/start-hook-test-app1.26-minimal/test/test-app1.26-minimal/test/test-lib-nginx.sh1.26-minimal/test/test-lib-openshift.sh1.26-minimal/test/test-lib-remote-openshift.sh1.26-minimal/test/test-lib.sh1.26-minimal/test/test-openshift.yaml1.26-minimal/test/test_container_application.py1.26-minimal/test/test_container_basics.py1.26-minimal/test/test_container_example_apps.py1.26-minimal/test/test_ocp_imagestream_s2i.py1.26-minimal/test/test_ocp_imagestreams.py1.26-minimal/test/test_ocp_local_example.py1.26-minimal/test/test_ocp_remote_example.py1.26-minimal/test/test_ocp_shared_helm_imagestreams.py1.26-minimal/test/test_ocp_shared_helm_template.py1.26-minimal/test/test_ocp_template_example_app.py
| io.openshift.expose-services="8080:http" \ | ||
| io.openshift.expose-services="8443:https" \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Duplicate io.openshift.expose-services label drops the HTTP mapping.
LABEL keys must be unique; the second declaration overrides the first, so the final image only advertises 8443:https and loses 8080:http. Combine both ports into a single value.
🐛 Proposed fix
- io.openshift.expose-services="8080:http" \
- io.openshift.expose-services="8443:https" \
+ io.openshift.expose-services="8080:http,8443:https" \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| io.openshift.expose-services="8080:http" \ | |
| io.openshift.expose-services="8443:https" \ | |
| io.openshift.expose-services="8080:http,8443:https" \ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/Dockerfile.rhel11` around lines 22 - 23, The Dockerfile RHEL11
image metadata has a duplicate io.openshift.expose-services label, so the later
8443:https entry overrides the earlier 8080:http mapping. Update the LABEL block
in Dockerfile.rhel11 to use a single io.openshift.expose-services value that
includes both HTTP and HTTPS ports, preserving both mappings in the final image.
| COPY $NGINX_VERSION/root / | ||
| COPY $NGINX_VERSION/s2i/bin/ $STI_SCRIPTS_PATH |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t d 'root$' -p '1.26*'
fd -a 'Makefile' -d 2
rg -n 'NGINX_VERSION|VERSIONS|context|COPY .*root' Makefile 2>/dev/nullRepository: sclorg/nginx-container
Length of output: 325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== top-level files/dirs around 1.26 =="
fd -a -t d '^(1\.26|1\.26-minimal)$' .
echo
echo "== Dockerfile snippets =="
for f in 1.26-minimal/Dockerfile.rhel11 1.26/Dockerfile.rhel11; do
if [ -f "$f" ]; then
echo "-- $f --"
nl -ba "$f" | sed -n '1,140p'
fi
done
echo
echo "== make/build context references =="
rg -n '^(VERSIONS|.*context|.*docker build|.*buildx|.*dockerfile|.*1\.26|minimal)' /home/jailuser/git/Makefile 2>/dev/null || true
echo
echo "== files under version dirs matching root or s2i/bin =="
find 1.26 1.26-minimal -maxdepth 3 \( -path '*/root' -o -path '*/s2i/bin' -o -path '*/s2i/bin/*' \) -print 2>/dev/null | sortRepository: sclorg/nginx-container
Length of output: 396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect repository tree relevant to 1.26 variants =="
find . -maxdepth 2 \( -type d -o -type f \) | sed 's#^\./##' | sort | rg '^(1\.26|1\.26-minimal)(/|$)|(^|/)Makefile$|(^|/)Dockerfile\.rhel11$'Repository: sclorg/nginx-container
Length of output: 602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1.26-minimal/Dockerfile.rhel11 =="
sed -n '1,120p' 1.26-minimal/Dockerfile.rhel11
echo
echo "== 1.26-minimal/Dockerfile.c11s =="
sed -n '1,120p' 1.26-minimal/Dockerfile.c11s
echo
echo "== Makefile build rules mentioning 1.26-minimal or Dockerfile.rhel11 =="
rg -n -C 2 '1\.26-minimal|Dockerfile\.rhel11|docker build|buildx|context|root /|s2i/bin' MakefileRepository: sclorg/nginx-container
Length of output: 8429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== minimal variant references across repo =="
rg -n -C 2 '1\.26-minimal|Dockerfile\.rhel11|COPY .*root|COPY .*s2i/bin|STI_SCRIPTS_PATH|NGINX_VERSION' 1.26-minimal 1.26 MakefileRepository: sclorg/nginx-container
Length of output: 21686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1.26-minimal/Dockerfile.rhel11 =="
sed -n '1,120p' 1.26-minimal/Dockerfile.rhel11
echo
echo "== 1.26-minimal/Dockerfile.c11s =="
sed -n '1,120p' 1.26-minimal/Dockerfile.c11s
echo
echo "== Makefile snippets for 1.26-minimal build context =="
rg -n -C 3 '1\.26-minimal|Dockerfile\.rhel11|docker build|buildx|context' MakefileRepository: sclorg/nginx-container
Length of output: 8412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== line-numbered excerpt from 1.26-minimal/Dockerfile.rhel11 =="
python3 - <<'PY'
from pathlib import Path
p = Path("1.26-minimal/Dockerfile.rhel11")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 120:
print(f"{i:4d}: {line}")
PYRepository: sclorg/nginx-container
Length of output: 4828
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Makefile around version/build rules =="
python3 - <<'PY'
from pathlib import Path
p = Path("Makefile")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 220:
print(f"{i:4d}: {line}")
PYRepository: sclorg/nginx-container
Length of output: 672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build tooling references to DOCKER_BUILD_CONTEXT =="
rg -n -C 3 'DOCKER_BUILD_CONTEXT|docker build|context-dir|Dockerfile\.rhel11|1\.26-minimal' common Makefile .github 2>/dev/null || true
echo
echo "== minimal README hints =="
sed -n '1,120p' 1.26-minimal/README.mdRepository: sclorg/nginx-container
Length of output: 6157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== common/build.sh excerpt =="
if [ -f common/build.sh ]; then
python3 - <<'PY'
from pathlib import Path
p = Path("common/build.sh")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 240:
print(f"{i:4d}: {line}")
PY
fi
echo
echo "== common/common.mk excerpt =="
if [ -f common/common.mk ]; then
python3 - <<'PY'
from pathlib import Path
p = Path("common/common.mk")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 260:
print(f"{i:4d}: {line}")
PY
fiRepository: sclorg/nginx-container
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tree comparison: 1.26 vs 1.26-minimal =="
for d in root s2i/bin; do
echo "-- $d --"
echo "1.26/$d:"
find "1.26/$d" -maxdepth 3 -type f | sort | sed 's#^# #'
echo "1.26-minimal/$d:"
find "1.26-minimal/$d" -maxdepth 3 -type f | sort | sed 's#^# #'
done
echo
echo "== build entrypoints mentioning Dockerfile.rhel11 or minimal =="
rg -n -C 2 'Dockerfile\.rhel11|1\.26-minimal|minimal' . --glob '!*README.md' --glob '!**/README.md' 2>/dev/null || trueRepository: sclorg/nginx-container
Length of output: 5701
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== root Makefile around build targets =="
python3 - <<'PY'
from pathlib import Path
p = Path("Makefile")
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 1 <= i <= 220:
print(f"{i:4d}: {line}")
PY
echo
echo "== search for DOCKER_BUILD_CONTEXT usage =="
rg -n -C 3 'DOCKER_BUILD_CONTEXT|docker build|buildx|Dockerfile\.rhel11|1\.26-minimal' . 2>/dev/null | sed -n '1,220p'Repository: sclorg/nginx-container
Length of output: 1715
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== versioned directories under repo root =="
find . -maxdepth 2 -type d \( -name '1.26' -o -name '1.26-minimal' -o -name 'common' \) | sort
echo
echo "== any Makefile in subdirs =="
find . -maxdepth 2 -name 'Makefile' -o -name 'makefile' | sortRepository: sclorg/nginx-container
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Makefile (full, if small enough) =="
wc -l Makefile
sed -n '1,260p' Makefile
echo
echo "== files referencing DOCKER_BUILD_CONTEXT or versioned Dockerfiles =="
rg -n -C 2 'DOCKER_BUILD_CONTEXT|Dockerfile\.rhel11|1\.26-minimal|docker build|build:' . 2>/dev/nullRepository: sclorg/nginx-container
Length of output: 1806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files at shallow depth =="
find . -maxdepth 2 \( -type f -o -type d \) | sed 's#^\./##' | sort | sed -n '1,240p'Repository: sclorg/nginx-container
Length of output: 2345
Copy the minimal overlay locally in 1.26-minimal/Dockerfile.rhel11:51-52. 1.26-minimal/ has its own root/ and s2i/bin/, so these paths should come from the minimal variant instead of resolving through NGINX_VERSION.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/Dockerfile.rhel11` around lines 51 - 52, The COPY instructions
in the Dockerfile are still pulling overlay files from the base NGINX version
path instead of the minimal variant. Update the two COPY statements in the
Dockerfile build stage so they source the local minimal overlay directories
under the 1.26-minimal variant (the root overlay and s2i/bin scripts), using the
existing COPY setup around the Dockerfile’s root and STI_SCRIPTS_PATH
references.
| Nginx 1.26 server and a reverse proxy server container image | ||
| ============================================================ | ||
| This container image includes Nginx 1.26 server and a reverse server for OpenShift and general usage. | ||
| Users can choose between RHEL, CentOS Stream and Fedora based images. | ||
| The RHEL images are available in the [Red Hat Container Catalog](https://access.redhat.com/containers/), | ||
| the CentOS Stream images are available in the [Quay.io](https://quay.io/organization/sclorg), | ||
| and the Fedora images are available in the [Quay.io](https://quay.io/organization/fedora). | ||
| The resulting image can be run using [podman](https://github.com/containers/libpod). | ||
|
|
||
| Note: while the examples in this README are calling `podman`, you can replace any such calls by `docker` with the same arguments. | ||
|
|
||
|
|
||
| Description | ||
| ----------- | ||
|
|
||
| Nginx is a web server and a reverse proxy server for HTTP, SMTP, POP3 and IMAP | ||
| protocols, with a strong focus on high concurrency, performance and low memory usage. The container | ||
| image provides a containerized packaging of the nginx 1.26 daemon. The image can be used | ||
| as a base image for other applications based on nginx 1.26 web server. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
README does not reflect the minimal / UBI10 variant.
This README lives under 1.26-minimal/ and the Dockerfiles are based on ubi10-minimal (RHEL11) and CentOS Stream 10, yet the document references ubi9/nginx-126 (Lines 25, 86, 110, 125, 168) and never mentions that this is the minimized image variant. Readers pulling/building from these instructions will reference the wrong base image. Please align the base-image references and add a short note describing the minimal variant.
Also applies to: 25-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/README.md` around lines 1 - 19, Update the 1.26-minimal README
so it matches the minimal/UBI10 image variant: replace any references to the
standard ubi9/nginx-126 base image with the minimal UBI10/CentOS Stream 10 image
names used by this variant, and add a short note near the introduction
explaining that this directory/documentation is for the minimized image build.
Keep the rest of the usage guidance aligned with the minimal image naming and
build/pull instructions so readers are not pointed at the wrong base image.
| See also | ||
| -------- | ||
| Dockerfile and other sources for this container image are available on | ||
| https://github.com/sclorg/nginx-container. | ||
| In that repository you also can find another versions of Python environment Dockerfiles. | ||
| for RHEL8 it's `Dockerfile.rhel8`, Dockerfile for RHEL10 is called `Dockerfile.rhel10`, | ||
| Dockerfile for CentOS Stream 9 is called `Dockerfile.c9s`, | ||
| Dockerfile for CentOS Stream 10 is called `Dockerfile.c10s`, and the Fedora Dockerfile is called `Dockerfile.fedora`. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
See also section has template copy-paste errors and stale Dockerfile names.
Two problems here:
- Line 201 says "another versions of Python environment Dockerfiles" — leftover from the template; this is the nginx repository.
- Lines 202-204 enumerate
Dockerfile.rhel8,Dockerfile.rhel10,Dockerfile.c9s,Dockerfile.c10s, andDockerfile.fedora, but this1.26-minimaldirectory actually shipsDockerfile.rhel11andDockerfile.c11s. The list is inaccurate for this variant.
📝 Proposed fix
-In that repository you also can find another versions of Python environment Dockerfiles.
-for RHEL8 it's `Dockerfile.rhel8`, Dockerfile for RHEL10 is called `Dockerfile.rhel10`,
-Dockerfile for CentOS Stream 9 is called `Dockerfile.c9s`,
-Dockerfile for CentOS Stream 10 is called `Dockerfile.c10s`, and the Fedora Dockerfile is called `Dockerfile.fedora`.
+In that repository you also can find other versions of nginx Dockerfiles.
+For this minimal variant, the RHEL Dockerfile is called `Dockerfile.rhel11`
+and the CentOS Stream 10 Dockerfile is called `Dockerfile.c11s`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| See also | |
| -------- | |
| Dockerfile and other sources for this container image are available on | |
| https://github.com/sclorg/nginx-container. | |
| In that repository you also can find another versions of Python environment Dockerfiles. | |
| for RHEL8 it's `Dockerfile.rhel8`, Dockerfile for RHEL10 is called `Dockerfile.rhel10`, | |
| Dockerfile for CentOS Stream 9 is called `Dockerfile.c9s`, | |
| Dockerfile for CentOS Stream 10 is called `Dockerfile.c10s`, and the Fedora Dockerfile is called `Dockerfile.fedora`. | |
| See also | |
| -------- | |
| Dockerfile and other sources for this container image are available on | |
| https://github.com/sclorg/nginx-container. | |
| In that repository you also can find other versions of nginx Dockerfiles. | |
| For this minimal variant, the RHEL Dockerfile is called `Dockerfile.rhel11` | |
| and the CentOS Stream 10 Dockerfile is called `Dockerfile.c11s`. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/README.md` around lines 197 - 205, The README See also section
contains template text and outdated Dockerfile references. Update the prose in
the README’s “See also” block to refer to the nginx container image instead of
“Python environment,” and correct the variant list so it matches the Dockerfiles
actually present for this image by replacing the stale names with the current
ones used in the 1.26-minimal tree, including Dockerfile.rhel11 and
Dockerfile.c11s. Use the README section itself as the target and keep the
repository reference aligned with the nginx-container source.
| Minimized Variant (Dockerfile.rhel11) | ||
| -------------------------------------- | ||
|
|
||
| A **minimized variant** is available via `Dockerfile.rhel11` that provides significant size reduction: | ||
|
|
||
| **Key Features:** | ||
| - **72% smaller image size** (81 MB vs 297 MB for standard variant) | ||
| - Built using multi-stage build: `ubi10/ubi → scratch` (not s2i-core) | ||
| - No package manager in final image (dnf/yum absent for enhanced security) | ||
| - Minimal package set: coreutils-single, glibc-minimal-langpack, nginx-core | ||
| - Same S2I functionality as standard builds | ||
| - Ideal for production, edge, and IoT deployments | ||
|
|
||
| **Limitations:** | ||
| - No package manager available for runtime package installation | ||
| - Perl modules excluded (minimization priority) | ||
| - Limited extensibility compared to standard variant | ||
|
|
||
| **Build the minimized variant:** | ||
| ```bash | ||
| make build TARGET=rhel11 VERSIONS=1.26 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Minimized-variant description does not match Dockerfile.rhel11.
This section claims a multi-stage ubi10/ubi → scratch build with no package manager and packages coreutils-single, glibc-minimal-langpack, nginx-core. The actual Dockerfile.rhel11 is a single-stage FROM ubi10-minimal:latest that installs nginx nss_wrapper-libs gettext hostname findutils tar via microdnf (so microdnf is present). The "72% smaller / 81 MB vs 297 MB" and "Perl modules excluded" claims (assemble still installs nginx-perl/*.pm) also look copied from another project. Please reconcile the docs with the real build.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/root/README.md` around lines 23 - 44, The minimized-variant
README section is inconsistent with the actual Dockerfile.rhel11 build, so
update the description to match the real single-stage FROM ubi10-minimal:latest
setup and its microdnf-based package installation. Align the listed packages and
runtime capabilities with what Dockerfile.rhel11 actually installs and ships,
and remove or revise claims like the scratch multi-stage build, no package
manager, 72% smaller/81 MB figures, and Perl exclusion if they are not true. Use
the README section heading and the Dockerfile.rhel11 build details as the source
of truth when rewriting this subsection.
|
|
||
| Usage in OpenShift | ||
| ------------------ | ||
| In this example, we assume that you are using the `ubi9/nginx-126` image, available through the `nginx:1.26` imagestream tag in Openshift. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale ubi9/nginx-126 references in a UBI10-based image.
This variant is built on ubi10-minimal, but the docs repeatedly reference ubi9/nginx-126 and registry.access.redhat.com/ubi9/nginx-126. Update these to the correct UBI10 image coordinates.
Also applies to: 114-114, 138-138, 153-153, 196-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/root/README.md` at line 53, The README still refers to the older
ubi9/nginx-126 image in a UBI10-based variant, so update the documented image
coordinates to the correct UBI10 equivalents throughout the affected examples.
Fix the stale references in the README sections that mention the image name and
registry path, using the existing nginx-126 and registry.access.redhat.com
references as the targets to replace.
| In that repository you also can find another versions of Python environment Dockerfiles. | ||
| for RHEL8 it's `Dockerfile.rhel8`, Dockerfile for RHEL10 is called `Dockerfile.rhel10`, | ||
| Dockerfile for CentOS Stream 9 is called `Dockerfile.c9s`, | ||
| Dockerfile for CentOS Stream 10 is called `Dockerfile.c10s`, and the Fedora Dockerfile is called `Dockerfile.fedora`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Copy-paste leftovers in "See also".
Line 229 references "Python environment Dockerfiles" (this is an nginx image), and lines 230-232 list Dockerfile.rhel8/rhel10/c9s/c10s/fedora which don't exist in this 1.26-minimal/ directory (only Dockerfile.rhel11 and Dockerfile.c11s). Please correct.
✏️ Suggested fix
-In that repository you also can find another versions of Python environment Dockerfiles.
-for RHEL8 it's `Dockerfile.rhel8`, Dockerfile for RHEL10 is called `Dockerfile.rhel10`,
-Dockerfile for CentOS Stream 9 is called `Dockerfile.c9s`,
-Dockerfile for CentOS Stream 10 is called `Dockerfile.c10s`, and the Fedora Dockerfile is called `Dockerfile.fedora`.
+In that repository you also can find other versions of nginx Dockerfiles.
+The RHEL Dockerfile is called `Dockerfile.rhel11` and the CentOS Stream 10 Dockerfile is called `Dockerfile.c11s`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/root/README.md` around lines 229 - 232, The “See also” text in
README is stale and refers to the wrong image type plus nonexistent Dockerfiles.
Update the repository note in the README section to match this nginx image and
replace the listed Dockerfile references with the actual variants present in
this directory, using the existing README wording style and the repository’s
current Dockerfile names.
| @@ -0,0 +1 @@ | |||
| Subproject commit 5dde0bc71e6ecb91ff2ef84cc2750e22d416e894 | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the submodule path is registered and compare pinned commits across variants.
echo "=== .gitmodules entries for perl-test-app ==="
git config -f .gitmodules --get-regexp 'submodule\..*perl-test-app.*' || echo "NOT REGISTERED"
echo "=== gitlink commits per variant ==="
git ls-files -s | rg 'perl-test-app$' || echo "no gitlinks found"Repository: sclorg/nginx-container
Length of output: 339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== files named .gitmodules ==="
git ls-files .gitmodules || true
echo
echo "=== .gitmodules content (if present) ==="
if git ls-files .gitmodules | grep -q .; then
cat -n .gitmodules
else
echo "(no tracked .gitmodules file)"
fi
echo
echo "=== all perl-test-app gitlinks ==="
git ls-files -s | awk '$1 == "160000" && $4 ~ /perl-test-app$/ {print}'Repository: sclorg/nginx-container
Length of output: 483
Register 1.26-minimal/test/perl-test-app in .gitmodules. This path is a gitlink, but .gitmodules only defines common, so git submodule update --init will not populate test/perl-test-app in a fresh checkout and the example-app Perl tests will break.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/test/perl-test-app` at line 1, Add an entry for the gitlink
`1.26-minimal/test/perl-test-app` in `.gitmodules` so it is recognized as a
submodule alongside `common`; update the submodule configuration for
`test/perl-test-app` to match the existing submodule naming/style so `git
submodule update --init` can populate it in a fresh checkout.
| @@ -0,0 +1 @@ | |||
| Subproject commit 3dce4cb0937c5dd1e64a3494894a6cd4ca90a376 | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify .gitmodules registration for the new submodule path
cat .gitmodules 2>/dev/null | grep -nA3 'start-hook-test-app'
# Show all gitlink pointers for start-hook-test-app across variant dirs to confirm SHA consistency
fd -t f -H 'start-hook-test-app' --exec sh -c 'echo "== $1 =="; cat "$1"' _ {}
# Check ls-tree gitlink mode (160000) for the changed path
git ls-tree HEAD 1.26-minimal/test/ | grep 'start-hook-test-app'Repository: sclorg/nginx-container
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .gitmodules entries =="
if [ -f .gitmodules ]; then
cat -n .gitmodules | sed -n '1,220p'
else
echo "no .gitmodules file"
fi
echo
echo "== path registration search =="
rg -n "start-hook-test-app|1\.26-minimal/test/start-hook-test-app" .gitmodules .gitmodules 2>/dev/null || true
echo
echo "== tree entries around the path =="
git ls-tree HEAD 1.26-minimal/test | sed -n '1,120p'
echo
echo "== sibling variant pointers =="
git ls-tree HEAD test 2>/dev/null | sed -n '1,120p' || true
git ls-tree HEAD 1.25-minimal/test 2>/dev/null | sed -n '1,120p' || true
git ls-tree HEAD 1.26-full/test 2>/dev/null | sed -n '1,120p' || trueRepository: sclorg/nginx-container
Length of output: 534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find any .gitmodules-like files =="
git ls-files | rg '(^|/)\.gitmodules$|start-hook-test-app' || true
echo
echo "== inspect file type at path if present in index =="
git ls-tree -r HEAD | rg 'start-hook-test-app|\.gitmodules' || trueRepository: sclorg/nginx-container
Length of output: 2795
Add the missing submodule registration for 1.26-minimal/test/start-hook-test-app
1.26-minimal/test/start-hook-test-app is a gitlink, but .gitmodules only defines common. Add the submodule entry for this path, or convert it back to regular files, so git submodule update --init can resolve the fixture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@1.26-minimal/test/start-hook-test-app` at line 1, The
`1.26-minimal/test/start-hook-test-app` path is currently a gitlink without a
matching `.gitmodules` entry, so `git submodule update --init` cannot resolve
it. Update the repository metadata by adding the submodule registration for this
path in `.gitmodules` using the existing submodule setup patterns (for example,
alongside `common`), or change the fixture back to regular tracked files if it
should not be a submodule.
2 participants
This is an updated minimization PR that follows postgresql-container minimal pattern.
Summary by CodeRabbit
New Features
Documentation