godaddy / kubernetes-client

Simplified Kubernetes API client for Node.js.
MIT License
962 stars 192 forks source link

.getStream() doesn't authenticate with refreshAuth #373

Open chriskinsman opened 5 years ago

chriskinsman commented 5 years ago

On an AWS EKS cluster calling:

        const stream = client.apis.apps.v1.watch.namespaces('default').deployments.getStream();
        const jsonStream = new JSONStream();
        stream.pipe(jsonStream);
        jsonStream.on('data', object => {
            console.log('Event: ', JSON.stringify(object, null, 2));
        });

Fails to authenticate with:

Event: { "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "deployments.apps is forbidden: User \"system:anonymous\" cannot watch deployments.apps in the namespace \"tilloo\"", "reason": "Forbidden", "details": { "group": "apps", "kind": "deployments" }, "code": 403 }

Workaround is to call any other API first. This may be related to: #248

jtschulz commented 5 years ago

I noticed the help wanted label here. This is also something I'm facing and would like to potentially help fix, but I'm new to the project. Is this because the stream operations use websocket connections and the refreshauth isn't hooked up to those types of requests? Or is there something deeper. With a little guidance, I could pick this up. cc: @silasbw

chriskinsman commented 5 years ago

I think the issue is that refreshAuth is triggered by a non http 2xx status code.

In the case of watch() it opens a stream which returns a 2xx code but then there is an error thrown in the stream itself that isn't using an http status code. i.e. you have to look at the code on the event.

jaredallard commented 5 years ago

I might be able to set some time aside to implement this, I did the original authentication refresh logic.

@PepperTeasdale Yeah, this is failing because getSteam() doesn't have any awareness of "authentication" refresh. Essentially we lightly wrap request and trigger auth refresh if we get a 401 (might be more than 401 now)

silasbw commented 5 years ago

@jaredallard -- :heart_eyes_cat: ! Happy to help support if you find the time to fix this.

jaredallard commented 5 years ago

@silasbw and everyone else. I'm currently working on using getStream() in a personal project, so will likely be able to fix this soon. I'm in Japan (:jp:) on vacation so no firm commitments, but it's on its way!

thecodejunkie commented 5 years ago

Just throwing in my 👍 on this one. Was just playing around with kubernetes-client and ran into this 😄

silasbw commented 5 years ago

Hi folks, thanks for the feedback and patience. This should be addressed by my work to switch to @kubernetes/node-client as the default "backend" for kubernetes-client. There's been a bunch of PRs already to facilitate that transition, and there will be probably be a bunch more. I'll tag this PR when I think it's safe/reasonable to try the new backend.

Related, we're also working to move the kubernetes-client code the official kubernetes-client org. FYI, https://github.com/kubernetes-client/javascript/pull/244