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.

Sign up for GitHub

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

Jump to bottom

[KEP-3203] Fetch and Render CVE JSON Feed #35228

Merged
merged 1 commit into from
Aug 8, 2022
Merged

[KEP-3203] Fetch and Render CVE JSON Feed #35228

merged 1 commit into from
Aug 8, 2022

Conversation

PushkarJ
Copy link
Member

@PushkarJ PushkarJ commented Jul 21, 2022 •
edited
Loading

Description

  • Pull JSON blob from queried issues
  • Use layout output formats + templates to generate HTML table and JSON blob
  • Add localized strings and caption for CVE feed
  • Add a new page to describe details about CVE feed and how to use it
  • Update existing pages and link the official CVE feed from it

Notes for Reviewers

Why are we regenerating JSON blob from the JSON blob in the bucket ?

  • Not all fields stored in the bucket are needed
  • JSON download directly to a file with Hugo data templates is not possible
  • Gives us better control and ability to add extra fields as needed
  • Pulling data through hugo data templates allow us to reuse built-in caching mechanisms

Why not version control the JSON blob?

To avoid multiple sources of truth. The content in the bucket will remain the only source of truth as the JSON blob is constructed from GitHub issues with label official-cve-feed (See this for more info: kubernetes/test-infra#23428). However, as a break glass, there is an option to temporarily push the JSON blob by overriding the layouts json file with raw JSON in cases where push to bucket fails or if bucket is inaccessible.

Are the locations for the layouts files correct?

Current locations are mostly my guess, so happy to refactor them as needed

TODO:

  • Update JSON blob location to k8s GCS bucket
  • Add more description in the PR
  • Address PR feedback from superseded PR

Preview: https://deploy-preview-35228--kubernetes-io-main-staging.netlify.app/docs/reference/issues-security/official-cve-feed/
KEP tracker: kubernetes/enhancements#3203
xref: kubernetes/sig-security#1
supersedes #31051 and #34765
/hold
/sig security docs

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/security Categorizes an issue or PR as relevant to SIG Security. sig/docs Categorizes an issue or PR as relevant to SIG Docs. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 21, 2022
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Jul 21, 2022
@PushkarJ PushkarJ changed the title [KEP-3203] Fetch and Render CVE JSON feed [KEP-3203] [WIP - Very Early to Review] Fetch and Render CVE JSON feed Jul 21, 2022
@PushkarJ PushkarJ changed the title [KEP-3203] [WIP - Very Early to Review] Fetch and Render CVE JSON feed [KEP-3203] [WIP - Not Ready to Review] Fetch and Render CVE JSON feed Jul 21, 2022
@PushkarJ PushkarJ marked this pull request as ready for review July 21, 2022 19:13
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 21, 2022
@netlify
Copy link

netlify bot commented Jul 21, 2022

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit b1e8d8e
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62d9a28cdb345e00093a058a
😎 Deploy Preview https://deploy-preview-35228--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jul 21, 2022 •
edited
Loading

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit cafe6d2
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62e953f31034230008c457f9
😎 Deploy Preview https://deploy-preview-35228--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@PushkarJ PushkarJ force-pushed the kep-3203-k-website-updates branch 2 times, most recently from ea22fca to 2a2fdcc Compare July 21, 2022 20:48
@PushkarJ PushkarJ mentioned this pull request Jul 21, 2022
Closed
@PushkarJ PushkarJ changed the title [KEP-3203] [WIP - Not Ready to Review] Fetch and Render CVE JSON feed [KEP-3203] Fetch and Render CVE JSON feed Jul 21, 2022
@PushkarJ PushkarJ mentioned this pull request Jul 21, 2022
Open
12 tasks
sftim
sftim reviewed Jul 22, 2022
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

  • Paths within static that are actually generated should include generated in their path. A find and grep should show some existing paths that follow that convention.
  • I'd name gen-cve-feed.sh as fetch-cve-feed.sh

❓ How do we make sure that people don't accidentally commit that fetched file?

❓ For a container build, shouldn't the fetch happen inside the container?

sftim
sftim reviewed Jul 24, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
sftim
sftim reviewed Jul 24, 2022
View reviewed changes
layouts/shortcodes/issues-security.html Outdated Show resolved Hide resolved
layouts/shortcodes/issues-security.html Outdated Show resolved Hide resolved
@sftim
Copy link
Contributor

sftim commented Aug 1, 2022

Rendered page LGTM

@sftim
Copy link
Contributor

sftim commented Aug 2, 2022

Should this PR target the dev-1.25 branch?

(I'm not sure - do we want to add the feed with the v1.25 release, or perhaps actually combine the blog announcement for the new feed with the switch-on)

We have options, and several of them are good. I want to be clear on which option we're picking so I can based my approval decision on that context.

@PushkarJ @nehaLohia27 @sftim
Fetch and Render CVE JSON feed
cafe6d2 
- Pull JSON blob from queried issues
- Use layout output formats + templates to generate HTML table and JSON blob
- Add localized strings and caption for CVE feed
- Add a new page to describe details about CVE feed and how to use it
- Update existing pages and link the official CVE feed from it

Co-authored-by: Neha Lohia <nehapithadiya444@gmail.com>
Co-authored-by: Tim Bannister <tim@scalefactory.com>
@PushkarJ PushkarJ changed the base branch from main to dev-1.25 August 2, 2022 16:42
@PushkarJ
Copy link
Member Author

PushkarJ commented Aug 2, 2022

@sftim I think the #35608 blog being published along with the CVE feed being available by v1.25 sounds like a good idea to me. I have switched target branch to dev-1.25 now.

@sftim
Copy link
Contributor

sftim commented Aug 3, 2022

@sftim I think the #35608 blog being published along with the CVE feed being available by v1.25 sounds like a good idea to me

I'm not sure I understand. Which of these is right:

  • the feed goes live on v1.25 release day
  • the feed goes live after v1.25 release day, simultaneous with the blog announcement about the new feed going live
  • something else!

?

@PushkarJ
Copy link
Member Author

PushkarJ commented Aug 3, 2022

What I meant was, the feed goes live on v1.25 release day, the feature blog is followed soon after (in next few days). This will allow the feed to have some soak time to iron out any issues we find after going live :)

@sftim
Copy link
Contributor

sftim commented Aug 3, 2022

What I meant was, the feed goes live on v1.25 release day, the feature blog is followed soon after (in next few days). This will allow the feed to have some soak time to iron out any issues we find after going live :)

Sounds good to me.

/approve
/remove-label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot removed the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 3, 2022
@sftim
Copy link
Contributor

sftim commented Aug 3, 2022

(I'd like somebody else to add the LGTM here)

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nehaLohia27, sftim

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 Aug 3, 2022
@PushkarJ
Copy link
Member Author

PushkarJ commented Aug 3, 2022

/cc @nehaLohia27 @savitharaghunathan

(Tagging you both for /lgtm and/or review comments)

@reylejano
Copy link
Member

/milestone 1.25
/assign @kcmartin

@k8s-ci-robot k8s-ci-robot added this to the 1.25 milestone Aug 3, 2022
@PushkarJ PushkarJ mentioned this pull request Aug 5, 2022
Closed
@reylejano
Copy link
Member

/lgtm

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

LGTM label has been added.

Git tree hash: 7581df102046b8f3ddd1be864ad4b2ad7561bd2b

@k8s-ci-robot k8s-ci-robot merged commit c465685 into kubernetes:dev-1.25 Aug 8, 2022
@katcosgrove
Copy link
Contributor

Hi from the Comms team! We have you tracked for a feature blog, but the link provided to us was a KEP, not a placeholder blog PR. Are you still intending to have a feature blog for this release?

@sftim
Copy link
Contributor

sftim commented Aug 15, 2022

Announcement blog article will be added via #35608

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

@sftim sftim sftim left review comments

@daminisatya daminisatya Awaiting requested review from daminisatya

@mehabhalodiya mehabhalodiya Awaiting requested review from mehabhalodiya

@kbhawkey kbhawkey Awaiting requested review from kbhawkey

@nehaLohia27 nehaLohia27 Awaiting requested review from nehaLohia27

@savitharaghunathan savitharaghunathan Awaiting requested review from savitharaghunathan

Assignees

@kcmartin kcmartin

@reylejano reylejano

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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/security Categorizes an issue or PR as relevant to SIG Security. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
1.25
Development

Successfully merging this pull request may close these issues.
7 participants
@PushkarJ @sftim @k8s-ci-robot @reylejano @katcosgrove @nehaLohia27 @kcmartin