-
Notifications
You must be signed in to change notification settings - Fork 95
fix(RELEASE-1701): return latest advisory URL when all components already released #1474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
969cd53
to
c986d74
Compare
/retest |
c986d74
to
f685e40
Compare
/retest |
1 similar comment
/retest |
/retest |
...managed/filter-already-released-advisory-images/filter-already-released-advisory-images.yaml
Outdated
Show resolved
Hide resolved
- "$(tasks.run-file-updates.results.sourceDataArtifact)=$(params.dataDir)" | ||
- "$(tasks.process-product-sbom.results.sourceDataArtifact)=$(params.dataDir)" | ||
- "$(tasks.process-component-sbom.results.sourceDataArtifact)=$(params.dataDir)" | ||
- "$(tasks.filter-already-released-advisory-images.results.sourceDataArtifact)=$(params.dataDir)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you make sure that it won't overwrite the advisory url with the empty string when the advisory did not already exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for mentioning! fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't really advocating for adding a new result, just to make sure that the empty string didn't overwrite the real one, but I'll leave implementation up to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the potential overwrite issue by making the filter task only create advisory data in the results file when skip_release=true
Pipeline behavior: When all components are already released (skip_release=true), the downstream tasks (including create-advisory and update-cr-status) are automatically skipped due to their when conditions. The latest advisory URL is exposed as a task result from the filter task for visibility in logs/UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ticket also talks about the task currently returning "just any" already released advisory instead of the latest. I don't see any changes here to address that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yes so create advisory also needs to return the latest match advisory URL. And filter-already-released abisory task do the same.
Does it make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acceptance criteria
- The latest advisory is returned as a new result to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both tasks now sort advisories by modification time (newest first) and return the first matching advisory (which is the latest)
...lter-already-released-advisory-images-task/filter-already-released-advisory-images-task.yaml
Outdated
Show resolved
Hide resolved
...lter-already-released-advisory-images-task/filter-already-released-advisory-images-task.yaml
Outdated
Show resolved
Hide resolved
I am making some adjustments, putting it back in the draft quickly. I will ping once ready for review again. |
5736055
to
1ae982a
Compare
/retest |
When all images in a snapshot are already released in advisories, the filter task now returns the URL of the latest matching advisory instead of no URL. This addresses user confusion during retries where they received older advisory URLs. - Add latest_advisory_url and latest_advisory_internal_url results - Write results file with advisory info for update-cr-status - Update pipeline to include filter task results - Add comprehensive tests for new functionality Signed-off-by: Happy Bhati <[email protected]>
1ae982a
to
6e9a5a2
Compare
/retest |
2 similar comments
/retest |
/retest |
When all images in a snapshot are already released in advisories, the filter task now returns the URL of the latest matching advisory instead of no URL. This addresses user confusion during retries where they received older advisory URLs.
Describe your changes
Relevant Jira
Checklist before requesting a review
do not merge
label if there's a dependency PRrelease-service-maintainers
handle if you are unsure who to tagSigned-off-by: My name <email>
.github/scripts/readme_generator.sh
and verified the results using.github/scripts/check_readme.sh