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

Document the unpublished APIs #23889

Closed
tengqm opened this issue Sep 15, 2020 · 38 comments
Closed

Document the unpublished APIs #23889

tengqm opened this issue Sep 15, 2020 · 38 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@tengqm
Copy link
Contributor

tengqm commented Sep 15, 2020

This is a Feature Request

What would you like to be added

There are a few APIs that are not published as RESTful resources. Here are some examples:

For these definitions, there may and may not be API paths associated with them, simply generate documentation for the struct definition would be very helpful.

Why is this needed

These types are not designed to be operated through the API server using CRUD operations. However, an administrator needs to understand their definitions in order to better configure and manage a Kubernetes cluster.

Comments

This needs some tooling effort to parse the Go source file. The tool can go to kubernetes-sigs/reference-docs repo.

@tengqm
Copy link
Contributor Author

tengqm commented Sep 15, 2020

@sftim
Copy link
Contributor

sftim commented Sep 15, 2020

/kind feature

@tengqm does it make sense to treat this as an umbrella issue?

Also potentially relevant (as an aside only) to #23809

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 15, 2020
@tengqm
Copy link
Contributor Author

tengqm commented Sep 15, 2020

@sftim I think this one is an umbrella issue. #23809 is not very relevant because the "unpublished" APIs don't have swagger JSON files.

@sftim
Copy link
Contributor

sftim commented Oct 8, 2020

/triage accepted

@sftim
Copy link
Contributor

sftim commented Oct 12, 2020

For this umbrella issue, I would include the gRPC socket that serves the Pod Resources API / also see kubernetes/kubernetes#92165

What do you think @tengqm ? Is this the right home for tracking that?

@tengqm
Copy link
Contributor Author

tengqm commented Oct 12, 2020

@sftim I'm don't have a strong opinion on where the API docs are served. Maybe just some simple markdown tables (or HTML tables) capturing the comments in the source code would be a good starting point.

@kbhawkey
Copy link
Contributor

kbhawkey commented Nov 12, 2020

Hi @tengqm . I need to read through the changes.

Can you elaborate on this comment:

These types are not designed to be operated through the API server using CRUD operations. However, an administrator needs to understand their definitions in order to better configure and manage a Kubernetes cluster.

How does an administrator use the content (component flags, client code)?

@tengqm
Copy link
Contributor Author

tengqm commented Nov 13, 2020

@kbhawkey See this section: https://kubernetes.io/docs/tasks/administer-cluster/kubelet-config-file/#create-the-config-file

When we are trying to tell admins how to create/customize kubelet config file, we are forcing them to read Go code. The configuration file is nothing more than a JSON data. See the need for this kind of reference?

@sftim
Copy link
Contributor

sftim commented Nov 13, 2020

This still feels like a really useful improvement.

@kbhawkey
Copy link
Contributor

kbhawkey commented Nov 13, 2020

@kbhawkey
Copy link
Contributor

Hi @tengqm . I think this information is useful ❇️ . Some questions:

  • Does the information represent APIs or configuration interfaces? If the information is not an API, I would
    not call it an API.
  • I'd like to see the pages generate as docs pages (front matter, common left nav, header, footer, ...) or generate as headless pages to be included in another page. Since the docs are generated from code, it makes sense to accept text changes only from the code unless there is an issue with the generator.
  • How often do the types change (versioning)?
  • The generated page includes the package name, an inline TOC (only for the resources type(s)?), and tables for the type fields and descriptions and included definitions.
  • Do you want to link to this type of page from docs pages or include snippets of information in the docs?
  • Does it makes sense to create a new section for configuration?
  • How do these pages fit in with the component tool pages (flags)?

@neolit123
Copy link
Member

neolit123 commented Nov 13, 2020

hi, i do agree we should publish the missing APIs (which we also call component config API, btw).

on here i'm asking a few questions to @tengqm.
kubernetes/kubernetes#96215 (comment)

my primary questions are around process, since i did not understand why are we using godoc to generate these swagger docs and the technical limitations around that. godocs are targeting developer users and not admin users, but since we didn't have anything else for kubeadm, we added some authored content in there.
https://godoc.org/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2

ideally we should use the same process for generating the swagger docs as for the core kubernetes APIs, hosted at:
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/

@alculquicondor
Copy link
Member

kubernetes/kubernetes#88919 would indeed be kube-scheduler part of this. This API is for configuring kube-scheduler at startup. It is read from a file. We also discussed some more here: kubernetes-sigs/reference-docs#138

What do you think about the generated scheduler reference documentation,

That seems directly obtained from the code, which is what we want. We just never got a contributor to work on the integration of generation scripts to the website.

@neolit123
Copy link
Member

neolit123 commented Nov 16, 2020

@tengqm @alculquicondor during the weekend i played with generating swagger.js from our API types and producing a page similar to:
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/

here kubernetes-sigs/reference-docs#138 i see you've discussed the tool https://github.com/ahmetb/gen-crd-api-reference-docs. i have not tried it by everything else did not work for me, or was simply poorly documented - including kube-openapi.
EDIT: ok, so gen-crd-api-reference-docs works, but it generates a bare-bone HTML. gen-apidocs does the same but needs swagger.json.

so i wrote yet another parser for go types-> swagger.js
https://gist.github.com/neolit123/6ffe06798e3583c9535afab6f909e75f

once i had a swagger.js i tried feeding it in https://github.com/kubernetes-sigs/reference-docs/tree/master/gen-apidocs, but soon realized that gen-apidocs is making a number of hardcoded assumptions and only works for core APIs. minimal patches were required...

my vote goes for producing swagger json and adapting gen-apidocs to support any component's API and publishing it potentially at a separate page or unifying all component-config API under the same page.
if swagger.json and gen-apidocs is not something we'd like to use, i'd like to understand why?

other things we need to consider:

@alculquicondor
Copy link
Member

I don't have an opinion on what tools are best. I'll leave the decision to sig-docs.

@tengqm
Copy link
Contributor Author

tengqm commented Nov 17, 2020

@neolit123 Actually you don't need a swagger/OpenAPI spec for this kind of types. What we are trying to document are some configuration structures rather than fully fledged RESTful APIs. In a swagger/OpenAPI, peopled define not only the structural representations that flow between the server and the client, they define other things such as API path, HTTP verbs to use, content-type to negotiate, query strings to accept, status code for each API, etc.

The swagger json file is generated by the kube-apiserver today. Behind the scene there are some preprocessing logics that parse the Go source code and generates some intermediate files containing half-baked API specification. When you inquire an API server for the swagger spec, the API server goes through all those struct definitions and yield a JSON result for you. There is no magic. At the end of the day, it is all about gengo.

The genref tool I'm proposing skips all the unnecessary conversions and generate the HTML output directly. If needed, we can easily tune it to generate markdowns.

As for tuning kubeadm comments to generate better docs, that is a different topic. The "kubeadm embdded authored contents" doesn't look very ugly if we are taking care of the format with some tweakings:

https://tengqm.github.io/doc/kubeadm-config.v1beta1.html

The output was generated from exactly the code in the PR I proposed to kubeadm source code.

@neolit123
Copy link
Member

neolit123 commented Nov 17, 2020

Actually you don't need a swagger/OpenAPI spec for this kind of types. What we are trying to document are some configuration structures rather than fully fledged RESTful APIs. In a swagger/OpenAPI, peopled define not only the structural representations that flow between the server and the client, they define other things such as API path, HTTP verbs to use, content-type to negotiate, query strings to accept, status code for each API, etc.

while kubeadm currently doesn't serve anything, the components like kubelet, kube-scheduler, kube-controller-manager and kube-proxy do expose endpoints which are currently mostly undocumented. the openapi format can help with that. so i slightly disagree if we are planning to discard openapi.

The swagger json file is generated by the kube-apiserver today. Behind the scene there are some preprocessing logics that parse the Go source code and generates some intermediate files containing half-baked API specification. When you inquire an API server for the swagger spec, the API server goes through all those struct definitions and yield a JSON result for you. There is no magic. At the end of the day, it is all about gengo.

yes, i figured that out. except i still cannot make the kube-openapi generate the structs.

The genref tool I'm proposing skips all the unnecessary conversions and generate the HTML output directly. If needed, we can easily tune it to generate markdowns.
As for tuning kubeadm comments to generate better docs, that is a different topic. The "kubeadm embdded authored contents" doesn't look very ugly if we are taking care of the format with some tweakings:

the output looks nice, but i think we should close the kubeadm PR and move these changes on the side of the generation tool as mutations. also, all the authored content in the kubeadm doc.go feels like should be moved to a MD file (this was discussed a few times already with the kubeadm maintainers)

@tengqm
Copy link
Contributor Author

tengqm commented Nov 17, 2020

@neolit123 Alright. If kubeadm decides to generate fully fledged OpenAPI/swagger specs, that is great. We can wait for that to happen. We have to clone the kubeadm code in order to generate the struct definitions today; that is another tech debt.

Moving content out for the doc.go into a markdown file is a good idea. I'm all for it.

@tengqm
Copy link
Contributor Author

tengqm commented Nov 17, 2020

@kbhawkey Sorry for the late response.

Does the information represent APIs or configuration interfaces? If the information is not an API, I would not call it an API.

We cannot decide how the development team (SIGs) want to call them. Some are actually just configuration data structures, others are exposed by the API endpoint of the component in question. For example, in kubelet, it is called a config, and the config is separated from kubelet apis where most APIs are only defined for protobuf rather than RESTful clients.

I'd like to see the pages generate as docs pages (front matter, common left nav, header, footer, ...) or generate as headless pages to be included in another page. Since the docs are generated from code, it makes sense to accept text changes only from the code unless there is an issue with the generator.

That is doable and can be done easily. We have some Go templates for rendering the HTML output, just for show cases purpose. We can have templates for markdowns.

How often do the types change (versioning)?

It really depends. However, I do believe each SIG is tracking changes to these types as official APIs, I mean, they are following the versioning practices. Since these types are bound to the binaries (e.g. kubeadm, kubelet) that consume them or expose them, we can safely update the generated docs once per release cycle, just as we do for API reference.

The generated page includes the package name, an inline TOC (only for the resources type(s)?), and tables for the type fields and descriptions and included definitions.

Yes. Only (exposed) resource types are included. If another type definition Bar is referenced from Foo in current package but Bar is not defined in current package, we can extract it as well.

Do you want to link to this type of page from docs pages or include snippets of information in the docs?

My original thought is just links. Point readers to the rendered HTML pages rather than source code or Go docs.

Does it makes sense to create a new section for configuration?

Yes, I think so. Maybe a subsection under reference.

How do these pages fit in with the component tool pages (flags)?

The overall trend is to deprecate all command line flags in favor of config files. Most of the so called "unpublished" APIs are actually about the config file format. So ... maybe we need both today so people know ... if I'm specifying this as command line flag, I should use --api-server, ... oh wait, that flag is marked as deprecated... oh I see, I should use apiServer in the configuration file which is an equivalent.

@neolit123
Copy link
Member

neolit123 commented Nov 17, 2020

@tengqm
for your comment here kubernetes/kubernetes#96215 (comment)

I'm fine with closing this PR. We have an issue to fix here and somewhere else. Generating user facing docs from source code comment where the fields have to have its first character in upper case so that they are accessible out of the package. While at the same time, the users see some different spelling and get confused.

the tooling should just parse the json tag of the field as the "field name".
this is possible with go's AST parser. https://golang.org/pkg/go/ast/#Field

Alright. If kubeadm decides to generate fully fledged OpenAPI/swagger specs, that is great. We can wait for that to happen. We have to clone the kubeadm code in order to generate the struct definitions today; that is another tech debt.

as mentioned above, kubeadm has the least need for OpenAPI/swagger specs. it's the other components that may want to document all of their endpoints / requests / responses.

Moving content out for the doc.go into a markdown file is a good idea. I'm all for it.

what is SIG Docs' preference?

instead of having content in doc.go file should we start adding readme.md's in API packages:
https://github.com/kubernetes/kubernetes/tree/master/cmd/kubeadm/app/apis/kubeadm/v1beta2
that include similar authored content or should we move this content to k8s.io pages?

i'm leaning towards moving this to a k8s.io page.

some history on this topic is that in the past SIG Docs objected to the kubeadm maintainers adding documentation in a doc.go file, but the API was moving too fast and it was a bit hard to maintain it at k8s.io.

cc @jimangel @fabriziopandini

@tengqm
Copy link
Contributor Author

tengqm commented Nov 18, 2020

@neolit123

the tooling should just parse the json tag of the field as the "field name".

The tool is doing exactly that. The problem is about the comment lines, from which the docs (including swagger spec) are generated.

Moving content out for the doc.go into a markdown file is a good idea. I'm all for it.
what is SIG Docs' preference?

Is it possible to paste that doc.go content into a markdown and find a place for it in website? Referencing docs at website from source code is an acceptable practice, maybe?

@neolit123
Copy link
Member

The tool is doing exactly that. The problem is about the comment lines, from which the docs (including swagger spec) are generated.

this is a problem here too:
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/

so instead of adapting all APIs to follow a new style, we should make the tooling post-process the comments.
and i wouldn't say it's much of a problem too.

"io.k8s.api.admissionregistration.v1.MutatingWebhook": {
      "description": "MutatingWebhook describes an admission webhook and the resources and operations it applies to.",
      "properties": {
        "admissionReviewVersions": {
          "description": "AdmissionReviewVersions is an ordered list of preferred `AdmissionReview` versions the Webhook expects. API server will try to use first version in the list which it supports. If none of the versions specified in this list supported by API server, validation will fail for this object. If a persisted webhook configuration specifies allowed versions and does not include any versions known to the API Server, calls to the webhook will fail and be subject to the failure policy.",

->
pseudo markdown:

`mutatingWebhook`: `mutatingWebhook` desribes....
  `admissionReviewVersions`: `admissionReviewVersions` is an ...

Is it possible to paste that doc.go content into a markdown and find a place for it in website? Referencing docs at website from source code is an acceptable practice, maybe?

we can certainly do that. wanted to see if someone has objections, this also means we have to link to the new page from kubeadm docs (also from k/kubeadm), so potentially we should make this after 1.20 releases.

@tengqm
Copy link
Contributor Author

tengqm commented Nov 19, 2020

@neolit123 I did considered that possibility -- having the post processing tool to make the comments more user friendly. However, it turned to be impractical. Let me give you a few examples where we will need an NLP (natural language processing) engine to correctly figure out what the text means:

Example 1

	// Expires specifies the timestamp when this token expires. Defaults to being set
	// dynamically at runtime based on the TTL. Expires and TTL are mutually exclusive.
	Expires *metav1.Time `json:"expires,omitempty"`

Example 2

	// TLSBootstrapToken is a token used for TLS bootstrapping.
	// If .BootstrapToken is set, this field is defaulted to .BootstrapToken.Token, but can be overridden.
	// If .File is set, this field **must be set** in case the KubeConfigFile does not contain any other authentication information
	TLSBootstrapToken string `json:"tlsBootstrapToken,omitempty"`

@sftim
Copy link
Contributor

sftim commented Nov 19, 2020

Unless Go adopts a (strong) convention for Markdown, or other formatting, in comments, I recommend against automatically inferring formatting from comments.

@neolit123
Copy link
Member

@sftim

Unless Go adopts a (strong) convention for Markdown, or other formatting, in comments, I recommend against automatically inferring formatting from comments.

it can be a problem in general. but it feels like in k8s we should adopt a style.
yet, as i pointed above, the API docs at https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/ have had the same problem where comments just talk about PascalCase fields but i don't think i've heard of complains from users.

@tengqm i think one way to solve this is if field references in comments are always prefixed with ., unless its the first word in the comment:

// Expires specifies the timestamp when this token expires. Defaults to being set
// dynamically at runtime based on .TTL. .Expires and .TTL are mutually exclusive.
Expires *metav1.Time `json:"expires,omitempty"`
field description
expires expires specifies the timestamp when this token expires. Defaults to being set dynamically at runtime based on ttl. expires and ttl are mutually exclusive.

@sftim
Copy link
Contributor

sftim commented Nov 19, 2020

adopt a style

Maybe a KEP? It does need buy-in.

@neolit123
Copy link
Member

neolit123 commented Nov 19, 2020

it does. until then we can leave the field descriptions unprocessed, but continue to think how to publish this documentation.
by that i mean, we can publish the component config documentation with unprocessed descriptions.

@sftim
Copy link
Contributor

sftim commented Nov 19, 2020

Doesn't block either PR but registrycredentials.k8s.io/v1alpha1 from #24929 might be relevant here too.

@sftim
Copy link
Contributor

sftim commented Nov 19, 2020

ABAC policy is also mentioned in #23885

@neolit123
Copy link
Member

neolit123 commented Nov 19, 2020

Doesn't block either PR but registrycredentials.k8s.io/v1alpha1 from #24929 might be relevant here too.

which is more appropriate for the swagger format, because there is (planned) inter-component communication with requests / responses.

@sftim
Copy link
Contributor

sftim commented Nov 23, 2020

https://coveooss.github.io/json-schema-for-humans/ might have some ideas about either the styling, a mechanism for documentation generation, or both.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 21, 2021
@neolit123
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 21, 2021
@sftim
Copy link
Contributor

sftim commented Apr 3, 2021

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Apr 3, 2021
s-kawamura-w664 pushed a commit to s-kawamura-w664/website that referenced this issue Apr 22, 2021
Remove reference in favor of kubernetes#23889
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 2, 2021
@k8s-triage-robot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 1, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/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
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

8 participants