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

Doc-Builder: Add docs.github.com url to ignore list #3154

Closed
wants to merge 2 commits into from

Conversation

praveenkumar
Copy link
Member

This pr try to solve following issue by adding docs.github.com
to ignore list.

FILE: ./source/topics/proc_troubleshooting-unknown-issues.adoc
[✓] https://github.com/code-ready/crc/issues
[✓] https://github.com/code-ready/crc/issues/new
[heavy_multiplication_x] https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/attaching-files

3 links checked.

ERROR: 1 dead links found!
[heavy_multiplication_x] https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/attaching-files → Status: 403
make: *** [Makefile:138: docs_check_links] Error 123
$ curl -I https://docs.github.com/
HTTP/2 403
x-azure-ref: 0fzhqYgAAAAA4TNP/TuUcR6h8UmZoI8OxRFhCMzBFREdFMDIxNQA1OTZkNzhhMi1jYTVmLTQ3OWQtYmNkYy0wODM1ODMzMTc0YjI=
accept-ranges: bytes
date: Thu, 28 Apr 2022 06:47:27 GMT
via: 1.1 varnish
x-served-by: cache-maa10249-MAA
x-cache: MISS
x-cache-hits: 0
x-timer: S1651128448.577384,VS0,VE51
strict-transport-security: max-age=31557600

This pr try to solve following issue by adding docs.github.com
to ignore list.

```
FILE: ./source/topics/proc_troubleshooting-unknown-issues.adoc
[✓] https://github.com/code-ready/crc/issues
[✓] https://github.com/code-ready/crc/issues/new
[✖] https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/attaching-files

3 links checked.

ERROR: 1 dead links found!
[✖] https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/attaching-files → Status: 403
make: *** [Makefile:138: docs_check_links] Error 123
```

```
$ curl -I https://docs.github.com
HTTP/2 403
x-azure-ref: 0fzhqYgAAAAA4TNP/TuUcR6h8UmZoI8OxRFhCMzBFREdFMDIxNQA1OTZkNzhhMi1jYTVmLTQ3OWQtYmNkYy0wODM1ODMzMTc0YjI=
accept-ranges: bytes
date: Thu, 28 Apr 2022 06:47:27 GMT
via: 1.1 varnish
x-served-by: cache-maa10249-MAA
x-cache: MISS
x-cache-hits: 0
x-timer: S1651128448.577384,VS0,VE51
strict-transport-security: max-age=31557600
```

- github/docs#17358
@openshift-ci openshift-ci bot requested review from gbraad and kowen-rh April 28, 2022 06:51
@openshift-ci
Copy link

openshift-ci bot commented Apr 28, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anjannath

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

@cfergeau
Copy link
Contributor

If the help.github.com URL is redirected to docs.github.com, I don't think it should/would show up as a deadlink?
Ignoring this URL for deadlink checks is not great though, especially as it was moved (with redirect) once. Any idea why this is misdetected as being dead?

Copy link
Contributor

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

For what it's worth, I initially totally missed the github/docs#17358 link in the commit log. Since you made the change you did because of this issue, it's useful to explicitly say so at the beginning of the log
"When using curl to get https://...., we get a 403 error. This is a known issue, see github/docs#17358 . While this is being fixed, this commit disables the problematic URL to solve CI

$ curl https://docs.github.com
[test code/output showing which bug this is fixing]

@@ -2,6 +2,7 @@
"ignorePatterns": [
{ "pattern": "^https://api.crc.testing" },
{ "pattern": "^http://api.crc.testing" },
{ "pattern": "^https://docs.github.com/.*$" },
Copy link
Contributor

@cfergeau cfergeau Apr 28, 2022

Choose a reason for hiding this comment

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

Adding this (approximate syntax, did not test this) should avoid the issue

 "httpHeaders": [
    {
      "urls": ["https://docs.github.com"],
      "headers": {
        "Accept-Encoding": "gzip, deflate"
      }
    }
  ],

https://github.com/tcort/markdown-link-check#config-file-format

Copy link
Member Author

Choose a reason for hiding this comment

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

@cfergeau following patch doesn't solve this one.

diff --git a/images/docs-builder/links_ignorelist.json b/images/docs-builder/links_ignorelist.json
index ad6348ef..bcd3e13d 100644
--- a/images/docs-builder/links_ignorelist.json
+++ b/images/docs-builder/links_ignorelist.json
@@ -2,9 +2,15 @@
     "ignorePatterns": [
         { "pattern": "^https://api.crc.testing" },
         { "pattern": "^http://api.crc.testing" },
-        { "pattern": "^https://docs.github.com/.*$" },
         { "pattern": "https://console-openshift-console.apps-crc.testing" },
         { "pattern": "http://proxy.example.com" }
+    ],
+    "httpHeaders": [
+        {
+          "urls": ["https://docs.github.com"],
+          "headers": {
+            "Accept-Encoding": "gzip, deflate"
+          }
+        }
     ]
-
 }

I am also confused why it is not working because with curl it does work

$ curl -H "Accept-Encoding: gzip, deflate" -I https://docs.github.com  
HTTP/2 302 
cache-control: private, no-store
content-type: text/plain; charset=utf-8
location: /en

FYI I did updated the doc-builder image after this change to make sure it is part of that build image.

Copy link
Contributor

Choose a reason for hiding this comment

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

#3155 works locally for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cfergeau #3155 PR created against master branch instead of main branch.

@praveenkumar
Copy link
Member Author

If the help.github.com URL is redirected to docs.github.com, I don't think it should/would show up as a deadlink? Ignoring this URL for deadlink checks is not great though, especially as it was moved (with redirect) once. Any idea why this is misdetected as being dead?

@cfergeau I don't know why it misdected redirect to be dead for https://help.github.com but https://docs.github.com is dead link so I put 2 commits.

$ curl -I https://help.github.com/en/articles/file-attachments-on-issues-and-pull-requests
HTTP/2 301 
content-length: 0
location: https://docs.github.com/en/articles/file-attachments-on-issues-and-pull-requests

$ curl -I https://docs.github.com/en/articles/file-attachments-on-issues-and-pull-requests
HTTP/2 403 
x-azure-ref: 0GEJqYgAAAADZwqPngMDHRrlJUgq3S7p6RFhCMzBFREdFMDIxMwA1OTZkNzhhMi1jYTVmLTQ3OWQtYmNkYy0wODM1ODMzMTc0YjI=
accept-ranges: bytes
date: Thu, 28 Apr 2022 07:28:24 GMT
via: 1.1 varnish

@cfergeau
Copy link
Contributor

@cfergeau I don't know why it misdected redirect to be dead for https://help.github.com/ but https://docs.github.com/ is dead link

301 is the http code for redirect, hopefully the link checker picks that up, and it then reports that the redirected URL fails

Copy link
Contributor

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

See #3154 (comment) for an alternate solution (hopefully :)

@openshift-ci
Copy link

openshift-ci bot commented Apr 28, 2022

@praveenkumar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-crc bb15620 link true /test e2e-crc

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@praveenkumar
Copy link
Member Author

See #3154 (comment) for an alternate solution (hopefully :)

#3154 (comment)

@praveenkumar
Copy link
Member Author

Closing in favor of #3155

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants