Open cmosgh opened 4 years ago
This is generated by the open api code generator. Looks like we could switch to Axios. Is there a preference for a particular HTTP client library?
Axios is pretty "big"... The "replacement" is https://github.com/mikeal/bent
but I haven't tried it yet...
Unfortunately to use an alternate library it needs to first be integrated into the code generator that we use:
We could try fetch, but it's not node native, so we'd have to use the node-fetch package (not a blocker though)
I will look into this.
Switching to fetch is going to be broken until this issue is resolved somehow:
https://github.com/OpenAPITools/openapi-generator/issues/3694
The generator is going to generate a broken path.
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-testing, kubernetes/test-infra and/or fejta. /lifecycle stale
/remove-lifecycle stale
Not sure what the movement is here, but can I suggest got
https://www.npmjs.com/package/got ?
@jschluchter GOT looks nice but unfortunately doesn't have a openapi client generator.
I am currently exploring how to resolve https://github.com/kubernetes-client/javascript/issues/165 it looks like the decision made here could impact that.
To choose a library to make the HTTP calls we need to choose one of the following:
Ignoring the deprecated and experimental ones. Also ignoring the JavaScript ones as having the types for such a large API is certainly useful.
I would say that typescript-angular
would prevent developers of other popular frontend frameworks would be locked out. I'm also not sure about it working on NodeJS environments as it uses an Angular library to make HTTP requests (see here).
typescript-aurelia
could need to be polyfilled and bundled for the browser (see here). Although I'm not 100% sure this is true, it might be a boilderplate README.md file.
@chadbr what makes typescript-axios
big? The outputted JS is huge when you run it against the k8s API? There is a configuration option that splits the outputted code out into individual files rather than one big one, I got that working locally and could share the output? Or are you talking about the bundle size?
We'll rule out typescript-fetch
for the reasons @brendandburns mentions above.
typescript-inversify
uses fetch
under the hood so again we'd need to handle how this works on NodeJS.
typescript-node
uses the native http
library which is why the browser side support ticket exists.
typesript-redux-query
would oblige the user to use Redux.
typescript-rxjs
this uses ajax
under the hood. This seems to have some compatibility issues with NodeJS, requiring extra libraries.
With that in mind we have the following:
Would be a good replacement, working for both front and backends.
Subject to questions raised.
These libraries would need some additional work to make compatible with NodeJS.
These libraries would need some additional work to make compatible with the browser.
This is based on my observations, I've tried to back up claims where possible. I'm happy to attempt the refactor but before going too far I'd like to be sure that we made the right decision.
Wow thanks for the detailed write-up @d-cs
Just to give my 2 cents on a preference basis. I prefer got
personally, but I think axios
would be better fit for this project. if we care about the size we should think about using node-fetch
Regarding the browser support, I think they should be tackled separately but still taking into account this fact.
@drubin not a problem 😄 , I don't want us to get weighed down with deciding which direction to go. I think it's important to get this fixed sooner rather than later.
I agree with your comment about browser support, I was tinkering and found that a lot of features could be included for frontend using webpack.
Another thought around fetch
/node-fetch
- what about something like https://github.com/lquixada/cross-fetch? The openapi client generator should still work, but which fetch to use would be delegated to another library? Also covers React Native in that case.
Given the pervasive dependency on request
or its eventual replacement, would it be useful to consider a wrapper that would isolate the actual implementation and/or allow people to plug in different implementations? There seems to be an attempt at that in watch.ts
-- may be it could be generally adopted?
Hi guys, what is the state of this issue? Which library would be taken forward to work in browser environment?
@alex-klimov there's no current implementation that works in the browser (nor any concrete plan to develop one) we're definitely open to it, but not currently under development.
Accessing Kubernetes from the browser is a little weird anyway, since most clusters use custom certificate authorities, and you can't load a custom CA inside the browser.
Thank you, Brendan for the update!
Regarding authentication, yes, I have read through this issue, where it was discussed https://github.com/kubernetes-client/javascript/issues/165
I was researching what would be required to provide web dashboard with basic diagnostic/health investigations for vanilla Kubernetes clusters. My goal was to have as little moving parts as possible. And, yes, it looks like the proxy or bearer token authentication support enabled in the cluster would be a requirement to make that web dashboard to connect to the cluster.
@d-cs thanks for the write-up above. Would you mind having another look at https://github.com/sindresorhus/got ?
In our projects (TypeScript codebase, NodeJS runtime) we've been using the HTTP client https://github.com/sindresorhus/got for more than a year now, with good results. I selected it in particular for its timeout control, error handling, configurable retrying behavior.
I would love to add to this discussion here that I hope that in the future this library is going to expose fine-grained timeout control for the underlying HTTP client and will also offer configurable retrying behavior for transient errors (such as for connect() timeouts or 5xx responses), also see #544. Hope that we all agree that this helps a lot towards building resilient systems. As a north star we may want to look at https://boto3.amazonaws.com/v1/documentation/api/latest/guide/retries.html for inspiration :-).
The "replacement" is https://github.com/mikeal/bent
I asked for "Expose options for underlying http client" about a year ago: https://github.com/mikeal/bent/issues/59 -- nothing moved, a no-go criterion I think.
About Axios, it's worth pointing out that as of today it does still not support connect() timeout control: https://github.com/axios/axios/issues/1739 -- a no-go IMO -- this library here should absolutely allow for configuring the connect() timeout separately from other timeouts.
Huh. I feel like an evangelist trying to push for better error handling in the world of client libraries in the node/ts/js ecosystem. https://github.com/googleapis/nodejs-common/issues/618 https://github.com/aws/aws-sdk-js-v3/issues/1549
As per the generator docs, typescript-fetch should produce a node compatible version.
@flyboarder it's not just about being node compatible. We need it to be API compatible. Meaning that any user of this API doesn't have to update their code. I don't believe that is the case with the fetch generator (though we could test and see)
We could take a one-off breaking change, but this library has lots of users as far as I can tell from the npm stats and it seems pretty drastic to break them all.
I think that a one off breaking change would be acceptable (especially since this package isn't even 1.0.0
yet). I would vote for got
as I think that it has the most mature API.
FYI, I have implemented an informer based on axios, if you want to try it out, feel free to do so. https://github.com/XiaocongDong/kubernetes-axios-informer.
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
This is still needed
/remove-lifecycle stale
@brendandburns Any update on this? What are the blockers currently? Is it still the generator? What specific do we need to see there so we can help move it forward?
This issue gets new relevance due to the transitive dependency to json-schema@v0.2.3
, which has a critical severity vulnerability.
That "critical" "vulnerability" is entirely irrelevant in this context as the method in question is never used in any of the code paths of request
or its dependencies.
Yes, I understand that. Unfortunately, typical vulnerability scanners cannot find out that, so that I need to ignore them (which I do). However, it leaves an uncomfortable feeling that in the future it might be possible to overlook changes which actually uses the vulnerable code which is still imported.
Found this thread due to the json-schema@v0.2.3
vulnerability .
I agree that the vulnerability might be misleading since it doesn't actually affect this lib.
However, this request, how it will never be updated and that another vulnerability could come up that does indeed affect this lib in the future.
Just to be clear:
It's not that this library will never be updated, it's that updating this library to use a different HTTP client is a lot of work and this library only has a few active maintainers.
If someone wants to volunteer to make the necessary changes to migrate to a different library I'm happy to help guide the effort.
There are three approaches that I think are viable:
got
) to the calls in the request
library which this client uses.node-typescript
to use a different request library.typescript
generatorI think that the second or third option is probably the right long-term solution. But there are seemingly a lot of users of this library, we will have to look at what the level of breakage pain is.
Again, volunteers welcome!
The Cypress team seems to have forked request
and they seem to be doing some maintenance there. I haven't checked if it's a straight drop-in (or whether they intend to keep it as such), but it might be worth exploring as an attempt to delay (prolong? depends on your point of view) the pain: https://github.com/cypress-io/request
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:
lifecycle/stale
is appliedlifecycle/stale
was applied, lifecycle/rotten
is appliedlifecycle/rotten
was applied, the issue is closedYou can:
/remove-lifecycle stale
/lifecycle rotten
/close
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
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:
lifecycle/stale
is appliedlifecycle/stale
was applied, lifecycle/rotten
is appliedlifecycle/rotten
was applied, the issue is closedYou can:
/remove-lifecycle stale
/lifecycle rotten
/close
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
Still very relavent
/remove-lifecycle stale
@Nokel81 it is introducing critical security vulnerability with old request
: https://github.com/kubernetes-client/javascript/issues/812
@gustaff-weldon It isn't introducing, this discussion is about removing that dependency. The best way forward would be to switch to a generator based on node-fetch
or once fetch
is no longer "stability 1" for node, just fetch
@Nokel81 what I mean is this client is a recommended kubernetes node client, and when installed it is introducing old request
into one's project dependencies.
request
has critical security vulnerability (https://github.com/advisories/GHSA-896r-f27r-55mw), that gets "introduced" into one's project with kubernetes-client
.
This should have higher priority.
@Nokel81 what I mean is this client is a recommended kubernetes node client, and when installed it is introducing old
request
into one's project dependencies.request
has critical security vulnerability (GHSA-896r-f27r-55mw), that gets "introduced" into one's project withkubernetes-client
.This should have higher priority.
@gustaff-weldon The only version of json-schema
I see in package-lock.json
is version 0.4.0
, which according to the advisory link you posted, is the patched version that should be used. Is there something I am missing?
@ftqo Looks like it is me who's missing sth here. I just tried a clean project with just a kube-client and it is as you say:
In our project, even after yarn dedupe it still resolves json-schema to old version. Which is unexpected. Looks like I have some more digging to do, thanks!
This looks to have been addressed with #812 so probably can be closed?
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:
lifecycle/stale
is appliedlifecycle/stale
was applied, lifecycle/rotten
is appliedlifecycle/rotten
was applied, the issue is closedYou can:
/remove-lifecycle stale
/lifecycle rotten
/close
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
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:
lifecycle/stale
is appliedlifecycle/stale
was applied, lifecycle/rotten
is appliedlifecycle/rotten
was applied, the issue is closedYou can:
/remove-lifecycle stale
/lifecycle rotten
/close
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
/lifecycle frozen
is there a released version already replacing the request library? if not, is it planning to be such?
As stated here request is fully deprecated. The reasons are described in this issue . I think it is the time to choose an alternative, considering this kubernetes client is used on some production applications.
These are the current alternatives.