Add WebSocket connection support and implement /attach and /exec

[kazk]

Attempt to support WebSocket connection.

@clux

I'd like to know if this is on the right track before spending more time on it. I think it works, but haven't tried. (outputting to stdout works, see pod_attach example)
This is based on #229 (comment)

I had to change Config a bit so that it's less coupled with reqwest to create AsyncTlsConnector for WebSocket connection when creating a Client.

---

• Support WebSocket connection
  • native-tls
  • rustls (accept_invalid_certs not supported at the moment, need rustls 0.17.0+ and that requires updating tokio first)
• Make WebSocket opt-in with ws feature
• Decide how to implement attach/exec methods.
  • Added AttachedProcess struct. Need review/feedback.
• Add /attach (see pod_attach example)
• Add /exec (see pod_exec example)
• Move most of ws specific code into a separate module. This should make it much easier to maintain.
• Validate params. We only get 400 status error without any information when making connection :(
  • At least one of stdin, stdout, stderr must be set
  • stderr and tty cannot be both set because multiplexing is not supported. stderr will be ignored.
• Make WebSocketConfig configurable. Will do when there's a need.
• Make the sizes of internal buffers configurable
• Rename api/streaming to api/remote_command
• Improve documentation for AttachParams
  • Explain how stdin, stdout, stderr allows access to writer/streams from AttachedProcess
• Improve documentation for AttachedProcess
  • Explain how stdin(), stdout(), stderr() works

[clux]

quick initial comments.

[clux]

I have to admit that I've not really looked at the websocket stuff at all within kubernetes, nor have I dealt much with them otherwise, so you'll have to bear with me a bit unless there are others to help review; feel free to ping others from the original issue* to try and get help. I will ping this PR in the discord (linked in the readme).

But integration wise, holy crap, you've done an excellent job of making it fit the patterns of kube, you've juggled a lot of the really hard bits in the config around TLS, and even have a reasonable setup for the features as well (which always become hairy). Everything is in the place I would expect, and is very readable 👍. I at least think you are going in the right direction.

The only thing I would request is that we try to make the tungstenite (and anything it pulls in) part of a "websocket" or "attach" feature (or something else that describes it better). The additional subresources this supports, while useful to some, is probably not going to be as popular for the people - whose dependency trees are inflated - without a use for them.

(There is also the more philosophical point that this moves kube even further away from not owning the underlying clients; it's already hard to juggle clients, and now we have another type of clients. I was always hoping we'd be able to make it generic long run ala #100 , but I don't really know how to best proceed in that area. If you don't either, don't worry, just noting down some thoughts.)

[kazk]

There are few more chores left to do, but I think it's now ready for reviews.

[clux]

Found the compile issue: examples or kube need to at least rev tokio to 0.22.4 for tokio::io::duplex. The 0.22.0 version does not have it (and that's what i got with a blank build).

[clux]

This is looking really good now. Have had a chance to fiddle around with examples and check that things work as expected.

There is one thing I feel is a little awkward re: stream conversions, but that might not be anything at fault herein.

I pushed an example to pod_shell branch trying to redirect the example's process stdin to ws stdin and ws stdout to process stdout. See ccd2c25
It's currently failing to compile on the second tokio::io::copy, haven't dug heavily into why this is, but felt that perhaps should have worked. What do you think?

[kazk]

@clux Maybe it's because of futures and tokio having different AsyncRead traits? I'll look into it.

[nightkr]

@clux Maybe it's because of futures and tokio having different AsyncRead traits? I'll look into it.

https://docs.rs/tokio-util/0.6.0/tokio_util/compat/index.html might be relevant here

[kazk]

I couldn't figure it out, but I decided to just return AsyncRead and AsyncWrite instead because users can convert AsyncRead to a stream easily if they want using tokio::io::read_stream() (tokio_util::io::ReadStream::new() for tokio 1.0).

Got pod_shell example to compile and kind of working.

[clux]

Wow, yeah, it works! If users put some ansi unescaping on top of that they'd have something damn close to kubectl exec -it.
The messages returned seem to wrap at 80 characters, which is probably the unsupported terminal resize, so that's expected.

One small spot, while playing around: there is some truncation somewhere. If you send a very large echo "some 2000chars" it seems to truncate the response to around ~1040 characters. Not sure if you have any idea why that might be. We can always open up a follow up issue if not.

At any rate, error handling is looking great, as does the rest of the code. I don't have anything else that I can really nitpick at anymore. So whenever you are satisfied, lemme know and we'll merge it :-).

Thank you so much for all the hard work over the holiday, this is really a huge contribution. Have invited you to the collaborators so you can push branches + merge on here in the future.

[kazk]

One small spot, while playing around: there is some truncation somewhere. If you send a very large echo "some 2000chars" it seems to truncate the response to around ~1040 characters. Not sure if you have any idea why that might be. We can always open up a follow up issue if not.

It might have something to do with the internal buffer size. The default is 1024 bytes.

Thank you so much for all the hard work over the holiday, this is really a huge contribution. Have invited you to the collaborators so you can push branches + merge on here in the future.

Thanks :) This is ready to be merged 👍 I'll do some clean up after #363 and look into the truncation.

[clux]

🎉🎉🎉

Will put it in the next minor, probably try to do it today so we can release it (unless reqwest gets a tokio one version today).

[praveenperera]

Thanks for this PR @kazk, this came at a perfect time for me personally.

I believe this is going to be super useful for me in the next 2 weeks.

Really appreciate the work.

[clux]

Released in 0.46.0!