Add skupper-source OCI image for downstream build contexts - #2560
Add skupper-source OCI image for downstream build contexts#2560AryanP123 wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe change adds Go dependency vendoring to the source OCI image, local Docker and Podman build and push targets, and release workflow publication to Quay. ChangesSource image publishing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The release job passes the Quay password through Docker command-line arguments, creating a bounded credential-exposure risk in CI; the PR is otherwise mergeable with explicit owner awareness and follow-up to move the secret out of command-line arguments. Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant MakeTargets
participant Quay
ReleaseWorkflow->>MakeTargets: Build source image with release tag
MakeTargets->>Quay: Authenticate and push tagged source image
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| docker-push-source: docker-build-source | ||
| ${DOCKER} push "${REGISTRY}/${SOURCE_IMAGE}:${IMAGE_TAG}" | ||
|
|
||
| podman-build-source: | ||
| ${PODMAN} build $(SHARED_IMAGE_LABELS) -t "${REGISTRY}/${SOURCE_IMAGE}:${IMAGE_TAG}" -f Dockerfile.source . | ||
|
|
||
| podman-push-source: podman-build-source | ||
| ${PODMAN} push "${REGISTRY}/${SOURCE_IMAGE}:${IMAGE_TAG}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg --glob '!.git/**' -n -C 8 \
'docker-push-source|podman-push-source|make[[:space:]].*(docker|podman)-push' . || trueRepository: skupperproject/skupper
Length of output: 1292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(Makefile|\.github/workflows/|scripts/|release|ci)' || true
printf '%s\n' '--- push targets and release invocations ---'
rg --glob '!.git/**' -n -C 5 \
'(^|[[:space:]])(docker|podman)-push(-source)?([:space:]|$)|make[[:space:]]+[^#\n]*(push|release)|make[[:space:]]+-C[^#\n]*(push|release)' \
. || true
printf '%s\n' '--- Makefile target definitions ---'
rg -n '^[A-Za-z0-9_.-]+:.*(push|release)|^[A-Za-z0-9_.-]+:' MakefileRepository: skupperproject/skupper
Length of output: 5420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(Makefile|\.github/workflows/|scripts/|release|ci)' || true
printf '%s\n' '--- push targets and release invocations ---'
rg --glob '!.git/**' -n -C 5 \
'(^|[[:space:]])(docker|podman)-push(-source)?([[:space:]$])|make[[:space:]]+[^#\n]*(push|release)|make[[:space:]]+-C[^#\n]*(push|release)' \
. || true
printf '%s\n' '--- Makefile target definitions ---'
rg -n '^[A-Za-z0-9_.-]+:.*(push|release)|^[A-Za-z0-9_.-]+:' MakefileRepository: skupperproject/skupper
Length of output: 3837
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n Makefile | sed -n '1,105p'
printf '%s\n' '--- source image and containerfile declarations ---'
rg -n -C 3 'CONTAINERFILES|SOURCE_IMAGE|Dockerfile\.source|docker-push|podman-push' Makefile .github scripts tests Dockerfile* 2>/dev/null || trueRepository: skupperproject/skupper
Length of output: 9595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path("Makefile").read_text()
containerfiles = re.search(r"^CONTAINERFILES\s*:=\s*(.+)$", text, re.MULTILINE).group(1).split()
source_target = "podman-push-source"
aggregate_targets = [f"podman-push-{Path(name).name.removeprefix('Dockerfile.')}" for name in containerfiles]
print("CONTAINERFILES:", containerfiles)
print("podman-push prerequisites:", aggregate_targets)
print("source target included:", source_target in aggregate_targets)
print("source target definition:", bool(re.search(r"^podman-push-source:\s+podman-build-source$", text, re.MULTILINE)))
PYRepository: skupperproject/skupper
Length of output: 537
Invoke a source push target in the release job.
CONTAINERFILES excludes Dockerfile.source, so aggregate podman-push does not invoke podman-push-source. If release automation uses only that aggregate target, skupper-source is not published. Invoke podman-push-source or docker-push-source explicitly.
| - run: | ||
| name: Docker login | ||
| command: docker login quay.io -u ${QUAY_LOGIN} -p ${QUAY_PASSWORD} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '370,410p' .circleci/config.yml
printf '\nDocker login references:\n'
rg -n -C 3 'docker login|QUAY_(LOGIN|PASSWORD)|docker-push-source' .circleci/config.yml Makefile 2>/dev/null || true
printf '\nShell expansion probe:\n'
bash -c 'set -- docker login quay.io -u ${QUAY_LOGIN} -p ${QUAY_PASSWORD}; printf "%s\n" "$@"' \
2>/dev/null || trueRepository: skupperproject/skupper
Length of output: 4284
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' 'Argument expansion with whitespace:'
QUAY_LOGIN='build user' QUAY_PASSWORD='secret with spaces' bash -c '
set -- docker login quay.io -u ${QUAY_LOGIN} -p ${QUAY_PASSWORD}
printf "argc=%s\n" "$#"
i=1
for arg in "$@"; do printf "arg%s=%q\n" "$i" "$arg"; i=$((i+1)); done
'
printf '\nRelevant job context and all matching commands:\n'
sed -n '160,190p' .circleci/config.yml
sed -n '370,405p' .circleci/config.yml
sed -n '415,455p' .circleci/config.yml
sed -n '495,520p' .circleci/config.yml
printf '\nMatching line numbers:\n'
rg -n 'docker login|helm registry login|skopeo login' .circleci/config.ymlRepository: skupperproject/skupper
Length of output: 4903
Use --password-stdin for registry authentication.
Line 397 exposes ${QUAY_PASSWORD} as a command-line argument and splits secrets containing whitespace. Apply the same change to the other registry login commands in .circleci/config.yml.
|
@AryanP123 should we also include a vendor directory as part of the source image? |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 037458c1-4a29-4775-8a2a-6cad6161fdc5
📒 Files selected for processing (3)
.dockerignoreDockerfile.sourceMakefile
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
|
@fgiorgetti Updated Dockerfile.source with a golang stage that runs go mod vendor before copying into scratch. Lmk what you think |
fgiorgetti
left a comment
fgiorgetti
left a comment
There was a problem hiding this comment.
@AryanP123 LGTM! Thank you.
4 participants
Fixes #2559
Summary by CodeRabbit