Run the link check for PRs and changes to master #1167

wallrj · 2023-01-19T12:31:57Z

There exists an npm run markdown-link-check script, which checks both internal markdown links and external links.

I've enabled it in CI and fixed a bunch of the broken internal links that it identified.

I've disabled the anchor checking because it is not reliable, due to:

anchor link checking is broken tcort/markdown-link-check#225

Also disabled external link checks because sites like cloudflare seem to block robot http clients.
And also because there are some broken links in the cert-manager API.

Example failures:

https://github.com/cert-manager/website/actions/runs/3958447700/jobs/6780031828

References:

markdown-link-check

Signed-off-by: Richard Wall <richard.wall@jetstack.io>

netlify · 2023-01-19T12:38:42Z

✅ Deploy Preview for cert-manager-website ready!

Name	Link
🔨 Latest commit	`e521164`
🔍 Latest deploy log	https://app.netlify.com/sites/cert-manager-website/deploys/63c9541e9b525e0007f1309f
😎 Deploy Preview	https://deploy-preview-1167--cert-manager-website.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 settings.

Signed-off-by: Richard Wall <richard.wall@jetstack.io>

Checking external links is flaky due to network and rate limiting Checking anchor links doesn't work when the anchors have been specified using `<a name` syntax. Checking /foo/bar/baz.png links doesn't work because they are served from a virtual webserver directory mapped to public/ Signed-off-by: Richard Wall <richard.wall@jetstack.io>

wallrj · 2023-01-19T14:35:20Z

README.md

