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

browser-side support #165

Open
gflarity opened this issue Dec 21, 2018 · 24 comments
Open

browser-side support #165

gflarity opened this issue Dec 21, 2018 · 24 comments
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@gflarity
Copy link
Contributor

For now, the client is implemented for server-side use with node using the request library.

Just curious. Aside from the use of the request library, are there any other major impediments to supporting direct browser usage?

Would this be a welcome/encouraged contribution?

@brendandburns
Copy link
Contributor

You would need to regenerate the client to use jquery or something else. It's a welcome contribution, but it requires working with the https://github.com/kubernetes-client/gen repository to change/update the code generator.

@NicolasT
Copy link

Recent versions of swagger-codegen (as used by kubernetes-client) have a typescript-fetch 'language' type which should be portable across NodeJS and browser (using portable-fetch, which is what the generated code uses).

However, things are not as simple. There's a bit of code (config.ts mainly) that's quite tightly coupled to a NodeJS environment (not sure how to read kubeconfig files in a browser from fs...). As such, having a single library entrypoint (index.ts) which exports everything and works cross-environment won't work. In a browser environment, you'll likely want a more simple configuration mechanism, e.g. providing a bearer token you already retrieved using an OIDC implicit flow dance.

Also, some code (watch support) is using request directly.

Anyway.

Since I need browser-based access to the API soon'ish, I took the liberty to fork this repository and 'hack things in'. The resulting branch is published at https://github.com/scality/kubernetes-client-javascript/commits/browser

I don't claim this is the 'right' way to achieve the goal (it's most certainly not), but does the trick (for now).

The way it works: it uses Webpack's alias feature to re-implement what api.ts uses from request, but using fetch instead. Code is in src/browser/request-fetch.js. This is not meant as a drop-in replacement for request, only the bare minimum required.

I also dropped the NodeJS config module for a slightly different one (src/browser/config.js) which for basic usage exposes a similar interface, given a known token.

Finally, because saw no way to implement the request API used by the watch module based on fetch (a request result is a stream.Readable you can pipe, which can't be constructed when a call to fetch returns), I re-implemented that module (src/browser/watch.js), using the same logic and libraries, mostly, now including code to morph a fetch ReadableStream into a stream.Readable for further processing.

I also include a very bare-bones example in example/browser (see README.md on how to run it) which does an OIDC dance, logs you in, performs a getNode call (assuming you have the right RBAC authz) and starts a watch for cluster events which get dumped to the Javascript console.

All of this only got some small amounts of manual testing. No guarantees everything works 'just fine', at all!.

To consume the library, don't import src/browser/index.ts or dist/browser/index.js, because that won't work, unless (maybe?) if you configure your project Webpack config to alias request properly. Instead, I set the browser entry-point in package.json to dist/browser/bundle.js, which is also checked into the repository, and has the request stubs in place.

Finally, if a browser-friendly version of this library would be developed, I believe the API of the watch module should be revisited: let it return a ReadableStream or stream.Readable of watch results instead of working callback-based with a custom 'end' callback.

@NicolasT
Copy link

I figured one could adjust how kubernetes-client/gen generates the API bindings, to make it generate typescript-fetch code which would then work in the browser, and be done with it.

However, things aren't as simple: with fetch, there's no way to emulate strictSSL = false to ignore 'invalid' TLS certificates, which is possible in the current library. This feature will never be possible on client-side anyway.

So, if this feature is to be retained, then we can't use fetch in a NodeJS environment. At the same time, the hack I did to substitute request with a fetch wrapper, and the code required to make watch work in a browser (which will likely need some duplication if #214 lands) with a somewhat odd API as a result, suggest the NodeJS and browser bindings may need to be two completely different libraries...

@burn2delete
Copy link
Contributor

I agree, this library makes heavy use of platform specific code. We should move the browser javascript to it's own repo.

@NicolasT
Copy link

Goh... not entirely sure about that, to be honest. It should be possible to keep NodeJS and browser libraries in the same 'source tree'. The major pain-point is generated code, I guess. However, I bet we can work-around this, it's mostly a matter of writing API-compatible stubs and delegating to the 'right' one.

For whatever using platform-specific code (e.g. the current config using fs and such), maybe we should not inclulde those in @kubernetes-client/client-node (to be renamed to client-javascript?), but put it in either another package, or a sub-package to be imported (@kubernetes-client/client-javascript/node/index.js, @kubernetes-client-client-javascript/browser/index.js and such?). Or use rollup or similar to generate dist modules for both platforms, each including the 'right' implementation. When creating dist bundles, let's aim for ESM-style such that tree-shaking can work (currently the bundle is... quite big, even if you only use one of the API calls).

Finally, when writing 'custom' code, e.g. to implement watch, I'd suggest we stick on the side of what a browser provides and put NodeJS-style APIs on top. What I mean is: instead of letting the browser code depend on the stream module while we already have ReadableStream and such in the WHATWG streams spec, use the latter and transform to a NodeJS 'native' stream later, the rationale being to focus on size-reduction of the browser bundle, which is less of a concern in NodeJS.

@brendandburns
Copy link
Contributor

Maybe we can extract out the node or browser specific stuff into separate things that we could have users dynamically require/import?

@NicolasT
Copy link

Maybe we can extract out the node or browser specific stuff into separate things that we could have users dynamically require/import?

To some extent, yes. However:

  • In-browser, request isn't available so the code as generated today can't work.
  • In NodeJS, fetch can be made available, but the fetch API doesn't support disabling TLS server certificate validation, which is currently supported by this lib.
  • As in my PoC, we can stub request with an implementation using fetch underneath (and not support disabling TLS server certificate validation), but this puts requirements on the library consumer build environment (Webpack's alias feature, rollups alias plugin,...)

Other approach would be to generate two api.tss: one as today, one using the typescript-fetch 'language', then dispatch to the 'correct' one. However, this could again push customization to the library user's build system.

One thing we could do, is to generate two api.tss, and create two pre-built bundles in dist, instead of a whole lot of tsc-generated modules, one for NodeJS (using typescript/request), one for browser (using typescript-fetch/fetch, and a slightly different API for config), and dispatch in package.json to the right bundle.

However, since then we'd have a single module, we better publish things as an actual ES6 module, such that (especially in browser environments) tree-shaking can work.

@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 May 29, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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 rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 28, 2019
@drubin
Copy link
Contributor

drubin commented Jun 29, 2019

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 29, 2019
@dinfer
Copy link

dinfer commented Jul 24, 2019

any progress?

@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 Oct 22, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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 rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 21, 2019
@axisofentropy
Copy link

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 21, 2019
@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 Feb 19, 2020
@mitar
Copy link

mitar commented Feb 19, 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 Feb 19, 2020
@brendandburns
Copy link
Contributor

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 24, 2020
@d-cs
Copy link

d-cs commented May 20, 2020

I am actively exploring this. I am looking at it from the perspective of creating a new client, but reusing as much of what already exists as possible.

The first question we need to address is how to approach authentication.

Bearer token authentication in the browser is easy, however this is not how most commonly deployed Kubernetes clusters are configured to authenticate. Client certificate is also possible, the certificate would need to be imported into the users browser.

The real issue is exec, working purely in the browser, this is not possible. This would make it difficult for users of EKS or the Digital Ocean Managed k8s.

As far as I can tell there are only two options that would require minimal additional effort in order to authenticate.

  1. Advise users of any software using the javascript client that they must run kubectl proxy and only use the browser based client over localhost.
  2. Create a proxy that performs the authentication such as this shamelessly self plugged one and create a client performs the other tasks

Both of these methods delegate the authentication responsibility elsewhere. I feel this is necessary so that the library doesn't enforce a particular security strategy.

The advantage of 1 is that it is a single command, already using what a k8s user would have installed.

The advantage of 2 is that, at least for things like electron applications, the proxy client can be bundled with the application. I also plan on adding an Electron ipc interface so that the user of the library doesn't need to open up a port on their local machine to communicate with the cluster

Any thoughts on this would be greatly appreciated, since it is this that will lay the foundation of how the client is built.

@brendandburns
Copy link
Contributor

I would prefer to disable exec authentication in the browser-based scenario.

In general, browser based authentication to a Kubernetes cluster mostly doesn't work because most Kubernetes clusters use their own certificate authority rather than a well-known certificate authority.

Unlike a client application, it is very, very hard (not to mention pretty insecure) to load a custom certificate authority into a browser, so I don't think we'd recommend that.

Given that, the only real options are some sort of proxy, either via kubectl proxy or the client library you point to.

I don't think this client library should worry about any of that. We can document the problems and the potential workarounds, and just leave this library as is and expect that people will use some sort of proxy outside of this client library.

Let me know if there are more questions.

@brandonkal
Copy link

A mostly browser-compatible version would be appreciated to use in the Deno runtime server-side.

@sfxworks
Copy link

sfxworks commented Nov 30, 2020

On our end, we use OpenID that requests a token. I believe it isn't entirely uncommon to integrate something like google's OIDC with the kube-api server.

I'd also like to voice the thought of at least including attach. Though similar I would love to develop a dashboard that I can let users quickly attach to their pods since some of their programs have tty/stdin.

@NicolasT
Copy link

I'm updating my branch with current upstream as we speak. However, getting isomorphic support upstream, even if this is only for a part of the library/API, would help a lot and remove the somewhat tedious work to keep things in sync.

In our application, we have a SPA which talks to K8s API using only CRUD APIs. Authentication is using bearer tokens (which the SPA retrieves from the IdP using the standard OIDC dance with a bunch of redirects). The K8s API access is TLS-terminated using a proxy running in the HTTP(S) server which also serves the SPA, so no need to accept any certificates on the client (except for the SPA server one).

@saltbo
Copy link

saltbo commented May 30, 2022

any progress?

@kingdonb
Copy link

kingdonb commented Oct 15, 2022

It sounds like there isn't much chance of this feature request advancing based on these issues:

  • the K8s API TLS certificate is unlikely to be from a well-known CA
  • browser (or electron-based) models don't generally let you override the CA without a big hassle
  • many K8s auth models require a binary that you can exec to perform the authentication, which further limits the likelihood that this can work
  • talking to k8s API through a proxy is therefore the only generally reliable or straightforward way to get a connection to the K8s API from inside a browser

Generally speaking, although all those issues above sound like they could be configured around, the workarounds are not standard or well-known configuration paths, would not be uniformly available across different providers, etc.

Did I miss anything here? Has anyone developed a decent exemplar eg. VSCode extension that contacts the K8s API directly (or via proxy) without shell exec'ing more than absolutely necessary, to create the proxy for example... or is there a decent straw-man that gets us most of the way so that we can have a better chance at this, to start from something?

We've based our work in Electron (vscode-gitops-tools from Weaveworks) on the official Kubernetes extension from Microsoft, which uses shell exec to get commands into the Kubernetes API via kubectl. We had our first (and second) CVE advisory reported and subsequently published not long ago, CVEs on vscode-gitops-tools which were arbitrary code execution vulns created because we took strings and passed them raw into shell execs. It's clear that this is risky behavior and as careful as you may be, the likelihood remains that some code hasn't escaped a user input properly.

IMHO we should not be promoting this method (shell exec kubectl) as a model approach to access Kubernetes from in a JavaScript application, but it's clear from reading this thread that alternative approaches might be fraught for other reasons as summarized above.

So what should we be recommending? How do JavaScript or TS clients*(in a browser context) perform K8s server-side apply operations today, synchronously, or maintain a persistent connection to get notified when some K8s event/resource subscriber gets tripped?

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