docker: add ubi images for redhat certification#5993
docker: add ubi images for redhat certification#5993josedev-union wants to merge 4 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: josedev-union <josebarato321@gmail.com>
josedev-union
requested review from
Divyaasm,
gaiksaya,
peterzhuamazon,
prudhvigodithi,
rishabh6788,
tianleh and
zelinh
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds two new Red Hat UBI 10-based Dockerfiles for OpenSearch and OpenSearch Dashboards. Both use multi-stage builds with explicit OpenShift UID/GID compatibility, dynamic version handling with fallbacks, group-writable permissions, and entrypoint scripts configured for arbitrary UID execution without setting a USER directive. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
docker/release/dockerfiles/opensearch.ubi8.dockerfile (4)
47-47:[[ ]]is bash-specific — verify/bin/shis bash in UBI 10 minimal.Docker
RUNexecutes commands via/bin/sh -c. On RHEL-based images/bin/shis typically bash, so[[ -d ... ]]works in practice. However, if UBI 10 minimal ever switches to a POSIX-only shell, this will break. Using[ -d ... ]is a safer, portable alternative.Portable alternative
- if [[ -d $SECURITY_PLUGIN_DIR ]] ; then chmod -v 750 $SECURITY_PLUGIN_DIR/tools/* && chgrp -R 0 $SECURITY_PLUGIN_DIR/tools/* && chmod -R g+rwX $SECURITY_PLUGIN_DIR/tools/* ; fi && \ - if [[ -d $PERFORMANCE_ANALYZER_PLUGIN_CONFIG_DIR ]] ; then cp -v $TEMP_DIR/performance-analyzer.properties $PERFORMANCE_ANALYZER_PLUGIN_CONFIG_DIR; fi && \ + if [ -d "$SECURITY_PLUGIN_DIR" ] ; then chmod -v 750 $SECURITY_PLUGIN_DIR/tools/* && chgrp -R 0 $SECURITY_PLUGIN_DIR/tools/* && chmod -R g+rwX $SECURITY_PLUGIN_DIR/tools/* ; fi && \ + if [ -d "$PERFORMANCE_ANALYZER_PLUGIN_CONFIG_DIR" ] ; then cp -v $TEMP_DIR/performance-analyzer.properties $PERFORMANCE_ANALYZER_PLUGIN_CONFIG_DIR; fi && \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile` at line 47, The conditional uses bash-specific [[ ]] which may break under /bin/sh; replace the test in the RUN line that references SECURITY_PLUGIN_DIR to use the POSIX test [ -d "$SECURITY_PLUGIN_DIR" ] (note the single brackets and quoted variable) while keeping the existing chmod/chgrp/chmod -R g+rwX sequence and overall command chaining intact so behavior and exit codes remain the same.
78-84: Redundant permission operations —findcommands duplicatechmod -R g+rwX.
chmod -R g+rwX(line 82) already grantsg+xon directories andg+ron files (the uppercaseXsets execute only on directories and files already executable). Lines 83–84 (find … -type d -exec chmod g+xandfind … -type f -exec chmod g+r) are therefore no-ops and add two full filesystem traversals to the build.Simplification
RUN chgrp -R 0 $OPENSEARCH_HOME && \ - chmod -R g+rwX $OPENSEARCH_HOME && \ - find $OPENSEARCH_HOME -type d -exec chmod g+x {} + && \ - find $OPENSEARCH_HOME -type f -exec chmod g+r {} + + chmod -R g+rwX $OPENSEARCH_HOME🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile` around lines 78 - 84, The Dockerfile RUN block that sets permissions for $OPENSEARCH_HOME is performing redundant filesystem traversals: the existing chmod -R g+rwX $OPENSEARCH_HOME already grants group read on files and group execute on directories via the capital X, so the subsequent find $OPENSEARCH_HOME -type d -exec chmod g+x {} + and find $OPENSEARCH_HOME -type f -exec chmod g+r {} + are unnecessary; remove those two find ... -exec lines and keep the chgrp -R 0 $OPENSEARCH_HOME && chmod -R g+rwX $OPENSEARCH_HOME sequence in the RUN command to achieve the same effect with a single traversal.
97-105: Permissions are re-applied three times in Stage 1 — consolidate into a single pass.Ownership/permissions are set on lines 81–84 after
COPY --chown, then again on lines 104–105 after the one-time setup, and the entrypoint getschmod g+xa third time on line 130. Eachchgrp -R 0+chmod -R g+rwXis a full recursive traversal. Consider consolidating: run the one-time setup first, then do a single permission-fixing pass afterward (and drop line 130 sinceg+rwXalready covers it).Consolidated approach
COPY --from=linux_stage_0 --chown=root:0 $OPENSEARCH_HOME $OPENSEARCH_HOME WORKDIR $OPENSEARCH_HOME -# Set group-writable permissions for OpenShift compatibility -RUN chgrp -R 0 $OPENSEARCH_HOME && \ - chmod -R g+rwX $OPENSEARCH_HOME && \ - find $OPENSEARCH_HOME -type d -exec chmod g+x {} + && \ - find $OPENSEARCH_HOME -type f -exec chmod g+r {} + - # Set $JAVA_HOME -RUN echo "export JAVA_HOME=$OPENSEARCH_HOME/jdk" >> /etc/profile.d/java_home.sh && \ - echo "export PATH=\$PATH:\$JAVA_HOME/bin" >> /etc/profile.d/java_home.sh && \ - ls -l $OPENSEARCH_HOME +RUN ls -l $OPENSEARCH_HOME ENV JAVA_HOME=$OPENSEARCH_HOME/jdk ENV PATH=$PATH:$JAVA_HOME/bin:$OPENSEARCH_HOME/bin @@ ... ARG DISABLE_INSTALL_DEMO_CONFIG=true ARG DISABLE_SECURITY_PLUGIN=false RUN ./opensearch-onetime-setup.sh && \ chgrp -R 0 $OPENSEARCH_HOME && \ chmod -R g+rwX $OPENSEARCH_HOME -# Ensure the entrypoint script is executable by any UID -RUN chmod g+x $OPENSEARCH_HOME/opensearch-docker-entrypoint.sh - ENTRYPOINT ["./opensearch-docker-entrypoint.sh"]Also applies to: 129-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile` around lines 97 - 105, The Dockerfile currently repeats recursive permission fixes (chgrp -R 0 $OPENSEARCH_HOME and chmod -R g+rwX $OPENSEARCH_HOME) multiple times around the COPY --chown step, the RUN ./opensearch-onetime-setup.sh step, and again in the entrypoint chmod g+x; consolidate by running the one-time setup (opensearch-onetime-setup.sh) before any final ownership/permission adjustments, then perform a single chgrp -R 0 $OPENSEARCH_HOME && chmod -R g+rwX $OPENSEARCH_HOME pass and remove the earlier and later redundant permission commands (including the entrypoint chmod g+x) so only one recursive traversal occurs.
86-89:/etc/profile.d/java_home.shis effectively dead code in a container.
/etc/profile.dscripts are only sourced by login shells. DockerENTRYPOINT/CMDinvocations don't use a login shell, so these exports are never applied. TheENVdirectives on lines 91–92 are what actually setJAVA_HOMEandPATHfor the container process — theprofile.dsnippet can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile` around lines 86 - 89, Remove the RUN block that writes to /etc/profile.d/java_home.sh and the ls -l $OPENSEARCH_HOME step because profile.d scripts aren't sourced for container processes; instead rely on the existing ENV directives that set JAVA_HOME and PATH (the ENV for JAVA_HOME and PATH present later in this Dockerfile) to expose the environment to the container runtime.docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile (3)
82-88: Same redundantfindcommands as the OpenSearch Dockerfile.
chmod -R g+rwXon line 86 already covers what lines 87–88 do. See the comment on the OpenSearch Dockerfile for details.Simplification
RUN chgrp -R 0 $OPENSEARCH_DASHBOARDS_HOME && \ - chmod -R g+rwX $OPENSEARCH_DASHBOARDS_HOME && \ - find $OPENSEARCH_DASHBOARDS_HOME -type d -exec chmod g+x {} + && \ - find $OPENSEARCH_DASHBOARDS_HOME -type f -exec chmod g+r {} + + chmod -R g+rwX $OPENSEARCH_DASHBOARDS_HOME🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile` around lines 82 - 88, The RUN block setting group-writable permissions is redundant: keep the chgrp -R 0 $OPENSEARCH_DASHBOARDS_HOME && chmod -R g+rwX $OPENSEARCH_DASHBOARDS_HOME and remove the extra find commands that reapply directory execute and file read bits (the two find ... -exec chmod ... lines), since chmod -R g+rwX on $OPENSEARCH_DASHBOARDS_HOME already grants group execute on directories and group read on files; update the RUN invocation to only use chgrp -R 0 and chmod -R g+rwX on $OPENSEARCH_DASHBOARDS_HOME.
115-116: Redundantchmod g+x— already covered bychmod -R g+rwXon line 86.This can be dropped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile` around lines 115 - 116, Remove the redundant RUN chmod g+x $OPENSEARCH_DASHBOARDS_HOME/opensearch-dashboards-docker-entrypoint.sh line; the entrypoint already gets execute permissions from the earlier chmod -R g+rwX call, so delete the explicit chmod invocation referencing OPENSEARCH_DASHBOARDS_HOME/opensearch-dashboards-docker-entrypoint.sh to avoid duplication.
58-72: Consider usingmicrodnfconsistently instead of installingdnfas a runtime dependency.Stage 1 installs
dnfviamicrodnf(line 62), then usesdnfon line 70 for font packages.microdnfcan install those same packages directly, avoiding the overhead of pulling in the fulldnfdependency tree. This applies to the OpenSearch Dockerfile as well.Additionally, silencing font package errors with
2>/dev/null || true(line 71) is pragmatic but could mask real failures (e.g., repo connectivity issues). Consider logging a warning instead of fully suppressing stderr.Proposed simplification
RUN microdnf update -y && \ - microdnf install -y tar gzip which dnf && \ + microdnf install -y tar gzip which && \ microdnf clean all -RUN dnf install -y nss fontconfig freetype && \ - (dnf install -y xorg-x11-fonts-Type1 xorg-x11-fonts-misc 2>/dev/null || true) && \ - dnf clean all +RUN microdnf install -y nss fontconfig freetype && \ + (microdnf install -y xorg-x11-fonts-Type1 xorg-x11-fonts-misc 2>/dev/null || echo "WARNING: Some optional font packages not available") && \ + microdnf clean all🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile` around lines 58 - 72, Replace the runtime use of dnf with microdnf and stop installing dnf as a dependency: change the RUN that installs font packages to use microdnf (install nss fontconfig freetype and xorg-x11-fonts-Type1 xorg-x11-fonts-misc via microdnf) instead of calling dnf; remove the separate dnf installation. For the fallback install of xorg fonts, avoid silencing stderr with "2>/dev/null || true" — instead check the command exit status and emit a clear warning (e.g., via echo to stderr) if microdnf fails so repository/connectivity errors aren’t masked; keep the final microdnf clean all call to reduce image size.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfiledocker/release/dockerfiles/opensearch.ubi8.dockerfile
🧰 Additional context used
🪛 Trivy (0.69.1)
docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
docker/release/dockerfiles/opensearch.ubi8.dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🔇 Additional comments (3)
docker/release/dockerfiles/opensearch.ubi8.dockerfile (1)
107-108: Trivy DS-0002 (noUSERdirective) is an intentional design choice here — acknowledged.The static analysis flag is a false positive in this context. Omitting
USERis the standard pattern for OpenShift arbitrary-UID images where the runtime UID is injected by the platform. The comments clearly document the rationale.docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile (2)
103-113: LGTM — labels and metadata look correct.The label schema follows the same conventions as the OpenSearch Dockerfile.
VERSION,BUILD_DATE, andNOTESare properly parameterized via ARGs.
118-120: LGTM — entrypoint and CMD follow established conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile`:
- Line 15: The Dockerfile opensearch-dashboards.ubi8.dockerfile incorrectly uses
the UBI10 base image string "registry.access.redhat.com/ubi10-minimal:10.1"
while the filename/label still say "ubi8"; rename the file to use "ubi10" in its
name (e.g., opensearch-dashboards.ubi10.dockerfile) and update the Dockerfile
LABEL that records the UBI version to reflect ubi10 (also update the other
identical occurrence noted in the file) so the filename, base image and labels
consistently indicate UBI10.
- Around line 36-43: The current logic computing MAJOR_VERSION_ENTRYPOINT and
MAJOR_VERSION_YML uses grep "$MAJOR..." which matches empty VERSION and prevents
the fallback to "default"; update the two conditional lines that set
MAJOR_VERSION_ENTRYPOINT and MAJOR_VERSION_YML so they first ensure the major
variables are non-empty before grepping (for example: change the if to check [
-n "$MAJOR_VERSION_ENTRYPOINT" ] && ls $TEMP_DIR | grep -E
"opensearch-dashboards-docker-entrypoint-.*.x.sh" | grep -q
"$MAJOR_VERSION_ENTRYPOINT" && ...; otherwise set
MAJOR_VERSION_ENTRYPOINT="default", and do the same pattern for
MAJOR_VERSION_YML), keeping the subsequent cp commands that use
MAJOR_VERSION_ENTRYPOINT and MAJOR_VERSION_YML unchanged.
- Line 44: Remove the unconditional copy of example certificates so the
production image does not ship self‑signed certs: delete or stop using the cp of
"$TEMP_DIR/opensearch.example.org.*" into "$OPENSEARCH_DASHBOARDS_HOME/config/"
(the glob "opensearch.example.org.*") and instead require certificates be
provided at runtime via a mounted volume or loader in the entrypoint (or gate
the copy behind an explicit opt‑in env var like INCLUDE_EXAMPLE_CERTS). Update
Dockerfile logic around TEMP_DIR and OPENSEARCH_DASHBOARDS_HOME/config to no
longer add the example cert files and add documentation/entrypoint checks
explaining how to mount/provide real certs.
In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile`:
- Around line 40-56: The build silently misroutes when VERSION is empty because
MAJOR_VERSION_ENTRYPOINT=`echo $VERSION | cut -d. -f1` yields empty and the grep
check matches everything; fix by explicitly validating
VERSION/MAJOR_VERSION_ENTRYPOINT before using it: after computing
MAJOR_VERSION_ENTRYPOINT (from VERSION), check if it is non-empty (e.g., test -n
"$MAJOR_VERSION_ENTRYPOINT") and only run the grep-based selection when
non-empty, otherwise set MAJOR_VERSION_ENTRYPOINT="default"; ensure the
subsequent cp of opensearch-docker-entrypoint-$MAJOR_VERSION_ENTRYPOINT.x.sh
uses the validated MAJOR_VERSION_ENTRYPOINT so a missing or empty VERSION falls
back to "default" instead of attempting to copy
opensearch-docker-entrypoint-.x.sh.
- Line 16: The filename and DOCKERFILE label are inconsistent with the base
image: rename the file from opensearch.ubi8.dockerfile to
opensearch.ubi10.dockerfile and update the DOCKERFILE label (the label value
currently on line ~127) to "opensearch.ubi10.dockerfile" so it matches the FROM
statement "registry.access.redhat.com/ubi10-minimal:10.1"; ensure any references
or comments inside the Dockerfile that mention "ubi8" are changed to "ubi10" as
well.
- Around line 65-70: Remove the unnecessary runtime packages from Stage 1: edit
the RUN line that currently does "microdnf install -y tar gzip which dnf" and
remove tar, gzip, which, and dnf so the stage only performs the microdnf
update/clean (or installs only truly required runtime packages); also update the
comment above the RUN to reflect that the builder stage handles tarball
extraction and that securityadmin.sh/install_demo_configuration.sh do not
require these tools so they are omitted to reduce image size.
---
Nitpick comments:
In `@docker/release/dockerfiles/opensearch-dashboards.ubi8.dockerfile`:
- Around line 82-88: The RUN block setting group-writable permissions is
redundant: keep the chgrp -R 0 $OPENSEARCH_DASHBOARDS_HOME && chmod -R g+rwX
$OPENSEARCH_DASHBOARDS_HOME and remove the extra find commands that reapply
directory execute and file read bits (the two find ... -exec chmod ... lines),
since chmod -R g+rwX on $OPENSEARCH_DASHBOARDS_HOME already grants group execute
on directories and group read on files; update the RUN invocation to only use
chgrp -R 0 and chmod -R g+rwX on $OPENSEARCH_DASHBOARDS_HOME.
- Around line 115-116: Remove the redundant RUN chmod g+x
$OPENSEARCH_DASHBOARDS_HOME/opensearch-dashboards-docker-entrypoint.sh line; the
entrypoint already gets execute permissions from the earlier chmod -R g+rwX
call, so delete the explicit chmod invocation referencing
OPENSEARCH_DASHBOARDS_HOME/opensearch-dashboards-docker-entrypoint.sh to avoid
duplication.
- Around line 58-72: Replace the runtime use of dnf with microdnf and stop
installing dnf as a dependency: change the RUN that installs font packages to
use microdnf (install nss fontconfig freetype and xorg-x11-fonts-Type1
xorg-x11-fonts-misc via microdnf) instead of calling dnf; remove the separate
dnf installation. For the fallback install of xorg fonts, avoid silencing stderr
with "2>/dev/null || true" — instead check the command exit status and emit a
clear warning (e.g., via echo to stderr) if microdnf fails so
repository/connectivity errors aren’t masked; keep the final microdnf clean all
call to reduce image size.
In `@docker/release/dockerfiles/opensearch.ubi8.dockerfile`:
- Line 47: The conditional uses bash-specific [[ ]] which may break under
/bin/sh; replace the test in the RUN line that references SECURITY_PLUGIN_DIR to
use the POSIX test [ -d "$SECURITY_PLUGIN_DIR" ] (note the single brackets and
quoted variable) while keeping the existing chmod/chgrp/chmod -R g+rwX sequence
and overall command chaining intact so behavior and exit codes remain the same.
- Around line 78-84: The Dockerfile RUN block that sets permissions for
$OPENSEARCH_HOME is performing redundant filesystem traversals: the existing
chmod -R g+rwX $OPENSEARCH_HOME already grants group read on files and group
execute on directories via the capital X, so the subsequent find
$OPENSEARCH_HOME -type d -exec chmod g+x {} + and find $OPENSEARCH_HOME -type f
-exec chmod g+r {} + are unnecessary; remove those two find ... -exec lines and
keep the chgrp -R 0 $OPENSEARCH_HOME && chmod -R g+rwX $OPENSEARCH_HOME sequence
in the RUN command to achieve the same effect with a single traversal.
- Around line 97-105: The Dockerfile currently repeats recursive permission
fixes (chgrp -R 0 $OPENSEARCH_HOME and chmod -R g+rwX $OPENSEARCH_HOME) multiple
times around the COPY --chown step, the RUN ./opensearch-onetime-setup.sh step,
and again in the entrypoint chmod g+x; consolidate by running the one-time setup
(opensearch-onetime-setup.sh) before any final ownership/permission adjustments,
then perform a single chgrp -R 0 $OPENSEARCH_HOME && chmod -R g+rwX
$OPENSEARCH_HOME pass and remove the earlier and later redundant permission
commands (including the entrypoint chmod g+x) so only one recursive traversal
occurs.
- Around line 86-89: Remove the RUN block that writes to
/etc/profile.d/java_home.sh and the ls -l $OPENSEARCH_HOME step because
profile.d scripts aren't sourced for container processes; instead rely on the
existing ENV directives that set JAVA_HOME and PATH (the ENV for JAVA_HOME and
PATH present later in this Dockerfile) to expose the environment to the
container runtime.
Signed-off-by: josedev-union <josebarato321@gmail.com>
Signed-off-by: josedev-union <josebarato321@gmail.com>
|
Hi @josedev-union If this is related to k8s operator then maybe the dockerfile can live in that repository (so k8s operator maintainer can properly maintain it) and we can discuss about potentially release this either on github package or some other spaces. This build repo in particular is hosting the build scripts for releases on https://opensearch.org/downloads/ only for now. Thanks. |
Hi @peterzhuamazon year, the final goal is still to get the operator certified by Red Hat. |
docker/release/dockerfiles/opensearch-dashboards.ubi10.dockerfile
Outdated
Show resolved
Hide resolved
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit ff77221.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
PR Reviewer Guide 🔍(Review updated until commit 3e1e0f3)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 3e1e0f3
Previous suggestionsSuggestions up to commit ff77221
|
Signed-off-by: josedev-union <josebarato321@gmail.com>
josedev-union
force-pushed
the
add-ubi-images
branch
from
ff77221 to
3e1e0f3
Compare
|
Persistent review updated to latest commit 3e1e0f3 |
|
Can i request the review please? |
3 participants
Description
This PR adds dockerfiles ubi-based.
Issues Resolved
closes #3625
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.