ci: publish log4j-chain images to ghcr (replace ttl.sh)#139
Conversation
The four log4j-chain demo images
(backend-{vulnerable,contained,patched} + attacker) currently rot on
ttl.sh after 24h. That keeps breaking fresh-PG provisioning when the
makefile / cluster-deploy pulls them; the manifests in
example/log4j-chain/{backend-b,backend-c,log4j-chain,log4j-attacks}.yaml
already reference ghcr.io/k8sstormcenter/log4j-chain-<component>:latest,
they just had nothing publishing them there.
This workflow mirrors ci-chain-images.yaml shape: matrix per component,
SHA + branch + latest tags, pinned GitHub Actions SHAs per repo policy.
Triggers only on changes under example/log4j-chain/backend/ or
example/log4j-chain/attacker/ + the workflow file itself.
amd64-only — the demo cluster is amd64 and the maven builds + JDK base
images take significantly longer multi-arch. arm64 can be added if
a consumer ever needs it.
No smoke-test step: the existing chain workflow's /healthz check
doesn't generalise to the attacker (marshalsec LDAP + python HTTP
serving Payload.class — no health endpoint). Push success is the
green signal; the demo runbooks exercise the images end-to-end.
Flagged by dx-agent in k8sstormcenter/pixie#47 — needed for the #7
pgsql re-test and the M6 combined run.
📝 WalkthroughWalkthroughA new GitHub Actions workflow file adds automated building and publishing of four Changeslog4j-chain Container Image Publishing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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: 5
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml:
- Around line 53-54: The Checkout step (named "Checkout" and using
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683) is missing
persist-credentials: false; update that step to add persist-credentials: false
so the GitHub token is not written into .git/config and cannot be copied into
Docker build contexts or accessed by later steps.
- Line 67: Remove the unused output by deleting the line that writes the
`primary` output to GitHub Actions outputs; specifically remove the echo that
appends "primary=${IMG}:${SHORT_SHA}" to the file referenced by `GITHUB_OUTPUT`
so that `primary` is no longer declared (leave other outputs/uses of IMG and
SHORT_SHA intact).
- Around line 28-31: Add a short explanatory comment above the permissions block
that clarifies why packages: write is required (e.g., to publish or update build
artifacts/packages from CI), so update the permissions section containing
"permissions:", "contents: read", and "packages: write" to include that comment;
keep the comment concise and specific to the CI workflow's need for package
write access.
- Around line 18-27: Add a concurrency policy to the GitHub Actions workflow to
prevent parallel runs from racing to push the same "latest" tag: inside the
workflow root (near the existing on: push / workflow_dispatch) add a concurrency
section that defines a stable group (e.g., use the branch ref like github.ref or
a combination such as "ci-log4j-chain-images-${{ github.ref }}") and set
cancel-in-progress: true so only the most recent run for a given branch
proceeds; update the workflow that currently contains the on: push block to
include this concurrency configuration to serialize pushes that could overwrite
the latest image.
- Line 61: The IMG variable currently uses ghcr.io/${{ github.repository_owner
}}/${{ matrix.component }} which causes forked PRs to target the fork owner’s
GHCR; to clarify intent either (A) keep the dynamic value to publish to the
workflow runner's owner (no change), or (B) explicitly target the upstream org
by replacing IMG with ghcr.io/k8sstormcenter/${{ matrix.component }} (only do
this if you intend CI to push into the k8sstormcenter namespace), or (preferred
for safety) add a conditional that sets IMG to k8sstormcenter only when
github.repository_owner == 'k8sstormcenter' and otherwise uses
github.repository_owner, updating the IMG assignment in the CI job where IMG is
defined.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 70128ee8-9289-4b2a-9c75-28e85d724a07
📒 Files selected for processing (1)
.github/workflows/ci-log4j-chain-images.yaml
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - 'example/log4j-chain/backend/**' | ||
| - 'example/log4j-chain/attacker/**' | ||
| - '.github/workflows/ci-log4j-chain-images.yaml' | ||
| workflow_dispatch: | ||
|
|
There was a problem hiding this comment.
Add concurrency control to prevent race conditions on the latest tag.
Without concurrency limits, parallel pushes to main will spawn multiple workflow runs that race to push the latest tag. If two commits land in quick succession, the older commit's workflow could finish second and overwrite latest with a stale image.
🔒 Recommended fix to add concurrency group
on:
push:
branches:
- main
paths:
- 'example/log4j-chain/backend/**'
- 'example/log4j-chain/attacker/**'
- '.github/workflows/ci-log4j-chain-images.yaml'
workflow_dispatch:
+concurrency:
+ group: log4j-chain-images-${{ github.ref }}
+ cancel-in-progress: true
+
permissions:
contents: read
packages: write📝 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.
| on: | |
| push: | |
| branches: | |
| - main | |
| paths: | |
| - 'example/log4j-chain/backend/**' | |
| - 'example/log4j-chain/attacker/**' | |
| - '.github/workflows/ci-log4j-chain-images.yaml' | |
| workflow_dispatch: | |
| on: | |
| push: | |
| branches: | |
| - main | |
| paths: | |
| - 'example/log4j-chain/backend/**' | |
| - 'example/log4j-chain/attacker/**' | |
| - '.github/workflows/ci-log4j-chain-images.yaml' | |
| workflow_dispatch: | |
| concurrency: | |
| group: log4j-chain-images-${{ github.ref }} | |
| cancel-in-progress: true |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 18-26: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml around lines 18 - 27, Add a
concurrency policy to the GitHub Actions workflow to prevent parallel runs from
racing to push the same "latest" tag: inside the workflow root (near the
existing on: push / workflow_dispatch) add a concurrency section that defines a
stable group (e.g., use the branch ref like github.ref or a combination such as
"ci-log4j-chain-images-${{ github.ref }}") and set cancel-in-progress: true so
only the most recent run for a given branch proceeds; update the workflow that
currently contains the on: push block to include this concurrency configuration
to serialize pushes that could overwrite the latest image.
Source: Linters/SAST tools
| permissions: | ||
| contents: read | ||
| packages: write | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding an explanatory comment for the permissions block.
Adding a brief comment explaining why packages: write is required improves clarity for future maintainers.
📝 Suggested improvement
+# Read repo contents for checkout; write packages to push images to GHCR.
permissions:
contents: read
packages: write🧰 Tools
🪛 zizmor (1.25.2)
[error] 30-30: overly broad permissions (excessive-permissions): packages: write is overly broad at the workflow level
(excessive-permissions)
[warning] 30-30: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml around lines 28 - 31, Add a
short explanatory comment above the permissions block that clarifies why
packages: write is required (e.g., to publish or update build artifacts/packages
from CI), so update the permissions section containing "permissions:",
"contents: read", and "packages: write" to include that comment; keep the
comment concise and specific to the CI workflow's need for package write access.
Source: Linters/SAST tools
| - name: Checkout | ||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 |
There was a problem hiding this comment.
Set persist-credentials: false to prevent credential leakage.
The checkout action defaults to persisting the GitHub token in .git/config. In a Docker build workflow, this credential could leak into the build context (via COPY .git patterns) or remain accessible to later steps, creating a security risk.
🛡️ Recommended fix
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 53-54: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml around lines 53 - 54, The
Checkout step (named "Checkout" and using
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683) is missing
persist-credentials: false; update that step to add persist-credentials: false
so the GitHub token is not written into .git/config and cannot be copied into
Docker build contexts or accessed by later steps.
Source: Linters/SAST tools
| run: | | ||
| SHORT_SHA=$(git rev-parse --short HEAD) | ||
| BRANCH_SAFE="${GITHUB_REF_NAME//\//-}" | ||
| IMG=ghcr.io/${{ github.repository_owner }}/${{ matrix.component }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if repository_owner is hardcoded in similar workflows
rg -n 'ghcr\.io/k8sstormcenter' --type yaml -g '.github/workflows/*.yaml'Repository: k8sstormcenter/bob
Length of output: 211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the relevant section of the workflow around the IMG definition
FILE=".github/workflows/ci-log4j-chain-images.yaml"
echo "== File: $FILE =="
# Print lines 45-90 if the file is reasonably small
nl -ba "$FILE" | sed -n '45,90p'
# 2) Find all uses of github.repository_owner in workflow YAMLs
echo
echo "== Uses of github.repository_owner in workflows =="
rg -n 'github\.repository_owner' --type yaml .github/workflows || true
# 3) Re-run search for the hardcoded ghcr.io/k8sstormcenter pattern with correct globs
echo
echo "== Uses of ghcr.io/k8sstormcenter in workflows =="
rg -n 'ghcr\.io/k8sstormcenter' --type yaml .github/workflows || trueRepository: k8sstormcenter/bob
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci-log4j-chain-images.yaml"
echo "== File: $FILE (context around IMG) =="
# Show approximate line window (adjust if needed)
sed -n '50,80p' "$FILE" | cat -n
echo
echo "== Uses of github.repository_owner in workflows =="
rg -n 'github\.repository_owner' --type yaml .github/workflows || true
echo
echo "== Uses of ghcr.io in workflows =="
rg -n 'ghcr\.io/' --type yaml .github/workflows || true
echo
echo "== Uses of hardcoded ghcr.io/k8sstormcenter in workflows =="
rg -n 'ghcr\.io/k8sstormcenter' --type yaml .github/workflows || trueRepository: k8sstormcenter/bob
Length of output: 6236
Clarify GHCR publish target for fork runs in CI.
.github/workflows/ci-log4j-chain-images.yaml sets IMG to ghcr.io/${{ github.repository_owner }}/${{ matrix.component }}, so runs from forks will push to the fork owner’s GHCR namespace. This pattern matches .github/workflows/ci-chain-images.yaml; only hardcode k8sstormcenter if publishing to the upstream org is required.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 61-61: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 61-61: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml at line 61, The IMG variable
currently uses ghcr.io/${{ github.repository_owner }}/${{ matrix.component }}
which causes forked PRs to target the fork owner’s GHCR; to clarify intent
either (A) keep the dynamic value to publish to the workflow runner's owner (no
change), or (B) explicitly target the upstream org by replacing IMG with
ghcr.io/k8sstormcenter/${{ matrix.component }} (only do this if you intend CI to
push into the k8sstormcenter namespace), or (preferred for safety) add a
conditional that sets IMG to k8sstormcenter only when github.repository_owner ==
'k8sstormcenter' and otherwise uses github.repository_owner, updating the IMG
assignment in the CI job where IMG is defined.
Source: Linters/SAST tools
| TAGS="${TAGS},${IMG}:latest" | ||
| fi | ||
| echo "tags=${TAGS}" >> "$GITHUB_OUTPUT" | ||
| echo "primary=${IMG}:${SHORT_SHA}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Remove unused primary output.
The primary output is defined but never consumed by any subsequent step. Remove it to reduce confusion.
🧹 Suggested cleanup
echo "tags=${TAGS}" >> "$GITHUB_OUTPUT"
- echo "primary=${IMG}:${SHORT_SHA}" >> "$GITHUB_OUTPUT"📝 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.
| echo "primary=${IMG}:${SHORT_SHA}" >> "$GITHUB_OUTPUT" | |
| echo "tags=${TAGS}" >> "$GITHUB_OUTPUT" |
🤖 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 @.github/workflows/ci-log4j-chain-images.yaml at line 67, Remove the unused
output by deleting the line that writes the `primary` output to GitHub Actions
outputs; specifically remove the echo that appends "primary=${IMG}:${SHORT_SHA}"
to the file referenced by `GITHUB_OUTPUT` so that `primary` is no longer
declared (leave other outputs/uses of IMG and SHORT_SHA intact).
Review — ci-log4j-chain-images.yamlReviewed the workflow against the sibling ✅ Correct
🟡 One gap vs the sibling workflow — add a smoke test before tagging
- name: Smoke-test image
run: |
docker run -d --name t -p 8080:8080 ${{ steps.tags.outputs.primary }}
for i in $(seq 1 30); do curl -fsS localhost:8080/ >/dev/null 2>&1 && break; sleep 2; done
docker logs t | tail -20
docker rm -f t(Adjust port/path to the backend's actual listener; for the attacker image a Minor (optional)
Note for contextThe four LGTM to merge once the smoke-test step is added (or as-is if you want freshness restored now and the smoke gate as a fast-follow). |
1 participant
The four log4j-chain demo images
(backend-{vulnerable,contained,patched} + attacker) currently rot on ttl.sh after 24h. That keeps breaking fresh-PG provisioning when the makefile / cluster-deploy pulls them; the manifests in example/log4j-chain/{backend-b,backend-c,log4j-chain,log4j-attacks}.yaml already reference ghcr.io/k8sstormcenter/log4j-chain-:latest, they just had nothing publishing them there.
This workflow mirrors ci-chain-images.yaml shape: matrix per component, SHA + branch + latest tags, pinned GitHub Actions SHAs per repo policy. Triggers only on changes under example/log4j-chain/backend/ or example/log4j-chain/attacker/ + the workflow file itself.
amd64-only — the demo cluster is amd64 and the maven builds + JDK base images take significantly longer multi-arch. arm64 can be added if a consumer ever needs it.
No smoke-test step: the existing chain workflow's /healthz check doesn't generalise to the attacker (marshalsec LDAP + python HTTP serving Payload.class — no health endpoint). Push success is the green signal; the demo runbooks exercise the images end-to-end.
Flagged by dx-agent in k8sstormcenter/pixie#47 — needed for the #7 pgsql re-test and the M6 combined run.