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.

Sign up for GitHub

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

Jump to bottom

Decouple REST Framework #250

Closed
austince opened this issue Sep 27, 2021 · 6 comments · Fixed by #252
Closed

Decouple REST Framework #250

austince opened this issue Sep 27, 2021 · 6 comments · Fixed by #252
Assignees
@austince
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@austince
Copy link
Contributor

austince commented Sep 27, 2021 •
edited

Currently, the builder package is tightly coupled with a 2017 version of go-restful. This dependency is exposed in three places:

  • the builder's BuildOpenAPISpec, which takes a slice of *restful.WebServices
  • the common config's GetOperationIDAndTags function, which takes a *restful.Route
  • the handler's BuildAndRegisterOpenAPIVersionedService

It would be nice to be able to use this builder with newer versions of go-restful, but also with other frameworks.

I propose taking ownership of the route interfaces and adding an adapter for the current version of go-restful to support the transition. We could add more adapters in this repo, or 3rd parties could implement them elsewhere.

This would result in three breaking changes that I can see, with the two places the current dependency is exposed. Would this require a formal KEP?
austince added a commit to austince/kube-openapi that referenced this issue Sep 27, 2021
@austince
Decouple REST Framework
b9c9257 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
@austince
Copy link
Contributor Author

austince commented Sep 27, 2021 •
edited

I've got a work-in-progress that accomplishes this and passes the current tests here: https://github.com/kubernetes/kube-openapi/compare/master...austince:feat/decouple-restful?expand=1

If the direction is good, just needs docs, tests, and a release strategy. 🙂

austince added a commit to austince/kube-openapi that referenced this issue Sep 27, 2021
@austince
Decouple REST Framework
e9a6c8a 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
austince added a commit to austince/kube-openapi that referenced this issue Sep 27, 2021
@austince
Decouple REST Framework
5fbebe2 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
austince added a commit to austince/kube-openapi that referenced this issue Sep 27, 2021
@austince
Decouple REST Framework
11c610f 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
@austince austince changed the title Decouple REST Framework from Builder Decouple REST Framework Sep 28, 2021
@fedebongio
Copy link

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Sep 28, 2021
@austince
Copy link
Contributor Author

austince commented Oct 6, 2021 •
edited

PR is not in ready state, but a rough changelog for k/k is here: https://github.com/kubernetes/kubernetes/compare/master...austince:feat/decouple-restful-openapi?expand=1

Main change to use the adapter: https://github.com/kubernetes/kubernetes/compare/master...austince:feat/decouple-restful-openapi?expand=1#diff-0c219ab2a174bb27d5e294d3e1179bd793927ec2f243ab364e13d37dc4b676f1

austince added a commit to austince/kube-openapi that referenced this issue Oct 7, 2021
@austince
Decouple REST Framework
4f2f372 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
austince added a commit to austince/kube-openapi that referenced this issue Oct 7, 2021
@austince
Decouple REST Framework
6ea5c02 
Takes ownership of the route interfaces
and removes exposing the `go-restful`
dependency.

Adds an adapter for the currently used
version of `go-restful`.

Closes kubernetes#250 (kubernetes#250)
@austince austince mentioned this issue Oct 7, 2021
Merged
@austince
Copy link
Contributor Author

austince commented Oct 7, 2021

/assign @austince

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough 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:

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

Please 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 Jan 5, 2022
@austince
Copy link
Contributor Author

austince commented Jan 5, 2022

/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 Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@austince austince

Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Decouple REST Framework austince/kube-openapi
4 participants
@fedebongio @austince @k8s-ci-robot @k8s-triage-robot