Untagged multiarch images #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds new functionality to inspect Docker multi-arch images in Cloudsmith repositories with support for finding and optionally deleting untagged manifest lists. The changes transform the repository from a generic CENG template to a Support Engineering resource collection with a specific Docker multi-arch inspection tool.
Key Changes:
- Introduced
--untaggedand--untagged-deleteflags to handle untagged manifest lists - Added comprehensive bash script for Docker multi-arch image inspection
- Simplified repository README to reflect new purpose as Support Engineering resources
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Simplified from generic CENG template to Support Engineering resources collection |
| Docker/Multi-Arch-Inspector/run.sh | New bash script that inspects Docker multi-arch images, displays digests/platforms/status/downloads, and supports finding/deleting untagged manifests |
| Docker/Multi-Arch-Inspector/README.md | Documentation for the multi-arch inspector tool with usage instructions and implementation overview |
| Docker/Multi-Arch-Inspector/results-golang.txt | Example output showing inspection results for golang images |
| Docker/Multi-Arch-Inspector/results-cgr-jdk.txt | Example output showing inspection results for JDK images |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| done | ||
| set -- "${ARGS[@]}" | ||
|
|
||
| # Usage: ./run.sh <org> <repo> <img> |
There was a problem hiding this comment.
The usage comment is outdated and doesn't reflect the current script signature. Based on the code at lines 43-46, the script expects 4 parameters (cloudsmith_url, workspace, repo, img), but the comment says <org> <repo> <img> (only 3 parameters). This should be updated to match the actual implementation or the README documentation which mentions "domain, org, repo and image name".
| # Usage: ./run.sh <org> <repo> <img> | |
| # Usage: ./run.sh <domain> <org> <repo> <img> |
| elif [[ "$arg" == "--untagged-delete" ]]; then | ||
| UNTAGGED_DELETE=true |
There was a problem hiding this comment.
The --untagged-delete flag can be used without --untagged, but based on lines 11-12 and 294, it will only have an effect when used together with --untagged. Consider either: (1) documenting that --untagged-delete requires --untagged, or (2) adding validation to ensure --untagged-delete is only accepted when --untagged is also specified, providing a clear error message otherwise.
| ARGS+=("$arg") | ||
| fi | ||
| done | ||
| set -- "${ARGS[@]}" |
There was a problem hiding this comment.
Potential bug: When no non-flag arguments are provided, ARGS[@] will be empty and set -- "${ARGS[@]}" could fail or behave unexpectedly with set -u (which is enabled at line 2). This can cause the script to exit prematurely when run with only flags like ./run.sh --untagged. Consider handling the case when ARGS is empty, e.g., set -- "${ARGS[@]+"${ARGS[@]}"}".
| set -- "${ARGS[@]}" | |
| set -- "${ARGS[@]+"${ARGS[@]}"}" |
|
|
||
| completed() { printf '%s%s%s %s\n' "$GREEN" "$CHECK" "$RESET" "$*"; } | ||
| progress() { printf '%s%s%s %s\n' "$YELLOW" "$TIMER" "$RESET" "$*"; } | ||
| quarantined() { printf '%s%s%s %s\n' "$ORANGE" "$VULN" "$RESET" "$*"; } |
There was a problem hiding this comment.
Undefined variable $ORANGE used in the quarantined() function. The color setup at lines 24-28 only defines $GREEN, $RED, and $RESET, but $ORANGE is never defined. This will result in an empty/uncolored output for the quarantined function.
| tag=" - ${images[t]}" | ||
| echo "$tag" | ||
| done | ||
| if $UNTAGGED; then |
There was a problem hiding this comment.
Missing validation: When --untagged flag is used, the script doesn't validate that required parameters (WORKSPACE, REPO, IMG) are provided. If these are empty, the API calls will fail with unclear error messages. Consider adding validation similar to the non-untagged path or providing clearer error messages.
| if $UNTAGGED; then | |
| if $UNTAGGED; then | |
| # Validate required parameters for untagged mode | |
| if [[ -z "${WORKSPACE:-}" || -z "${REPO:-}" || -z "${IMG:-}" ]]; then | |
| echo "Error: WORKSPACE, REPO, and IMG must be set when using --untagged." >&2 | |
| exit 1 | |
| fi |
| curl -sS -X DELETE "${AUTH_HEADER[@]}" \ | ||
| "https://api.cloudsmith.io/v1/packages/${WORKSPACE}/${REPO}/${slug}/" | ||
| echo " Deleted." |
There was a problem hiding this comment.
The deletion operation at lines 294-300 lacks error handling. If the DELETE request fails (e.g., due to authentication issues, network problems, or cascading feature flag not enabled), the script will still print "Deleted." misleadingly. Consider checking the HTTP response code and providing appropriate error messages. The PR description mentions this "requires the cascading feature flag" but there's no check or user-friendly error message if this requirement isn't met.
| curl -sS -X DELETE "${AUTH_HEADER[@]}" \ | |
| "https://api.cloudsmith.io/v1/packages/${WORKSPACE}/${REPO}/${slug}/" | |
| echo " Deleted." | |
| # Perform DELETE and capture HTTP status and response | |
| response=$(curl -sS -w "%{http_code}" -o /tmp/delete_response.txt -X DELETE "${AUTH_HEADER[@]}" \ | |
| "https://api.cloudsmith.io/v1/packages/${WORKSPACE}/${REPO}/${slug}/") | |
| http_code="${response: -3}" | |
| body=$(cat /tmp/delete_response.txt) | |
| if [[ "$http_code" =~ ^2 ]]; then | |
| echo " Deleted." | |
| else | |
| echo " Failed to delete package: HTTP $http_code" | |
| if [[ "$http_code" == "403" ]] && grep -qi "cascading" <<< "$body"; then | |
| echo " Error: Cascading delete feature flag is not enabled for this repository." | |
| echo " Please enable the cascading feature flag to allow deletion." | |
| else | |
| echo " Response: $body" | |
| fi | |
| fi |
| getDigestData "${DIGESTS[i]}" | ||
| done | ||
|
|
There was a problem hiding this comment.
Potential issue with variable scope: The totalDownloads variable is declared as local at line 289 but is never used or displayed. Unlike in the getDockerDigests function (line 207, commented out), there's no output of the total downloads for untagged images. This creates an inconsistency in the output format between tagged and untagged images.
| getDigestData "${DIGESTS[i]}" | |
| done | |
| # getDigestData should echo the downloads as its last output, so we capture it | |
| digest_downloads=$(getDigestData "${DIGESTS[i]}") | |
| totalDownloads=$((totalDownloads + digest_downloads)) | |
| done | |
| # Print total downloads for this untagged image (for consistency) | |
| echo "Total downloads for (untagged) [List]: $totalDownloads" |
| SEP_LINE="+----------------------+-----------------+--------------------------------+------------+-----------------------------------------------------------------------------+" | ||
|
|
||
| completed() { printf '%s%s%s %s\n' "$GREEN" "$CHECK" "$RESET" "$*"; } | ||
| progress() { printf '%s%s%s %s\n' "$YELLOW" "$TIMER" "$RESET" "$*"; } |
There was a problem hiding this comment.
Undefined variable $YELLOW used in the progress() function. The color setup at lines 24-28 only defines $GREEN, $RED, and $RESET, but $YELLOW is never defined. This will result in an empty/uncolored output for the progress function.
📄 Summary