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

Handle multiple CVEs per issue in official CVE feed #117

Merged

Conversation

robert-cronin
Copy link
Contributor

@robert-cronin robert-cronin commented Jun 28, 2024

Fix multiple CVE handling in official CVE feed

Fixes kubernetes/website#47003

This change modifies the script used to generate the CVE feed to correctly handle issues containing multiple CVEs. Key updates:

  • Create separate entries for each CVE ID when multiple are present in a single issue
  • Maintain original structure and behavior for the first CVE entry
  • Ensure compatibility with existing feed consumers

This approach aims to resolve the malformed GUID issue while preserving the feed's integrity. Feedback and suggestions for improvement are welcome.

Here is the output from before and after the change:

before.txt
after.txt

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2024
@k8s-ci-robot
Copy link
Contributor

Welcome @robert-cronin!

It looks like this is your first PR to kubernetes/sig-security 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/sig-security has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 28, 2024
@PushkarJ
Copy link
Member

/hold

(Discussing more on possible solutions in linked issue)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2024
Copy link
Member

@PushkarJ PushkarJ left a comment

Choose a reason for hiding this comment

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

Few minor comments, I think this is okay to merge once these are addressed. I am also convinced this is the right approach to resolve this, so no issues in terms of revisiting that. Shared more here: kubernetes/website#47003 (comment)

- Modify script to create separate entries for each CVE ID when multiple are present in a single issue
@robert-cronin robert-cronin force-pushed the fix/multiple-cves-single-issue branch from 94591f2 to 84bb15a Compare June 29, 2024 04:21
@robert-cronin
Copy link
Contributor Author

@PushkarJ I've gone through and updated the script based on the feedback received. Here's the updated output:

after2.txt

Thank you for your suggestions!

@PushkarJ
Copy link
Member

@robert-cronin Thank you so much for your efforts in fixing this and addressing the comments and coming up with even better idea about dictionary copy

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 29, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: PushkarJ, robert-cronin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 29, 2024
@PushkarJ
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 29, 2024
@k8s-ci-robot k8s-ci-robot merged commit fbd194b into kubernetes:main Jun 29, 2024
2 checks passed
@robert-cronin
Copy link
Contributor Author

@robert-cronin Thank you so much for your efforts in fixing this and addressing the comments and coming up with even better idea about dictionary copy

/lgtm
/approve

No problems, happy to help 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid entry in vulnerability feed
3 participants