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

Refine what is possible via withRequestConfig #4708

Closed
shawkins opened this issue Dec 22, 2022 · 1 comment · Fixed by #4837
Closed

Refine what is possible via withRequestConfig #4708

shawkins opened this issue Dec 22, 2022 · 1 comment · Fixed by #4837
Assignees
Milestone

Comments

@shawkins
Copy link
Contributor

The RequestConfig object exposes quite a few timeouts, authentication properties, and other things that we are allowing to be set on a per request basis if one starts with NamespacedKubernetesClient.withRequestConfig.

There are several issues with this:

  • in the code it's effectively part of the Config, which is otherwise immutable. That is confusing to deal with and requires extra logic to pass around to the interceptors.
  • passing alternative authentication is confusing to the logic in the interceptors. Although that may be improved by fix #4698: refining authentication mechanisms to be clearer #4702.

It would be good to re-assess what about this feature is actually needed, and if it should just be part of the regular dsl - that is instead of client.adapt(NamespacedKubernetesClient.class).withRequestConfig(new RequestConfigBuilder().withImpersonateUsername("name").build()).(c -> ...)

should we just offer

client.withImpersonateUsername("name")...

With methods for whatever seems still useful.

@shawkins
Copy link
Contributor Author

shawkins commented Feb 3, 2023

The alternative to

client.adapt(NamespacedKubernetesClient.class).withRequestConfig(new RequestConfigBuilder().withImpersonateUsername("name").build()).(c -> ...)

is currently how it's implemented under the covers

client.newClient(new RequestConfigBuilder().withImpersonateUsername("name").build()).adapt(KubernetesClient.class)

The javadocs are incorrect for the newClient method - I'll correct that as part of this issue. It no longer needs to create a fully new instance now that we are passing the augmented requestconfig into the interceptors.

shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 6, 2023
also trying to clean up the logic related to how the config is (or not)
modified
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 6, 2023
also trying to clean up the logic related to how the config is (or not)
modified
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 6, 2023
also trying to clean up the logic related to how the config is (or not)
modified
@manusa manusa added this to the 6.5.0 milestone Feb 22, 2023
manusa added a commit that referenced this issue Feb 22, 2023
fix #4708 refining what is possible via the requestconfig
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants