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

Add "latest" version of API docs to provide more durable link targets #21931

Closed
wants to merge 1 commit into from

Conversation

joelsmith
Copy link
Contributor

This will allow external referrers to link to (for example)
https://kubernetes.io/docs/reference/generated/kubernetes-api/latest/#pod-v1-core
which might continue to work for many many versions instead
of the 5 or so that we support for current versions.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 20, 2020
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Jun 20, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign kbhawkey
You can assign the PR to them by writing /assign @kbhawkey in a comment when ready.

The full list of commands accepted by this bot can be found 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

@netlify
Copy link

netlify bot commented Jun 20, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit eb934ac

https://deploy-preview-21931--kubernetes-io-master-staging.netlify.app

@joelsmith
Copy link
Contributor Author

Hmm... it's working for the root page, but none of the assets. Looking at netlify redirect syntax to see if it supports wildcard-style matching.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2020
@joelsmith
Copy link
Contributor Author

Looks like it's working now. Try, for example, https://deploy-preview-21931--kubernetes-io-master-staging.netlify.app/docs/reference/generated/kubernetes-api/latest/#pod-v1-core
I'll squash it and it should be ready for review.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2020
This will allow external referrers to link to (for example)
https://kubernetes.io/docs/reference/generated/kubernetes-api/latest/#pod-v1-core
which might continue to work for many many versions instead
of the 5 or so that we support for current versions.
@@ -189,7 +189,8 @@
/docs/reference/generated/kubernetes-api/v1.15/ https://v1-15.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/ 301
/docs/reference/generated/kubernetes-api/v1.16/ https://v1-16.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/ 301
/docs/reference/generated/kubernetes-api/v1.17/ https://v1-17.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/ 301

