Change the endpoint called by loadSpec #356
Conversation
According to this Kubernetes doc: https://kubernetes.io/docs/concepts/overview/kubernetes-api/#openapi-and-swagger-definitions The `/swagger*` endpoints have been deprecated starting with v1.10. This commit changes `loadSpec` to address this: 1. Initially attempts to contact `/openapi/v2` with `Accept: application/json` in order to get the swagger file via the new URL 2. If that fails with a 404, attempts to contact `swagger.json` to support clusters which do not have the new endpoint
If the response is a 404, the async call must be returned so that execution of the first callback stops.
- Changed existing test to mock `/openapi/v2` - Added new tests to check fallback-logic and error logic
lib/swagger-client.js
Outdated
| @@ -39,10 +39,19 @@ class Root extends Component { | |||
| */ | |||
| loadSpec() { | |||
| return new Promise((resolve, reject) => { | |||
| this.http.request('GET', { path: '/swagger.json' }, (err, res) => { | |||
| this.http.request('GET', { headers: { accept: 'application/json' }, path: '/openapi/v2' }, (err, res) => { | |||
There was a problem hiding this comment.
Is the headers required? I would expect https://github.com/godaddy/kubernetes-client/blob/master/lib/request.js#L147 to do that implicitly.
There was a problem hiding this comment.
You're right, it's unnecessary, I explicitly added it because the k8s docs (https://kubernetes.io/docs/concepts/overview/kubernetes-api/#openapi-and-swagger-definitions) use headers to differentiate between json and pb swaggers, but k8s also explicitly uses application/json if header is not set, will remove.
lib/swagger-client.js
Outdated
| if (res.statusCode !== 200) { | ||
| return reject(new Error(`Failed to get swagger.json: ${res.statusCode}`)); | ||
| if (res.statusCode === 404) { | ||
| return this.http.request('GET', { path: '/swagger.json' }, (err, res) => { |
There was a problem hiding this comment.
Can you run the linter? I think it will complain about aliasing here (e.g., err in the first callback and err here).
There was a problem hiding this comment.
fixed err and res aliasing
- The 'accept' header defaults to 'application/json' on both the http lib and the kubernetes server, so no need to add it - Aliased err and res objects were given different names
|
What's your policy on Hope you had a good vacation! |
|
Published this change with |
According to this Kubernetes doc:
https://kubernetes.io/docs/concepts/overview/kubernetes-api/#openapi-and-swagger-definitions
The
/swagger*endpoints have been deprecated starting with v1.10.This commit changes
loadSpecto address this:/openapi/v2withAccept: application/jsonin orderto get the swagger file via the new URL
swagger.jsonto support clusters whichdo not have the new endpoint
fixes #352