- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15.1k
Update CVE feed layouts for new JSON feed format #38579
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,23 +1 @@ | ||
| { | ||
| "version": "https://jsonfeed.org/version/1.1", | ||
| "title": "Auto-refreshing Official CVE Feed", | ||
| "home_page_url": "https://kubernetes.io", | ||
| "feed_url": "https://kubernetes.io/docs/reference/issues-security/official-cve-feed/index.json", | ||
| "description": "Auto-refreshing official CVE feed for Kubernetes repository", | ||
| "authors": [ | ||
| { | ||
| "name": "Kubernetes Community", | ||
| "url": "https://www.kubernetes.dev" | ||
| } | ||
| ], | ||
| "items": [ | ||
| {{ range $i, $e := getJSON .Site.Params.cveFeedBucket }} | ||
| {{ if $i }}, {{ end }} | ||
| { | ||
| {{ T "cve_json_id" | jsonify }}: {{ .cve_id | jsonify }}, | ||
| {{ T "cve_json_url" | jsonify }}: {{ .issue_url | jsonify }}, | ||
| {{ T "cve_json_external_url" | jsonify }}: {{ .cve_url | jsonify}}, | ||
| {{ T "cve_json_summary" | jsonify }}: {{ replace (.summary | jsonify ) "\\u003e" ">" }} | ||
| }{{ end }} | ||
| ] | ||
| } | ||
| {{ getJSON .Site.Params.cveFeedBucket | jsonify }} | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,19 +1,23 @@ | ||
| {{ $feed := getJSON .Site.Params.cveFeedBucket }} | ||
| {{ if ne $feed.version "https://jsonfeed.org/version/1.1" }} | ||
| {{ errorf "Build Failed. CVE feed does not comply with JSON feed v1.1" }} | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be better if the build did not fail, but instead use a 'placeholder page' and send an alert (or something like that) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok makes sense. This is a bit in contradiction with this previous review #38579 (review), @sftim do you have an opinion on that as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like a nice enhancement; however, I would save that for a follow-up PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep we'll look at this here #39513 next! | ||
| {{ end }} | ||
| <table class="security-cves"> | ||
| <caption>{{ T "cve_table" }}</caption> | ||
| <caption style="caption-side: top;">{{ T "cve_table" }} {{ T "cve_table_date_before" }}{{ $feed._kubernetes_io.updated_at | time.Format ( T "cve_table_date_format" ) }}{{ T "cve_table_date_after" }}</caption> | ||
| <thead> | ||
| <tr> | ||
| <th>{{ T "cve_id" }}</th> | ||
| <th>{{ T "cve_summary"}}</th> | ||
| <th>{{ T "cve_summary" }}</th> | ||
| <th>{{ T "cve_issue_url" }}</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {{ range $issues := getJSON .Site.Params.cveFeedBucket }} | ||
| {{ range $feed.items }} | ||
| <tr> | ||
| <td><a href="{{ .cve_url }}">{{ .cve_id | htmlEscape | safeHTML }}</a></td> | ||
| <td><a href="{{ .url }}">{{ .id | htmlEscape | safeHTML }}</a></td> | ||
| <td>{{ .summary | htmlEscape | safeHTML }}</td> | ||
| <td><a href="{{ .issue_url }}">#{{ .number }}</a></td> | ||
| <td><a href="{{ .url }}">#{{ ._kubernetes_io.issue_number }}</a></td> | ||
| </tr> | ||
| {{ end }} | ||
| </tbody> | ||
| </table> | ||
| </table> | ||
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.
This is a bit unusual
I'll look at how you are constructing the string
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 was thinking on the contrary that it was how we usually do these things, but we can do whatever you think is better.
website/data/i18n/en/en.toml
Lines 275 to 279 in 206231d
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 prefer to have the localized string be a format string:
and then use a string formatter to include the localized date. That's equivalent, but the placeholder makes it easier for localization teams to understand.
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'll update this in the follow-up PR #39513 :)