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

Request is fully deprecated as of February 11 2020. #414

Open
cmosgh opened this issue Feb 14, 2020 · 56 comments
Open

Request is fully deprecated as of February 11 2020. #414

cmosgh opened this issue Feb 14, 2020 · 56 comments
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@cmosgh
Copy link
Contributor

cmosgh commented Feb 14, 2020

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.

@brendandburns
Copy link
Contributor

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?

@chadbr
Copy link

chadbr commented Feb 24, 2020

Axios is pretty "big"... The "replacement" is https://github.com/mikeal/bent

but I haven't tried it yet...

@brendandburns
Copy link
Contributor

Unfortunately to use an alternate library it needs to first be integrated into the code generator that we use:

https://github.com/OpenAPITools/openapi-generator

@brendandburns
Copy link
Contributor

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.

@brendandburns
Copy link
Contributor

Switching to fetch is going to be broken until this issue is resolved somehow:

OpenAPITools/openapi-generator#3694

The generator is going to generate a broken path.

@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-testing, kubernetes/test-infra and/or fejta.
/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 Jun 8, 2020
@chadbr
Copy link

chadbr commented Jun 9, 2020

/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 Jun 9, 2020
@jschluchter
Copy link

jschluchter commented Jun 17, 2020

Not sure what the movement is here, but can I suggest got https://www.npmjs.com/package/got ?

@d-cs
Copy link

d-cs commented Jun 24, 2020

@jschluchter GOT looks nice but unfortunately doesn't have a openapi client generator.

@d-cs
Copy link

d-cs commented Jun 24, 2020

I am currently exploring how to resolve #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:

  • typescript-angular
  • typescript-aurelia
  • typescript-axios
  • typescript-fetch
  • typescript-inversify
  • typescript-jquery
  • typescript-node
  • typescript-redux-query
  • typescript-rxjs

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:

Suitable Candiate

Would be a good replacement, working for both front and backends.

  • typescript-jquery

May be Suitable

Subject to questions raised.

  • typescript-axios

Would work with compatibility changes

With further work for NodeJS

These libraries would need some additional work to make compatible with NodeJS.

  • typescript-rxjs
  • typescript-inversify
  • typescript-fetch

With further work for Browser

These libraries would need some additional work to make compatible with the browser.

  • typescript-node
  • typescript-aurelia

Not suitable

  • typescript-angular
  • typescript-redux-query

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.

@drubin
Copy link
Contributor

drubin commented Jun 24, 2020

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.

@d-cs
Copy link

d-cs commented Jun 24, 2020

@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.

@nick-ivanov-ibm
Copy link

nick-ivanov-ibm commented Aug 24, 2020

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?

@alex-klimov
Copy link

Hi guys, what is the state of this issue? Which library would be taken forward to work in browser environment?

@brendandburns
Copy link
Contributor

@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.

@alex-klimov
Copy link

Thank you, Brendan for the update!

Regarding authentication, yes, I have read through this issue, where it was discussed #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.

@jgehrcke
Copy link

jgehrcke commented Nov 5, 2020

@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: mikeal/bent#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: axios/axios#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.
googleapis/nodejs-common#618
aws/aws-sdk-js-v3#1549

@burn2delete
Copy link
Contributor

As per the generator docs, typescript-fetch should produce a node compatible version.

@brendandburns
Copy link
Contributor

brendandburns commented Nov 28, 2020

@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.

@Nokel81
Copy link
Contributor

Nokel81 commented Feb 11, 2021

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.

@XiaocongDong
Copy link

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.

@gustaff-weldon
Copy link

@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 with kubernetes-client.

This should have higher priority.

@ftqo
Copy link

ftqo commented Jun 8, 2022

@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 with kubernetes-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?

@gustaff-weldon
Copy link

gustaff-weldon commented Jun 10, 2022

@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:

Screenshot 2022-06-10 at 11 20 00

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!

@kschaab
Copy link

kschaab commented Jul 8, 2022

This looks to have been addressed with #812 so probably can be closed?

@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 Oct 6, 2022
@Nokel81
Copy link
Contributor

Nokel81 commented Oct 6, 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 Oct 6, 2022
@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 4, 2023
@Nokel81
Copy link
Contributor

Nokel81 commented Jan 4, 2023

/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 4, 2023
@Nokel81
Copy link
Contributor

Nokel81 commented Jan 13, 2023

/lifecycle frozen

@panush
Copy link

panush commented Sep 12, 2023

is there a released version already replacing the request library?
if not, is it planning to be such?

@mstruebing
Copy link
Member

@panush there is v1.0.0-rc3 which was released 22 days ago: 1.0.0-rc3

@karlbohlmark
Copy link

I'd love to understand the status of this library. Since it is rather official it discourages from creating alternative k8s clients, but to have it still be based on request after 3+ years makes me kind of lose hope. Is there still ongoing work to move to a version without this dependency? What are the roadblocks? Kubernetes is pushed by the largest companies in the industry and javascript/typescript is maybe the number 1 language in use today, still we have an official library dependent on a library (used for security-critical parts of the functionality) that has been unmaintained and deprecated for 3.5 years.

@thw0rted
Copy link

Hello from the future! I would also like to know what's going on with this update. 1.0.x has been in RC status for nearly a year, and https://github.com/kubernetes-client/javascript/blob/master/FETCH_MIGRATION.md is now almost 2 years out of date. fetch is available natively on all major platforms, including Node LTS, without a flag, and support was recently added to @types/node too. I just started looking for a JS/TS k8x client but I'm hesitant to engage with this one until it's both safe (without introducing a pile of years-old security findings) and stable.

@mstruebing
Copy link
Member

mstruebing commented Jan 17, 2024

Hello, the update is still in progress and is happening in the release-1.x branch.
There you can find an updated document as well: https://github.com/kubernetes-client/javascript/blob/release-1.x/FETCH_MIGRATION.md

We are moving pretty slow as we all mainly do it in our spare time.
I would think the missing/broken examples which needs to be migrated could be fixed/tested again because this PR is merged: #1341 even though this is mostly a workaround for now.

I would believe (but did not test it to be honest) that the release-1.x branch is able to work properly in a lot of scenarios and depending on what you want to achieve you could start using it. There is a release available on npm: https://www.npmjs.com/package/@kubernetes/client-node/v/1.0.0-rc4

Feel free to contribute to speed thing up. :)

@jrtaylorJH
Copy link

jrtaylorJH commented Jan 22, 2024

Has anybody done a write-up thus far describing the way this vulnerability interacts with the client itself? That is, does using this client meaningfully introduce an attack vector for the SSRF or is this more of a cosmetic introduction of a vulnerable dependency to the tree?

@isand3r
Copy link

isand3r commented Jun 19, 2024

Has anyone considered https://www.npmjs.com/package/@cypress/request as a replacement? We used it as a drop-in replacement for request in a critical backend service at my company with no issues.

I'm not sure what the implications would be for this library, so just a suggestion. It would be nice to have this resolved :)

Edit: if contributors are needed, I would be open to submitting the fix as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests