chore(cloud-native): upgrade cryptography library in OCI images#13488
Open
chore(cloud-native): upgrade cryptography library in OCI images#13488
Conversation
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
📝 WalkthroughWalkthroughBumps bellsoft Liberica Java images from 17.0.17 → 17.0.18 and updates ENV JANS_SOURCE_VERSION to commit 1b66569...4547e across multiple Dockerfiles; upgrades grpcio from 1.72.0 → 1.76.0 in multiple requirements.txt files; removes kc-sync runtime in cloudtools entrypoint. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
mo-auto
added
comp-docker-jans-all-in-one
…hy upgrade Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Member
|
| Status | Scan Engine |  Critical |
 High |
 Medium |
 Low |
Total (0) |
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docker-jans-cloudtools/Dockerfile (2)
47-59: 🧹 Nitpick | 🔵 TrivialRemove the KC scheduler scaffolding in the same cleanup.
Lines 54-59 no longer fetch the scheduler template source, but the image still carries the scheduler setup from earlier stages. Since
docker-jans-cloudtools/scripts/entrypoint.sh:10-45keepsrun_kc_sync()commented out, this is dead payload for now and only increases image size and CVE surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-cloudtools/Dockerfile` around lines 47 - 59, Remove the unused KC scheduler scaffolding to avoid shipping dead payload: delete the mkdir -p /app/templates/kc-scheduler and any steps that add or copy scheduler assets (e.g., removal of /app/templates/kc-scheduler creation in the RUN that performs git sparse-checkout and any prior COPY/ADD or git sparse-checkout entries for scheduler templates); ensure the Dockerfile no longer adds jans-pycloudlib scheduler files and adjust the sparse-checkout/git commands accordingly (refer to the RUN that performs git clone/sparse-checkout and the mkdir invocation), since entrypoint.sh keeps run_kc_sync() commented out.
1-10:⚠️ Potential issue | 🟠 MajorPin the Alpine Python package revisions you are validating.
Lines 7-10 install
py3-cryptographyandpy3-grpciowithout pinned versions. Combined withapk upgrade --available, this allows Alpine packages to drift on future rebuilds, making the cryptography/grpcio upgrade non-reproducible. Note that whilegrpcio==1.76.0is pinned in requirements.txt for PyPI installation,py3-cryptographyhas no version control in requirements.txt and is solely dependent on unpinned Alpine packages.Proposed fix
+ARG PY3_CRYPTOGRAPHY_VERSION=<exact-apk-revision> +ARG PY3_GRPCIO_VERSION=<exact-apk-revision> RUN apk update \ - && apk upgrade --available \ - && apk add --no-cache openssl python3 curl tini py3-cryptography py3-psycopg2 py3-grpcio \ + && apk add --no-cache \ + openssl \ + python3 \ + curl \ + tini \ + py3-psycopg2 \ + "py3-cryptography=${PY3_CRYPTOGRAPHY_VERSION}" \ + "py3-grpcio=${PY3_GRPCIO_VERSION}" \ && apk add --no-cache --virtual .build-deps wget git🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-cloudtools/Dockerfile` around lines 1 - 10, The Dockerfile RUN that does "apk upgrade --available" and installs py3-cryptography and py3-grpcio allows Alpine packages to drift; pin those Alpine Python package revisions by specifying exact package versions in the same RUN that installs packages (e.g., use py3-cryptography=<version> and py3-grpcio=<version> with apk add) or move those Python deps into a reproducible build step (wheel/PyPI with pinned versions), and remove or avoid a blanket "apk upgrade --available" so future rebuilds remain reproducible; update the RUN line that installs openssl python3 curl tini py3-cryptography py3-psycopg2 py3-grpcio accordingly.
🤖 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-jans-cloudtools/scripts/entrypoint.sh`:
- Around line 29-45: Removing the run_kc_sync handler makes incoming "kc-sync"
args silently fall through to show_help (exit 0); restore explicit handling so
"kc-sync" either invokes the original run_kc_sync flow or fails loudly. Re-add a
kc-sync branch in the main arg dispatch that checks for "kc-sync" and either
calls run_kc_sync (restore the commented run_kc_sync function/body) or prints an
explicit error and exits non-zero (e.g., exit 1) so the Helm CronJob doesn't
become a silent no-op; alternatively update the chart that still passes
"kc-sync" to stop doing so.
---
Outside diff comments:
In `@docker-jans-cloudtools/Dockerfile`:
- Around line 47-59: Remove the unused KC scheduler scaffolding to avoid
shipping dead payload: delete the mkdir -p /app/templates/kc-scheduler and any
steps that add or copy scheduler assets (e.g., removal of
/app/templates/kc-scheduler creation in the RUN that performs git
sparse-checkout and any prior COPY/ADD or git sparse-checkout entries for
scheduler templates); ensure the Dockerfile no longer adds jans-pycloudlib
scheduler files and adjust the sparse-checkout/git commands accordingly (refer
to the RUN that performs git clone/sparse-checkout and the mkdir invocation),
since entrypoint.sh keeps run_kc_sync() commented out.
- Around line 1-10: The Dockerfile RUN that does "apk upgrade --available" and
installs py3-cryptography and py3-grpcio allows Alpine packages to drift; pin
those Alpine Python package revisions by specifying exact package versions in
the same RUN that installs packages (e.g., use py3-cryptography=<version> and
py3-grpcio=<version> with apk add) or move those Python deps into a reproducible
build step (wheel/PyPI with pinned versions), and remove or avoid a blanket "apk
upgrade --available" so future rebuilds remain reproducible; update the RUN line
that installs openssl python3 curl tini py3-cryptography py3-psycopg2 py3-grpcio
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7d44447-b6b1-4061-a458-98d9d0cbd455
📒 Files selected for processing (20)
docker-jans-all-in-one/Dockerfiledocker-jans-all-in-one/app/requirements.txtdocker-jans-auth-server/Dockerfiledocker-jans-auth-server/requirements.txtdocker-jans-casa/Dockerfiledocker-jans-casa/requirements.txtdocker-jans-cloudtools/Dockerfiledocker-jans-cloudtools/requirements.txtdocker-jans-cloudtools/scripts/entrypoint.shdocker-jans-config-api/Dockerfiledocker-jans-config-api/requirements.txtdocker-jans-configurator/Dockerfiledocker-jans-fido2/Dockerfiledocker-jans-fido2/requirements.txtdocker-jans-link/Dockerfiledocker-jans-link/requirements.txtdocker-jans-persistence-loader/Dockerfiledocker-jans-persistence-loader/requirements.txtdocker-jans-scim/Dockerfiledocker-jans-scim/requirements.txt
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docker-jans-cloudtools/scripts/entrypoint.sh (1)
92-94:⚠️ Potential issue | 🟠 MajorDisabling
kc-synchere now breaks the shipped CronJob and documented CLI.
charts/janssen-all-in-one/templates/cronjobs.yaml:110-211still schedulesargs: ["kc-sync"]whenkc-scheduler.enabledandsaml.enabledare true, so this branch turns that deployment path into a permanently failing CronJob.docker-jans-cloudtools/README.md:186-194also still documentscloudtools kc-syncas a supported command. Please land the chart/docs removal in the same PR, or keeprun_kc_syncuntil those consumers are updated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-cloudtools/scripts/entrypoint.sh` around lines 92 - 94, The branch that unconditionally fails the "kc-sync" case breaks external consumers; revert the change so the "kc-sync" case invokes the existing run_kc_sync logic instead of exiting with error, i.e., replace the echo+exit with a call to run_kc_sync (or conditionalize the error only when both the scheduler and SAML flags indicate it should be disabled), and coordinate updating the chart/template and README that still reference "kc-sync" in the same PR so deployments and docs remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docker-jans-cloudtools/scripts/entrypoint.sh`:
- Around line 92-94: The branch that unconditionally fails the "kc-sync" case
breaks external consumers; revert the change so the "kc-sync" case invokes the
existing run_kc_sync logic instead of exiting with error, i.e., replace the
echo+exit with a call to run_kc_sync (or conditionalize the error only when both
the scheduler and SAML flags indicate it should be disabled), and coordinate
updating the chart/template and README that still reference "kc-sync" in the
same PR so deployments and docs remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf88f743-34a0-4372-81bc-d4b02eec1054
📒 Files selected for processing (1)
docker-jans-cloudtools/scripts/entrypoint.sh
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
2 participants
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.
Prepare
Description
Target issue
closes #13487
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit
Chores
Refactor