Skip to content
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

fix(KFLUXBUGS-1581): force releaseNotes.type when cves defined #619

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

johnbieren
Copy link
Collaborator

This commit modifies the create-advisory task to fail when releaseNotes.type is RHSA but there are no CVEs listed as fixed. Also, if there are CVEs fixed, the type is forced to be RHSA regardless of what type the user provides.

Copy link

openshift-ci bot commented Oct 16, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

This commit modifies the create-advisory task to fail when
releaseNotes.type is RHSA but there are no CVEs listed as fixed. Also,
if there are CVEs fixed, the type is forced to be RHSA regardless of
what type the user provides.

Signed-off-by: Johnny Bieren <[email protected]>
Copy link

openshift-ci bot commented Oct 16, 2024

@johnbieren: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/release-pipelines-e2e-suite 9b3c6ae link true /test release-pipelines-e2e-suite

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@@ -55,10 +55,10 @@ spec:
RESULTS_FILE="$(workspaces.data.path)/$(params.resultsDirPath)/create-advisory-results.json"
# Obtain application from snapshot
application=$(jq -rc .application "$(workspaces.data.path)/$(params.snapshotPath)")
application=$(jq -r .application "$(workspaces.data.path)/$(params.snapshotPath)")
Copy link
Contributor

Choose a reason for hiding this comment

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

-r doesn't make sense here - it's an object, not just a string. So if anything, I would change it to -c.

# Obtain origin workspace from releasePlanAdmission
origin=$(jq -rc '.spec.origin' "$(workspaces.data.path)/$(params.releasePlanAdmissionPath)")
origin=$(jq -r '.spec.origin' "$(workspaces.data.path)/$(params.releasePlanAdmissionPath)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is a string, right? So yeah, I agree. But how did this work before? There must have been redundant "" in the string passed to the IR...

Comment on lines +75 to +76
NUM_CVES=$(jq '.content.images[]?.cves.fixed // 0 | length' <<< "$advisoryData" \
| awk '{sum=sum+$0} END{print sum}')
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do the sum directly with jq. Also, while it works with the default 0, you're expecting an array, so defaulting to [] seems more correct.

Suggested change
NUM_CVES=$(jq '.content.images[]?.cves.fixed // 0 | length' <<< "$advisoryData" \
| awk '{sum=sum+$0} END{print sum}')
NUM_CVES=$(jq '[.content.images[]?.cves.fixed // [] | length] | add' <<< "$advisoryData")

But if you prefer awk, that's fine with me.

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.

2 participants