# "latest" API version is a transparent redirect (i.e. rewrite) to the current version.
/docs/reference/generated/kubernetes-api/latest/* /docs/reference/generated/kubernetes-api/v1.18/:splat 200
Copy link
Contributor

Choose a reason for hiding this comment

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

The only problem as I see it ... we will need to revise this hard coded version number on every release, because latest is a relative term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true. I'm happy to automate it via the copyapi target in the Makefile in reference-docs, but I figured that somebody would be in this file adding an entry just above this. For example, when 1.19 comes out, someone will already be adding a 1.18 link right there, and could easily update the latest at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a way to keep it updated automatically. kubernetes-sigs/reference-docs#160

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @joelsmith .
I will read through the changes.
I am uncertain about using latest as well as adding onto the copyapi target to edit the _redirects file.
What is the advantage of providing a redirect from latest?
I realize that there will be updates per release to make sure that the links (that users previously depended on) are correct.

Copy link
Contributor Author

@joelsmith joelsmith Jun 20, 2020

Choose a reason for hiding this comment

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

Perhaps the user story would be a good place to start:

As a user, I would like to be able to create links to Kubernetes API object docs, and have those links continue to work for as long a time as possible. I am frustrated when I create a link, and then 15 months later, it stops working because the version of the API that I linked to is no longer supported. I would like my link to go to the current version of the API object I am interested in. If I had created a link to the latest version of the pod spec in the API docs on March 20, 2019 (15 months ago), that link would today be broken, since the latest version at the time was 1.13, which is no longer supported (and no longer hosted or redirected). (See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.13/#pod-v1-core for an example of such a broken link).

So my proposed PR (with or without the makefile target to auto-update latest) is just one way of supporting that user story. I'm sure there are lots of other good ways. This PR on its own will provide a (relatively) stable link target. Of course, if the API reference link scheme changes, then external links will all be broken, but this change hopefully increases the likelihood that a 5-year-old link might actually still work 5 years from now. And who knows? Maybe we'll be able to accommodate any such link schemes on a best-effort basis via redirects just as we already do for moved content.

The other PR in the makefile target would auto-update the redirect, but is not a requirement for this PR (IMO). The new version process could include a step to manually update it, just as it must already include a step to manually add a redirect rule for the just-prior version.

Since nothing links to the "latest" versions of the API docs in this change, it's only useful to people who know about it, but if it turns out to be a good docs feature, we can tell people about it by providing links to it that people can start to use.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

HTTP doesn't recommend using a 2xx status for a redirect. How about 307?

@joelsmith
Copy link
Contributor Author

@sftim It's not a true redirect, it's a transparent redirect, or a rewrite. A 200 is how Netlify handles rewriting paths. So with the 200, the server will serve files from the target location but the client will not know about it and will see the original URL, and will get back the requested content without a redirect. See https://docs.netlify.com/routing/redirects/rewrites-proxies/ for more information.

I'm proposing it this way so that the URL the user sees if they visit a "latest" version stays at the "latest" version of the URL as they browse around so once they find the object they want to link to and copy the URL, it will be the "latest" version, instead of (for example) the "v1.18" version of the URL it would be if we used a 30X HTTP redirect.

If you visit the URL I posted in the above comment you should be able to see this behavior in action if you watch the browser's URL. It should remain at the "latest" version without a redirect.

@kbhawkey
Copy link
Contributor

Reviewing and holding to consider a number of options.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2020
@kbhawkey
Copy link
Contributor

Related to PR #21929

@kbhawkey
Copy link
Contributor

/cc @jimangel @kbarnard10 @zacharysarah

@sftim
Copy link
Contributor

sftim commented Jun 21, 2020

I recommend moving the discussion about how to approach this into an issue. Then, reviewers can verify that the PR implements the agreed approach.

@joelsmith how does that sound?

@jimangel
Copy link
Member

👋 @joelsmith I love the idea, curious if you're still interested in working on it?

I agree with @kbhawkey & @sftim that this adds additional maintenance on the release cycle and there might be a better approach that is more hands-off.

Let us know how we can help! Thanks!

@kbhawkey
Copy link
Contributor

kbhawkey commented Aug 5, 2020

Hi @joelsmith . Sorry for the delay in getting back to you.
At this point, I think the redirect change would work. The question is maintaining the link.
I have an outstanding PR to add a small set of the previous API content back to the current location.
This would remove the need for the other API ref. redirects. This script could be run at release time.
I could add an update of the "latest" redirect path to this script so that the link was maintained.

Another possibility is to add a reference/generated/kubernetes-api/latest directory alongside the other API directories
and maintain the content as part of the "update script" (copy or link the files (2) to this directory).
This is also a quick change to the script and does not involve maintaining the _redirect file.
This change is similar to what you proposed previously.
What do you think?

@kbhawkey
Copy link
Contributor

kbhawkey commented Aug 5, 2020

Let's go with this change and work on the maintenance of the redirect. I am happy to do this.
@joelsmith , Do you agree that there should only be ONE "latest" url -- the path associated with the
current release? Or do you envision that a "latest" path would be available in the snapshot versions,
such as https://v1-17.docs.kubernetes.io/? I am thinking that the redirect would be removed (or the latest directory would be removed) before the snapshot is created and then reapplied once the release is final.
@tengqm @sftim Do you have any objections to moving this PR forward?

@sftim
Copy link
Contributor

sftim commented Aug 5, 2020

Could we use a 307 (temporary) redirect for now and then consider a Netlify transparent rewrite as a separate change?

You can still link to the latest version of an API with the 307 redirect but you have to be a bit more intentional about it.
(Another option for that: we could provide a permalink and a current link and let people choose which to use).

@sftim
Copy link
Contributor

sftim commented Aug 5, 2020

Long term I think we want https://kubernetes.io/ to host API documentation for historical releases as well, but that needs discussing properly in an issue.

@joelsmith
Copy link
Contributor Author

Sorry that I haven't gotten back to this, it's been a busy month. I think either approach (copied content vs. redirect) provides what I'm hoping for. I agree that it doesn't make sense for the snapshots to have a latest; that's something I didn't think much about.

I prefer transparent redirect to 307, but I can see the case for both. I'd like to be able to browse the "latest" version, then copy the URL without having to edit it. With the 307, I have to edit every URL I copy. So, yes, much more intentional that way.

Thanks, all, for looking into this for me.

@jimangel
Copy link
Member

With https://developers.google.com/season-of-docs/docs/participants/project-cncf-feloy underway, would it be possible to hold or close this PR until GSoC is completed (3 months). Also, it might make sense to scope into the project.

/cc @feloy

@k8s-ci-robot k8s-ci-robot requested a review from feloy September 13, 2020 00:44
@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 1, 2020

I plan to close this pull request. In coordination with the site's GSOD project, look into creating several links or ways of accessing
the API from the docs site. For more information, see issue #23809.
/close

@k8s-ci-robot
Copy link
Contributor

@kbhawkey: Closed this PR.

In response to this:

I plan to close this pull request. In coordination with the site's GSOD project, look into creating several links or ways of accessing
the API from the docs site. For more information, see issue #23809.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants