fix: Fix offline build errors in CVE feed #45016

sergeyshevch · 2024-02-04T17:03:54Z

Hello! This PR addresses issue #44223. I added checks for nil in getJson shortcode usage and now the website can be built without internet access if all required images already exist on the machine.

How to check:

run make container-serve locally
wait until build will be completed and stop it (you will pull all required images)
disable internet access on your machine
run make container-serve again. It should be completed with warnings about CVE feed missing

netlify · 2024-02-04T17:08:08Z

✅ Pull request preview available for checking

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`58455df`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/65f9eb2331b92f00089bce10
😎 Deploy Preview	https://deploy-preview-45016--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

sftim · 2024-02-04T21:53:38Z

/area web-development

sftim

Thanks.

One important thing to change: for production builds, let's make sure we never skip building the CVE feed.

layouts/_default/cve-feed.rss.xml

layouts/shortcodes/cve-feed.html

sergeyshevch · 2024-02-10T17:11:32Z

@sftim Thanks for your commits. I applied suggested changes

Review was stale

hugo.toml

sftim · 2024-02-14T13:21:36Z

This helps with #42162

dipesh-rawat

The changes look good based on my testing. However, it's important to make similar changes in release-binaries.htmland cve-feed.json, as highlighted in the earlier review feedback (here).

dipesh-rawat · 2024-03-18T01:18:46Z

layouts/shortcodes/cve-feed.html

 {{ if ne $feed.version "https://jsonfeed.org/version/1.1" }}
  {{ warnf "CVE feed shortcode. KEP-3203: CVE feed does not comply with JSON feed v1.1." }}
 {{ end }}
 <table class="security-cves">
+  <!-- Handle case if CVE feed is not available. For example for offline build -->
+  {{ if ne $feed nil }}


Optional improvement: Instead of just displaying an empty table with headers, we could enhance the user experience by showing a helpful message, perhaps in a note, indicating that the offline preview does not support the CVE Feed.

There is another way.

If we change the fetch to use https://gohugo.io/functions/resources/getremote/, then we can attempt to fetch offline and warn at build time. And, yes, we can substitute something in.

For a production build, we should error out instead of warning.

User will receive a warning message in build log. I guess that build should works offline if some of resources are unavailable.

Yes we won't display some content that require remote files but other content will be ok and editable

Co-authored-by: Tim Bannister <tim@scalefactory.com>

k8s-ci-robot · 2024-03-19T19:44:51Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign reylejano for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

sergeyshevch · 2024-03-19T19:46:04Z

I rebased this PR on actual main and addressed all comments above

sftim · 2024-03-20T23:52:39Z

layouts/_default/cve-feed.rss.xml

@@ -1,4 +1,11 @@
 {{ $feed := getJSON .Site.Params.cveFeedBucket -}}


Really, we should switch this to use transform.GetRemote; see https://gohugo.io/functions/resources/getremote/#remote-data for an example that includes error handling. Then I don't think we need to edit the hugo.toml to ignore error-remote-getjson.

