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

Broken links docs fix #2134

Merged
merged 18 commits into from
Nov 4, 2020
Merged

Conversation

antgamdia
Copy link
Contributor

Description of the change

Across the years, some external links have changed (e.g., v2 Helm docs, oauth2-proxy was renamed, Harbor moved docs from GitHub, etc.). Besides, a bunch of relative links was pointing to nowhere.
This PR intents to keep up to date the Kubeapps documentation links.

Benefits

The broken links from the Kubeapps GitHub docs should not exist (al least for, hopefully, a while).

Possible drawbacks

If any external action is automatically using this docs source in order to generate an external documentation portal, this PR could affect it. Please, check it before merging.

Applicable issues

No issue

Additional information

During this process, I've noticed that contacting the project team belongs to a former member. I didn't take further action, but I suppose it should be changed at some point.

https://github.com/kubeapps/kubeapps/blob/master/CONTRIBUTING.md#enforcement

Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere
Some relative links were wrongly pointing to nowhere. In this case, pusher/oauth2_proxy but has been renamed as of 29/03/2020 to oauth2-proxy/oauth2-proxy, so the references here have been changed accordingly.
Some relative links were wrongly pointing to nowhere. The Harbor repo has moved their docs; consequently, references here have been changed accordingly.
Some relative links were wrongly pointing to nowhere. Here, a k8s link was also broken
Some relative links were wrongly pointing to nowhere. Here, a k8s link was also broken
A k8s link was broken. Minikube also misspelled
Removed  service-catalog.md link
Update the current index of proposals
Some relative links were wrongly pointing to nowhere. Here, a Helm link was also broken
- Secure authentication to Kubeapps using an [OAuth2/OIDC provider](https://github.com/kubeapps/kubeapps/blob/master/docs/user/using-an-OIDC-provider.md)
- Secure authorization based on Kubernetes [Role-Based Access Control](https://github.com/kubeapps/kubeapps/blob/master/docs/user/access-control.md)
- Secure authentication to Kubeapps using an [OAuth2/OIDC provider](../../docs/user/using-an-OIDC-provider.md)
- Secure authorization based on Kubernetes [Role-Based Access Control](../../docs/user/access-control.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these can be relative links, as the chart readme is displayed on non-GH pages, such as https://hub.kubeapps.com/charts/bitnami/kubeapps ? A relative link there would fail, I think?

Copy link
Contributor

@andresmgot andresmgot Nov 3, 2020

Choose a reason for hiding this comment

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

Yes, this file will be rendered in external places (hub.kubeapps.com, artifacts-hub, kubeapps itself) so it should not contain relative links.

@@ -17,7 +17,7 @@ Two of the most common authentication strategies for providing a token identifyi

### OpenID Connect authentication

The most common and secure authentication for users to authenticate with the cluster (and therefore Kubeapps) is to use the built-in Kubernetes support for OpenID Connect. In this setup your clusters trust an OAuth2 provider such as Azure Active Directory, Google OpenID Connect or your own installation of the Dex auth provider. You can read more about [using an OIDC provider with Kubeapps](../using-an-OIDC-provider.md).
The most common and secure authentication for users to authenticate with the cluster (and therefore Kubeapps) is to use the built-in Kubernetes support for OpenID Connect. In this setup your clusters trust an OAuth2 provider such as Azure Active Directory, Google OpenID Connect or your own installation of the Dex auth provider. You can read more about [using an OIDC provider with Kubeapps](./using-an-OIDC-provider.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, so my guess is that @andresmgot updated this so it works on the new docs site, but you're right, it needs to be able to work in the GH docs also - I assumed that with the change for the docs site it worked for both already.

From a brief look, the issue is that on the docs site, this page (access-control.md) is presented with a nice clean URL: https://kubeapps.com/docs/access-control/ but this requires the ../ to get back to /docs/. We need the links to work on both - it may be that an easy option is to not use pretty URLs on the docs site (ie. if it was /docs/access-control without the slash, then ./using-an-OIDC-provider.md would translate, I think? Or perhaps there's a better way with the docs config. Let's see what @andresmgot thinks - but I'd avoid landing this change until we find out his intention.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the thing is that the trailing slash / cannot be removed (or at least I am not aware of a way of doing that). In any case, we already have a hook to replace links so we can just do ./ --> ../ when rendering the docs in kubeapps.com.

Summary: this change is fine, sorry for not noticing when working on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

so we can just do ./ --> ../ when rendering the docs in kubeapps.com

Maybe, though it'd be better if links just worked between GH and the docs site. I don't remember, but think you mentioned that you chose middleman for the docs site? Does this help? https://gist.github.com/mxhold/00b076c996d6d74401c1

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, will try that as well

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing all the links!

- Secure authentication to Kubeapps using an [OAuth2/OIDC provider](https://github.com/kubeapps/kubeapps/blob/master/docs/user/using-an-OIDC-provider.md)
- Secure authorization based on Kubernetes [Role-Based Access Control](https://github.com/kubeapps/kubeapps/blob/master/docs/user/access-control.md)
- Secure authentication to Kubeapps using an [OAuth2/OIDC provider](../../docs/user/using-an-OIDC-provider.md)
- Secure authorization based on Kubernetes [Role-Based Access Control](../../docs/user/access-control.md)
Copy link
Contributor

@andresmgot andresmgot Nov 3, 2020

Choose a reason for hiding this comment

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

Yes, this file will be rendered in external places (hub.kubeapps.com, artifacts-hub, kubeapps itself) so it should not contain relative links.

@@ -17,7 +17,7 @@ Two of the most common authentication strategies for providing a token identifyi

### OpenID Connect authentication

The most common and secure authentication for users to authenticate with the cluster (and therefore Kubeapps) is to use the built-in Kubernetes support for OpenID Connect. In this setup your clusters trust an OAuth2 provider such as Azure Active Directory, Google OpenID Connect or your own installation of the Dex auth provider. You can read more about [using an OIDC provider with Kubeapps](../using-an-OIDC-provider.md).
The most common and secure authentication for users to authenticate with the cluster (and therefore Kubeapps) is to use the built-in Kubernetes support for OpenID Connect. In this setup your clusters trust an OAuth2 provider such as Azure Active Directory, Google OpenID Connect or your own installation of the Dex auth provider. You can read more about [using an OIDC provider with Kubeapps](./using-an-OIDC-provider.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the thing is that the trailing slash / cannot be removed (or at least I am not aware of a way of doing that). In any case, we already have a hook to replace links so we can just do ./ --> ../ when rendering the docs in kubeapps.com.

Summary: this change is fine, sorry for not noticing when working on this.

@@ -78,7 +78,8 @@ Paste the token generated in the previous step to authenticate and access the Ku

![Dashboard main page](../img/dashboard-home.png)

***Note:*** If you are setting up Kubeapps for other people to access, you will want to use a different service type or setup Ingress rather than using the above `kubectl port-forward`. For detailed information on installing, configuring and upgrading Kubeapps, checkout the [chart README](https://github.com/kubeapps/kubeapps/tree/master/chart/kubeapps).
***Note:*** If you are setting up Kubeapps for other people to access, you will want to use a different service type or setup Ingress rather than using the above `kubectl port-forward`. For detailed information on installing, configuring and upgrading Kubeapps, checkout the [chart README](../../chart/kubeapps/README.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that in kubeapps.com/docs we are only rendering the directy docs/user so no relative link outside that directory should be used. In this case we need the URL to the chart readme.

- [Detailed installation instructions](https://github.com/kubeapps/kubeapps/tree/master/chart/kubeapps)
- [Deploying Operators](../operators.md)
- [Kubeapps Dashboard documentation](../dashboard.md)
- [Detailed installation instructions](../../chart/kubeapps/README.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

same about the chart readme

Since this file is being rendered outside github.com it should not contain any relative links.
Minor typos fix.
Since docs/user is being rendered in other places rather than github.com, links pointing outside /docs must be absolutes
To avoid an unnecessary redirection, /tree links should only point to 'directories' not specific files.
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Thanks for all the fixes

@antgamdia antgamdia merged commit 1c3d387 into vmware-tanzu:master Nov 4, 2020
@antgamdia antgamdia deleted the brokenLinksDocsFix branch November 4, 2020 14:46
@antgamdia antgamdia mentioned this pull request Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants