chore(docker): merge consecutive RUN instructions#3465
Closed
zarfishan20 wants to merge 0 commit intoOWASP:mainfrom
Closed
chore(docker): merge consecutive RUN instructions#3465zarfishan20 wants to merge 0 commit intoOWASP:mainfrom
zarfishan20 wants to merge 0 commit intoOWASP:mainfrom
Conversation
Contributor
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConsolidated consecutive RUN instructions across multiple backend and frontend Dockerfiles, merging cache mounts and cache-setup steps (APK, POETRY, PIP) into single chained RUN blocks while preserving existing package installs, symlink creation, and ownership operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/backend/Dockerfile.video (1)
14-31: Critical syntax error: Invalid&& --mount=...pattern.Lines 17 and 25 use
&& --mount=type=cache,...which is invalid Dockerfile syntax. Hadolint (DL1000) confirms this will cause a build failure.🐛 Proposed fix
RUN mkdir -p ${APK_CACHE_DIR} ${POETRY_CACHE_DIR} && \ - ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} - -&& --mount=type=cache,target=${PIP_CACHE_DIR} \ - python -m pip install poetry --cache-dir ${PIP_CACHE_DIR} + ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} + +RUN --mount=type=cache,target=${PIP_CACHE_DIR} \ + python -m pip install poetry --cache-dir ${PIP_CACHE_DIR} COPY --chmod=444 --chown=root:root poetry.lock pyproject.toml ./ RUN --mount=type=cache,target=${POETRY_CACHE_DIR},uid=${OWASP_UID},gid=${OWASP_GID} \ poetry install --no-root --without dev --without test && \ python -m pip uninstall -y poetry -&& --mount=type=cache,target=${APK_CACHE_DIR} \ +RUN --mount=type=cache,target=${APK_CACHE_DIR} \ apk update && apk upgrade && \ apk add ffmpeg \ # WeasyPrint dependencies: https://doc.courtbouillon.org/weasyprint/stable/first_steps.html so:libfontconfig.so.1 so:libgobject-2.0.so.0 so:libharfbuzz-subset.so.0 \ so:libharfbuzz.so.0 so:libpango-1.0.so.0 so:libpangoft2-1.0.so.0 ttf-freefont && \ chown -R owasp:owasp /home/owaspNote: In this file, the
COPYinstruction between the pip install and apk install prevents merging those RUN instructions. The fix restores valid syntax while keeping them separate.
🤖 Fix all issues with AI agents
In `@docker/backend/Dockerfile`:
- Around line 18-28: The Dockerfile has invalid use of "&& --mount=..." inside a
RUN shell chain; fix by placing the mount modifiers directly after RUN (e.g.
change "RUN mkdir -p ... && --mount=type=cache,target=${APK_CACHE_DIR} apk
update..." to separate or combined RUN instructions that start with "RUN
--mount=type=cache,target=${APK_CACHE_DIR}" and "RUN
--mount=type=cache,target=${PIP_CACHE_DIR}" as needed), keeping the same actions
(mkdir, ln -fns with ${APK_CACHE_DIR}/${POETRY_CACHE_DIR}/${APK_SYMLINK_DIR},
apk update/upgrade/addgroup/adduser/chown in the APK-mount RUN, and python -m
pip install poetry --cache-dir ${PIP_CACHE_DIR} in the PIP-mount RUN) so mounts
immediately follow RUN and shell chaining no longer contains "--mount".
In `@docker/backend/Dockerfile.fuzz`:
- Around line 11-22: The Dockerfile uses invalid "&& --mount=..." chaining; move
each --mount to directly follow the RUN keyword and split the commands
accordingly so the mount flags are part of the RUN instruction. Specifically,
make the APK cache mount part of the RUN that performs mkdir/ln/apk
update/upgrade/adduser (use RUN --mount=type=cache,target=${APK_CACHE_DIR} ...
followed by the chained shell commands that reference ${APK_CACHE_DIR} and
${APK_SYMLINK_DIR}), and make the pip install use its own RUN with
--mount=type=cache,target=${PIP_CACHE_DIR} so the pip --cache-dir points to
${PIP_CACHE_DIR}; ensure no "&& --mount=..." sequences remain.
In `@docker/backend/Dockerfile.local`:
- Around line 44-54: The RUN lines use invalid "&& --mount=..." chaining; update
the two occurrences that reference APK_CACHE_DIR/APK_SYMLINK_DIR and
PIP_CACHE_DIR so the --mount flags are part of the RUN instruction itself (e.g.,
use "RUN --mount=type=cache,target=${APK_CACHE_DIR} apk update ... apk add ..."
and "RUN --mount=type=cache,target=${PIP_CACHE_DIR} python -m pip install poetry
--cache-dir ${PIP_CACHE_DIR}") instead of prefixing --mount with "&&"; ensure
APK_SYMLINK_DIR logic (ln -s ...) is included in the same RUN or in a separate
RUN that correctly uses --mount without leading &&.
- Around line 14-24: The Dockerfile uses invalid "&& --mount=..." fragments; to
fix, combine these into a single RUN instruction that places all --mount=...
options immediately after RUN (e.g., RUN
--mount=type=cache,target=${APK_CACHE_DIR}
--mount=type=cache,target=${PIP_CACHE_DIR}
--mount=type=cache,target=${POETRY_CACHE_DIR}) and then chain the shell commands
with && inside that RUN; ensure you keep the mkdir -p ${APK_CACHE_DIR}
${POETRY_CACHE_DIR}, ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR}, apk
update/upgrade, addgroup/adduser/chown steps and the python -m pip install
poetry --cache-dir ${PIP_CACHE_DIR} all within that single RUN so the cache
mounts (APK_CACHE_DIR, PIP_CACHE_DIR, POETRY_CACHE_DIR) are applied correctly.
In `@docker/backend/Dockerfile.test`:
- Around line 12-22: The RUN lines use an invalid pattern "&& --mount=..."
causing Docker syntax errors; change each compound RUN so that cache mounts are
part of the RUN directive itself (e.g., use "RUN
--mount=type=cache,target=${APK_CACHE_DIR} ..." for the apk-related commands and
a separate "RUN --mount=type=cache,target=${PIP_CACHE_DIR} ..." for the
pip/poetry install). Concretely, move the "--mount=type=cache,target=..."
immediately after RUN and split the operations into two RUNs: one RUN --mount
for APK_CACHE_DIR (mkdir -p ${APK_CACHE_DIR} ${POETRY_CACHE_DIR}; ln -fns
${APK_CACHE_DIR} ${APK_SYMLINK_DIR}; apk update && apk upgrade && addgroup ...
&& adduser ... && chown -R owasp:owasp /home/owasp) and a separate RUN --mount
for PIP_CACHE_DIR to run "python -m pip install poetry --cache-dir
${PIP_CACHE_DIR}".
In `@docker/frontend/Dockerfile`:
- Around line 15-18: The RUN line currently places --mount after shell
continuations which is invalid; change the RUN to use BuildKit inline mount
syntax immediately after RUN (e.g., RUN
--mount=type=cache,target=${APK_CACHE_DIR}) and then perform the mkdir/ln/apk
commands in the same RUN command body so that APK_CACHE_DIR and APK_SYMLINK_DIR
are mounted before executing apk update && apk upgrade && apk add libc6-compat;
ensure the mount uses the same ${APK_CACHE_DIR} variable and keep the ln -fns
${APK_CACHE_DIR} ${APK_SYMLINK_DIR} and apk commands inside that RUN block.
- Around line 60-62: The version-check grep against "${TAR_DIR}/package.json" is
chained with addgroup/adduser so a failed check skips creating the nextjs user
and later breaks at USER nextjs; separate the concerns by running the grep check
first and explicitly fail with a clear error message if it does not match (e.g.,
check "grep -q 'version.*7.5.3' \"${TAR_DIR}/package.json\"" and if it fails
call echo >&2 with a descriptive message and exit 1), then independently run
addgroup --system --gid 1001 nodejs and adduser --system --uid 1001 -G nodejs
nextjs so user/group creation always occurs regardless of the version check
result.
In `@docker/frontend/Dockerfile.local`:
- Around line 14-17: The RUN line uses --mount mid-command which is invalid;
move the mount flag immediately after the RUN token and preserve the chained
commands: change the invocation so the RUN begins with the mount (e.g., RUN
--mount=type=cache,target=${APK_CACHE_DIR}) followed by the existing commands
(mkdir -p ${APK_CACHE_DIR} && ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} && apk
update && ...), ensuring the chain stays intact and no other tokens appear
before --mount; update the RUN that contains mkdir -p / ln -fns and apk update
to use this form.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/backend/Dockerfile.video (1)
20-29: Same critical syntax error:&&after--mountflag.Line 21 has
--mount=type=cache,target=${APK_CACHE_DIR} && \which will cause build failure.🐛 Proposed fix
RUN --mount=type=cache,target=${POETRY_CACHE_DIR},uid=${OWASP_UID},gid=${OWASP_GID} \ - --mount=type=cache,target=${APK_CACHE_DIR} && \ + --mount=type=cache,target=${APK_CACHE_DIR} \ poetry install --no-root --without dev --without test && \ python -m pip uninstall -y poetry && \ apk update && apk upgrade && \ apk add ffmpeg \ # WeasyPrint dependencies: https://doc.courtbouillon.org/weasyprint/stable/first_steps.html so:libfontconfig.so.1 so:libgobject-2.0.so.0 so:libharfbuzz-subset.so.0 \ so:libharfbuzz.so.0 so:libpango-1.0.so.0 so:libpangoft2-1.0.so.0 ttf-freefont && \ chown -R owasp:owasp /home/owasp
🤖 Fix all issues with AI agents
In `@docker/backend/Dockerfile.local`:
- Around line 14-22: The RUN line in Dockerfile.local incorrectly places
shell-chaining operators directly after a --mount flag (e.g.,
"--mount=type=cache,target=${PIP_CACHE_DIR} && \"), which is invalid; update the
RUN invocation so all --mount=... clauses come first and then the shell commands
follow (for example: RUN --mount=type=cache,target=${APK_CACHE_DIR}
--mount=type=cache,target=${PIP_CACHE_DIR} <newline> mkdir -p ${APK_CACHE_DIR}
${POETRY_CACHE_DIR} && ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} && apk update
&& apk upgrade && addgroup -S -g ${OWASP_GID} owasp && adduser -S -h /home/owasp
-u ${OWASP_UID} -G owasp owasp && chown -R owasp:owasp /home/owasp && python -m
pip install poetry --cache-dir ${PIP_CACHE_DIR}), ensuring no "&&" immediately
follows any --mount and referencing the same variables (${APK_CACHE_DIR},
${PIP_CACHE_DIR}, ${POETRY_CACHE_DIR}, ${APK_SYMLINK_DIR}, ${OWASP_GID},
${OWASP_UID}) and the RUN instruction in Dockerfile.local.
- Around line 42-50: The RUN line duplicates a mount token causing Dockerfile
syntax error; update the RUN in the final stage to declare both mounts as RUN
--mount=type=cache,target=${APK_CACHE_DIR}
--mount=type=cache,target=${PIP_CACHE_DIR} \ (i.e. remove the second repeated
"&& \" after the mount) and then continue the shell commands with && \ (keeping
mkdir -p ${APK_CACHE_DIR}, ln -s ${APK_CACHE_DIR} ${APK_SYMLINK_DIR}, apk
update/upgrade, apk add, addgroup/adduser and python -m pip install poetry
--cache-dir ${PIP_CACHE_DIR}) so both caches are mounted properly and the
command chain remains valid.
In `@docker/backend/Dockerfile.video`:
- Around line 14-17: The RUN instruction in the Dockerfile.video contains
invalid syntax and a trailing-space line continuation: remove the stray `&&`
immediately after `--mount=type=cache,target=${PIP_CACHE_DIR}` so the mount
option is attached to RUN properly, and delete the trailing space after the
backslash on the line containing `ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} &&
\` to ensure the line continuation is valid; keep the intent to create
`${APK_CACHE_DIR}` and `${POETRY_CACHE_DIR}` and then run `python -m pip install
poetry --cache-dir ${PIP_CACHE_DIR}` in the same RUN layer.
♻️ Duplicate comments (1)
docker/backend/Dockerfile (1)
18-26: Critical syntax error:&&cannot follow--mountflag.Line 19 has
--mount=type=cache,target=${PIP_CACHE_DIR} && \which is invalid Dockerfile syntax. The&&operator is for chaining shell commands, but--mountis a RUN instruction modifier—there's no shell command before the&&.This will cause the build to fail.
🐛 Proposed fix
RUN --mount=type=cache,target=${APK_CACHE_DIR} \ - --mount=type=cache,target=${PIP_CACHE_DIR} && \ - mkdir -p ${APK_CACHE_DIR} ${POETRY_CACHE_DIR} && \ - ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} &&\ + --mount=type=cache,target=${PIP_CACHE_DIR} \ + mkdir -p ${APK_CACHE_DIR} ${POETRY_CACHE_DIR} && \ + ln -fns ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} && \ apk update && apk upgrade && \ addgroup -S -g ${OWASP_GID} owasp && \ adduser -S -h /home/owasp -u ${OWASP_UID} -G owasp owasp && \ - chown -R owasp:owasp /home/owasp &&\ + chown -R owasp:owasp /home/owasp && \ python -m pip install poetry --cache-dir ${PIP_CACHE_DIR}
🧹 Nitpick comments (1)
docker/frontend/Dockerfile.local (1)
40-47: Consider consolidating the final stage RUN instructions.Per the PR objective of merging consecutive RUN instructions to reduce image layers, these two RUN blocks (lines 40-41 and 43-47) could potentially be merged. However, the current implementation is functionally correct.
♻️ Optional consolidation
-RUN mkdir -p ${APK_CACHE_DIR} && \ - ln -s ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} - -RUN --mount=type=cache,target=${APK_CACHE_DIR} \ +RUN --mount=type=cache,target=${APK_CACHE_DIR} \ + mkdir -p ${APK_CACHE_DIR} && \ + ln -s ${APK_CACHE_DIR} ${APK_SYMLINK_DIR} && \ apk update && \ apk add git && \ mkdir -p /home/owasp/.next && \ chown -R node:node /home/owasp
zarfishan20
force-pushed
the
merge-docker-run-instructions
branch
from
09c9938 to
e002985
Compare
|
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
This PR addresses SonarQube warnings by merging consecutive
RUNinstructionsin Dockerfiles to reduce image layers and follow Docker best practices.
No functional changes were introduced.
Resolves #3427
Checklist
make check-testlocally: all warnings addressed, tests passed