Skip to content

Conversation

@dougszumski
Copy link
Member

@dougszumski dougszumski commented Nov 27, 2025

Backport from 2025.1

@dougszumski dougszumski requested a review from a team as a code owner November 27, 2025 10:42
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the reliability of fetching Docker image lists by using the --format option, which is a good change. However, the script does not correctly adapt the parsing of this new output format, leading to a potential bug when processing image names with port numbers in their repository. I've left a specific comment with details on how to fix this to make the change complete and robust.

Comment on lines +26 to +29
docker image ls \
--filter "reference=ark.stackhpc.com/stackhpc-dev/*:$2*" \
--format "{{.Repository}}:{{.Tag}}" \
> "$output_file"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using --format is a great improvement for robustness. However, this change is incomplete as it doesn't update the logic that parses the command's output. The parsing on line 34 is now incorrect and will fail for image names that include a port in the registry name (e.g., my.registry:5000/image:tag) due to the cut -f 1,2 -d: command.

To complete this fix, please also update line 34 in this PR. It should be simplified to:

images=$(cat "$output_file")

This will correctly read the image names that are already formatted by the docker image ls command. Note that on line 34, $1-scanned-container-images.txt should also be replaced with $output_file for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants