Skip to content

fix(KONFLUX-10809): fail incrementer on empty tag list#2328

Open
FilipNikolovski wants to merge 1 commit into
konflux-ci:developmentfrom
FilipNikolovski:KONFLUX-10809
Open

fix(KONFLUX-10809): fail incrementer on empty tag list#2328
FilipNikolovski wants to merge 1 commit into
konflux-ci:developmentfrom
FilipNikolovski:KONFLUX-10809

Conversation

@FilipNikolovski

Copy link
Copy Markdown
Contributor

Describe your changes

Add extract_tags() that validates the list-tags response and checks repo existence via skopeo inspect when Tags is empty. If the repo exists but returned no tags, the task now fails instead of producing a potentially destructive increment value.

Assisted-by: Claude Code

Relevant Jira

https://redhat.atlassian.net/browse/KONFLUX-10809

Checklist before requesting a review

  • I have marked as draft or added do not merge label if there's a dependency PR
    • If you want reviews on your draft PR, you can add reviewers or add the release-service-maintainers handle if you are unsure who to tag
  • My commit message includes Signed-off-by: My name <email>
  • I read CONTRIBUTING.MD and commit formatting
  • I have run the README.md generator script in .github/scripts/readme_generator.sh and verified the results using .github/scripts/check_readme.sh
  • If an AI agent was used, I marked that via a commit footer like Assisted-By: Cursor

Comment on lines +103 to +108
elif [[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repoa"* ]] || \
[[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repo2"* ]]
then
# Genuinely new repos (no manifests yet) - inspect fails
echo "Error: repository not found" >&2
return 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice fix!
One small suggestion: consider adding an explicit mock for repo-empty-tags-existing for the skopeo inspect call. Currently it works via the default fallthrough, but making it explicit improves test clarity:

Suggested change
elif [[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repoa"* ]] || \
[[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repo2"* ]]
then
# Genuinely new repos (no manifests yet) - inspect fails
echo "Error: repository not found" >&2
return 1
elif [[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repo-empty-tags-existing"* ]]
then
# Existing repo that transiently returns empty tags - inspect succeeds
echo '{"config": {"mediaType": "application/vnd.oci.image.config.v1+json"}, "annotations": {}}'
return
elif [[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repoa"* ]] || \
[[ "$*" == "inspect --retry-times 3 --no-tags --raw docker://repo2"* ]]
then
# Genuinely new repos (no manifests yet) - inspect fails
echo "Error: repository not found" >&2
return 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks, updated!

@elenagerman

Copy link
Copy Markdown
Contributor

except for a small proposal: /LGTM

Add extract_tags() that validates the list-tags response and checks
repo existence via skopeo inspect when Tags is empty. If the repo
exists but returned no tags, the task now fails instead of producing
a potentially destructive increment value.

Assisted-by: Claude Code
Signed-off-by: Filip Nikolovski <fnikolov@redhat.com>
@FilipNikolovski

Copy link
Copy Markdown
Contributor Author

/retest

@FilipNikolovski

Copy link
Copy Markdown
Contributor Author

/retest

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants