search

kubernetes-sigs / reference-docs

Tools to build reference documentation for Kubernetes APIs and CLIs.
Apache License 2.0
89 stars 105 forks source link

Update API reference generator for 1.28 #325

Closed tengqm closed 1 year ago

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

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](https://github.com/kubernetes-sigs/reference-docs/blob/master/OWNERS)~~ [tengqm] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
sftim commented 1 year ago

Could you explain the change to initOperationParameters() @tengqm? It looks reasonable but I don't have context for why we're updating that function here.

tengqm commented 1 year ago

Could you explain the change to initOperationParameters() @tengqm? It looks reasonable but I don't have context for why we're updating that function here.

The new swagger doc starting v1.28 has some drastical changes to how path parameters are documented. Previously, the swagger generator (in the upstream source code) generates the complete list of parameters for each and every operation, and the parameters are repeatedly documented underneath that operation. For example, every "listNamespacedFooBar" operation has a watch parameter documented. This means the how swagger spec is large, with duplicated contents.

Starting with v1.28, the generator was "optimized". All parameters are extracted to the top level swagger parameters block, leaving the per-operation parameter definition merely a reference to the global one. The has heavily reduced the spec size. It was a good intent.

Our previous code wasn't able to handle this case. So we have to parse the parameter reference, fetch the global parameter definitions and document them on a per-operation basis.

There are significant bugs in the generated swagger today. Mainly because the logic cannot handle duplicated parameter names properly. For example, for connectNamespacedPod....ProxyWithPath operations, there are two parameters named path, one in the URL path, the other in the body. The generated swagger in 1.28 is not handling this correctly. That is something to be fixed in the upstream.

sftim commented 1 year ago

There are significant bugs in the generated swagger today. Mainly because the logic cannot handle duplicated parameter names properly. For example, for connectNamespacedPod....ProxyWithPath operations, there are two parameters named path, one in the URL path, the other in the body. The generated swagger in 1.28 is not handling this correctly. That is something to be fixed in the upstream.

Is there an issue? Let's try to file one.

/lgtm