+> ℹ️ All these checks are also run automatically for pull requests.
+> The results will be reported in the [checks summary](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks) at the bottom of your GitHub PR.
+> Read the [cert-manager-website-presubmits.yaml prow configuration file](https://github.com/jetstack/testing/blob/master/config/jobs/cert-manager/website/cert-manager-website-presubmits.yaml) and the [check.yaml workflow file](.github/workflows/check.yaml) for more details.
+


Added a couple of paragraphs about these npm scripts and a note to explain how they are run by CI.

wallrj · 2023-01-19T14:36:49Z

content/docs/projects/approver-policy.md

@@ -4,19 +4,19 @@ description: ''
 ---

 approver-policy is a cert-manager
-[approver](https://cert-manager.io/docs/concepts/certificaterequest/#approval)
+[approver](../concepts/certificaterequest.md#approval)


There were a bunch of absolute links which I converted to internal links. Probably because this content was copy - pasted from the README file of the project.

wallrj · 2023-01-19T14:37:47Z

content/docs/projects/approver-policy.md

@@ -269,7 +269,7 @@ CertificateRequestPolicies against CertificateRequests for evaluation.  A
 CertificateRequestPolicy must select, and therefore match, a CertificateRequest
 for it to be considered for evaluation of the request.

-> ⚠️ Note that the user must still be bound by [RBAC](#Configuration) for
+> ⚠️ Note that the user must still be bound by [RBAC](#configuration) for


The link checker picked this up before I disabled the #anchor checks.
It seems that the autogenerated markdown anchors are always lower case.

wallrj · 2023-01-19T14:38:56Z

content/docs/release-notes/release-notes-1.0.md

@@ -145,12 +145,12 @@ For users of Kubernetes `v1.15` we keep offering support for the `v1beta1` Kuber
 For this release we upgraded our logging library to `klog/v2` analog to Kubernetes `v1.19`.
 We also reviewed every log we write to assign it an appropriate log level.

-We followed the (Kubernetes logging guidelines)[https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md]. To come up with 5 log levels ranging from `Error` (level 0) which only prints important errors to `Trace` (level 5) which can help you to know exactly what is gong on.
+We followed the [Kubernetes logging guidelines](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md). To come up with 5 log levels ranging from `Error` (level 0) which only prints important errors to `Trace` (level 5) which can help you to know exactly what is gong on.


This link was marked up incorrectly.

wallrj · 2023-01-19T14:41:30Z

markdown-link-check.json

-      "pattern": "^https://developers.cloudflare.com"
+      "description": "Ignore external links which may fail due to network flakiness and rate limits on foreign servers",
+      "pattern": "^https?://"
+    },


For example

https://github.com/cert-manager/website/actions/runs/3958447700/jobs/6780031828

FILE: content/docs/tutorials/getting-started-aks-letsencrypt/README.md [✖] https://www.cloudflare.com/en-gb/learning/dns/glossary/what-is-a-domain-name-registrar/ [✖] https://www.cloudflare.com/en-gb/learning/dns/dns-server-types/ [✖] clusterissuer-selfsigned.yaml [✖] certificate.yaml ERROR: 11 dead links found! [✖] deployment.yaml [✖] service.yaml [✖] https://www.cloudflare.com/en-gb/learning/dns/dns-records/dns-cname-record/ [✖] ../../configuration/acme/dns01/azuredns/README.md [✖] values.yaml [✖] clusterissuer-lets-encrypt-staging.yaml [✖] clusterissuer-lets-encrypt-production.yaml 34 links checked. [✖] https://www.cloudflare.com/en-gb/learning/dns/glossary/what-is-a-domain-name-registrar/ → Status: 403 [✖] https://www.cloudflare.com/en-gb/learning/dns/dns-server-types/ → Status: 403 [✖] clusterissuer-selfsigned.yaml → Status: 400

wallrj · 2023-01-19T14:43:26Z

markdown-link-check.json

@@ -1,13 +1,21 @@
 {
  "ignorePatterns": [
    {
-      "pattern": "^https://github.com"
+      "description": "Ignore root relative links to images which markdown-link-check is unable to verify",
+      "pattern": "^/"


For example:

https://github.com/cert-manager/website/actions/runs/3958447700/jobs/6780031828

FILE: content/docs/troubleshooting/webhook.md [✖] https://web.archive.org/web/20210903183221/https://serverfault.com/questions/1076563/creating-issuer-for-kubernetes-cert-manager-is-causing-404-and-500-error [✖] #io-timeout [✖] #context-deadline-exceeded [✖] #gke-private-cluster [✖] #diagram [✖] /images/troubleshooting/webhook-pod-networking-diagram.png 48 links checked. ERROR: 6 dead links found! [✖] https://web.archive.org/web/20210903183221/https://serverfault.com/questions/1076563/creating-issuer-for-kubernetes-cert-manager-is-causing-404-and-500-error → Status: 0 [✖] #io-timeout → Status: 404 [✖] #context-deadline-exceeded → Status: 404 [✖] #gke-private-cluster → Status: 404 [✖] #diagram → Status: 404 [✖] /images/troubleshooting/webhook-pod-networking-diagram.png → Status: 400

SgtCoDFish

/lgtm
/approve

I added suggestions, but I'm not even gonna add a hold. I'm generally super concerned about developing two parallel CI pipelines, but I don't want to get in the way of progress.

SgtCoDFish · 2023-01-19T14:43:48Z

.github/workflows/check.yaml

+  link-check:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 16
+          cache: npm
+      - run: npm ci
+      - run: npm run markdown-link-check


suggestion (non-blocking): I think proliferating a pattern where some jobs are run in github actions and some are run in prow will lead to everything being harder to maintain in the future.

I'd rather not use github actions at all, personally, but more important than "actions vs prow" is consistency.

I don't think it's a blocker, because I don't want to get in the way of a good PR... but I think it's a concern.

SgtCoDFish · 2023-01-19T14:47:58Z

README.md

+
+```bash
+npm run lint
+```
+
+To check the links in all pages, run [markdown-link-check](https://github.com/tcort/markdown-link-check):
+
+```bash
+npm run markdown-link-check
+```
+
+> ℹ️ All these checks are also run automatically for pull requests.
+> The results will be reported in the [checks summary](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks) at the bottom of your GitHub PR.
+> Read the [cert-manager-website-presubmits.yaml prow configuration file](https://github.com/jetstack/testing/blob/master/config/jobs/cert-manager/website/cert-manager-website-presubmits.yaml) and the [check.yaml workflow file](.github/workflows/check.yaml) for more details.
+


suggestion (non-blocking): It would be nice to have a single script which can be run to check everything locally, and to have the CI environment call that script. Doesn't really matter though

markdown-link-check.json

jetstack-bot · 2023-01-19T14:50:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish, wallrj

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:

~~OWNERS~~ [SgtCoDFish,wallrj]

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

wallrj added 2 commits January 19, 2023 12:30

Run link check via a GitHub Actions job

7d7c4fd

Signed-off-by: Richard Wall <richard.wall@jetstack.io>

Document the automated checks

8357ebc

Signed-off-by: Richard Wall <richard.wall@jetstack.io>

jetstack-bot 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 Jan 19, 2023

wallrj added 2 commits January 19, 2023 14:27

Fix broken links

b18307b

Signed-off-by: Richard Wall <richard.wall@jetstack.io>

wallrj force-pushed the link-check branch from 3c9fa14 to e521164 Compare January 19, 2023 14:30

jetstack-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 19, 2023

wallrj changed the title ~~WIP: Run the link check for PRs and changes to master~~ Run the link check for PRs and changes to master Jan 19, 2023

jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 19, 2023

wallrj requested a review from SgtCoDFish January 19, 2023 14:34

wallrj commented Jan 19, 2023

View reviewed changes

SgtCoDFish approved these changes Jan 19, 2023

View reviewed changes

jetstack-bot assigned SgtCoDFish Jan 19, 2023

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

jetstack-bot merged commit 99a5d62 into cert-manager:master Jan 19, 2023

wallrj deleted the link-check branch January 19, 2023 15:42

This was referenced Feb 20, 2023

[release-next] Fast-forward to master #1189

Merged

Vault: document the new field "serviceAccountRef" #1081

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run the link check for PRs and changes to master #1167

Run the link check for PRs and changes to master #1167

wallrj commented Jan 19, 2023 •

edited

Loading

netlify bot commented Jan 19, 2023 •

edited

Loading

wallrj Jan 19, 2023

wallrj Jan 19, 2023

wallrj Jan 19, 2023

wallrj Jan 19, 2023

wallrj Jan 19, 2023

wallrj Jan 19, 2023

SgtCoDFish left a comment

SgtCoDFish Jan 19, 2023

SgtCoDFish Jan 19, 2023

jetstack-bot commented Jan 19, 2023

Run the link check for PRs and changes to master #1167

Run the link check for PRs and changes to master #1167

Conversation

wallrj commented Jan 19, 2023 • edited Loading

netlify bot commented Jan 19, 2023 • edited Loading

✅ Deploy Preview for cert-manager-website ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SgtCoDFish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetstack-bot commented Jan 19, 2023

wallrj commented Jan 19, 2023 •

edited

Loading

netlify bot commented Jan 19, 2023 •

edited

Loading