(If we switch, we can still change the error handling based on which environment we're in)

There's already an existing PR #45375 that focuses on enhancing data retrieval by replacing 'getJSON' with 'transform.Unmarshal'. Perhaps we could consider combining these changes into a single PR.

(EDIT: I have updated the PR-45375 to include the error handling based on environment.)

sftim · 2024-03-20T23:54:34Z

layouts/shortcodes/cve-feed.html

@@ -1,9 +1,19 @@
 {{ $feed := getJSON .Site.Params.cveFeedBucket }}


Really, we should switch this to use transform.GetRemote; see https://gohugo.io/functions/resources/getremote/#remote-data for an example that includes error handling. Then I don't think we need to edit the hugo.toml to ignore error-remote-getjson.

(If we switch, we can still change the error handling based on which environment we're in)

sftim · 2024-03-20T23:54:40Z

layouts/shortcodes/release-binaries.html

@@ -1,5 +1,13 @@
 <!-- Fetch release_binaries.json from kubernetes-sigs/downloadkubernetes to render in table -->
 {{ $response := getJSON "https://raw.githubusercontent.com/kubernetes-sigs/downloadkubernetes/master/dist/release_binaries.json" }}


Really, we should switch this to use transform.GetRemote; see https://gohugo.io/functions/resources/getremote/#remote-data for an example that includes error handling. Then I don't think we need to edit the hugo.toml to ignore error-remote-getjson.

(If we switch, we can still change the error handling based on which environment we're in)

sftim · 2024-03-20T23:55:43Z

I'd prefer to see this PR switch to using transform.GetRemote, leaving the existing hugo.toml intact.

sergeyshevch · 2024-03-25T17:13:35Z

@dipesh-rawat Looks like that #45375 fully include this PR fix. Maybe we should close this one then? I'm totally ok with that

k8s-ci-robot · 2024-03-25T23:36:36Z

PR needs rebase.

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/test-infra repository.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 4, 2024

k8s-ci-robot requested a review from mengjiao-liu February 4, 2024 17:04

k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Feb 4, 2024

k8s-ci-robot requested a review from shannonxtreme February 4, 2024 17:04

k8s-ci-robot added the area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes label Feb 4, 2024

sftim previously requested changes Feb 4, 2024

View reviewed changes

layouts/_default/cve-feed.rss.xml Outdated Show resolved Hide resolved

layouts/shortcodes/cve-feed.html Outdated Show resolved Hide resolved

sftim reviewed Feb 14, 2024

View reviewed changes

hugo.toml Show resolved Hide resolved

dipesh-rawat reviewed Mar 18, 2024

View reviewed changes

sergeyshevch and others added 4 commits March 19, 2024 21:41

fix: Fix offline build errors in CVE feed

37e6fd7

fix: Return error on production build without cveFeed

171fd7b

Co-authored-by: Tim Bannister <tim@scalefactory.com>

fix: Return error on production build without cveFeed

303f9a0

Co-authored-by: Tim Bannister <tim@scalefactory.com>

feat: Update getJSON usages

58455df

sergeyshevch force-pushed the fix/offline-build-errors branch from 9043182 to 58455df Compare March 19, 2024 19:44

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 19, 2024

sergeyshevch requested review from sftim and dipesh-rawat March 19, 2024 19:46

sftim reviewed Mar 20, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2024

sergeyshevch closed this Apr 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix offline build errors in CVE feed #45016

fix: Fix offline build errors in CVE feed #45016

sergeyshevch commented Feb 4, 2024 •

edited

Loading

netlify bot commented Feb 4, 2024 •

edited

Loading

sftim commented Feb 4, 2024

sftim left a comment

sergeyshevch commented Feb 10, 2024

sftim commented Feb 14, 2024

dipesh-rawat left a comment

dipesh-rawat Mar 18, 2024

sftim Mar 18, 2024

sergeyshevch Mar 19, 2024

k8s-ci-robot commented Mar 19, 2024

sergeyshevch commented Mar 19, 2024

sftim Mar 20, 2024

dipesh-rawat Mar 21, 2024 •

edited

Loading

sftim Mar 20, 2024

sftim Mar 20, 2024

sftim commented Mar 20, 2024

sergeyshevch commented Mar 25, 2024 •

edited

Loading

k8s-ci-robot commented Mar 25, 2024

		@@ -1,4 +1,11 @@
		{{ $feed := getJSON .Site.Params.cveFeedBucket -}}

		@@ -1,9 +1,19 @@
		{{ $feed := getJSON .Site.Params.cveFeedBucket }}

		@@ -1,5 +1,13 @@
		<!-- Fetch release_binaries.json from kubernetes-sigs/downloadkubernetes to render in table -->
		{{ $response := getJSON "https://raw.githubusercontent.com/kubernetes-sigs/downloadkubernetes/master/dist/release_binaries.json" }}

fix: Fix offline build errors in CVE feed #45016

fix: Fix offline build errors in CVE feed #45016

Conversation

sergeyshevch commented Feb 4, 2024 • edited Loading

netlify bot commented Feb 4, 2024 • edited Loading

✅ Pull request preview available for checking

sftim commented Feb 4, 2024

sftim left a comment

Choose a reason for hiding this comment

sergeyshevch commented Feb 10, 2024

sftim commented Feb 14, 2024

dipesh-rawat left a comment

Choose a reason for hiding this comment

dipesh-rawat Mar 18, 2024

Choose a reason for hiding this comment

sftim Mar 18, 2024

Choose a reason for hiding this comment

sergeyshevch Mar 19, 2024

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 19, 2024

sergeyshevch commented Mar 19, 2024

sftim Mar 20, 2024

Choose a reason for hiding this comment

dipesh-rawat Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

sftim Mar 20, 2024

Choose a reason for hiding this comment

sftim Mar 20, 2024

Choose a reason for hiding this comment

sftim commented Mar 20, 2024

sergeyshevch commented Mar 25, 2024 • edited Loading

k8s-ci-robot commented Mar 25, 2024

sergeyshevch commented Feb 4, 2024 •

edited

Loading

netlify bot commented Feb 4, 2024 •

edited

Loading

dipesh-rawat Mar 21, 2024 •

edited

Loading

sergeyshevch commented Mar 25, 2024 •

edited

Loading