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

Removing both TLS stacks results in freeze in-cluster #1275

Closed
clux opened this issue Aug 8, 2023 · 0 comments · Fixed by #1261
Closed

Removing both TLS stacks results in freeze in-cluster #1275

clux opened this issue Aug 8, 2023 · 0 comments · Fixed by #1261
Assignees
Labels
bug Something isn't working config Kube config related

Comments

@clux
Copy link
Member

clux commented Aug 8, 2023

Current and expected behavior

When building with default-features = false in Cargo.toml we end up removing the default tls stack. This is intended, but the failure mode that results from this is not intended.

Basically, Config::incluster succeeds because we can parse the cert and read the evars, however the Client builder ends up not chaining on an ssl connector because it's not in the dependency tree (effectively skipping this crucial piece of code).

A controller i tested booted up, and the controller basically froze as a result.

Possible solution

I think we should change the client builder to fail if there are no tls stacks WHEN there are tls parameters that can be found in the Config (because anonymous auth exists and we don't need to break this). Maybe some kind of AuthInfo::needs_tls fn? AuthInfo basically has all the state.
EDIT: better to just check if ConfigExt::auth_layer() returns None.

Environment

incluster

Configuration and features

[dependencies.kube]
features = ["runtime", "unstable-runtime", "client"] # testing w/o tls
default-features = false
version = "0.85.0"

Affected crates

kube-client

Would you like to work on fixing this bug?

maybe

@clux clux added bug Something isn't working config Kube config related labels Aug 8, 2023
@clux clux self-assigned this Aug 8, 2023
clux added a commit that referenced this issue Aug 8, 2023
…esent

fixes #1275

Signed-off-by: clux <sszynrae@gmail.com>
@clux clux linked a pull request Aug 8, 2023 that will close this issue
@clux clux closed this as completed in #1261 Sep 8, 2023
clux added a commit that referenced this issue Sep 8, 2023
* Make rustls-tls the default tls stack feature

Signed-off-by: clux <sszynrae@gmail.com>

* try unhacking windows ci for rustls

Signed-off-by: clux <sszynrae@gmail.com>

* invert the readme on tls

Signed-off-by: clux <sszynrae@gmail.com>

* disallow Client construction that requires auth when no tls stacks present

fixes #1275

Signed-off-by: clux <sszynrae@gmail.com>

* fix defaults in examples also

Signed-off-by: clux <sszynrae@gmail.com>

* properly fix example defaults

Signed-off-by: clux <sszynrae@gmail.com>

* also force TlsRequired when cluster_url is https scheme

Signed-off-by: clux <sszynrae@gmail.com>

---------

Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: Eirik A <sszynrae@gmail.com>
@github-project-automation github-project-automation bot moved this to Defining in Kube Roadmap Sep 13, 2023
@clux clux moved this from Defining to Done in Kube Roadmap Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working config Kube config related
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant