kubernetes-client use kubeconfigs listed in KUBECONFIG env var

fixes #6240

Description

Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] Feature (non-breaking change which adds functionality)
[x] Breaking change (fix or feature that would cause existing functionality to change
[ ] Chore (non-breaking change which doesn't affect codebase; test, version modification, documentation, etc.)

Checklist

[x] Code contributed by me aligns with current project license: Apache 2.0
[x] I Added CHANGELOG entry regarding this change
[x] I have implemented unit tests to cover my changes
[x] I have added/updated the javadocs and other documentation accordingly
[x] No new bugs, code smells, etc. in SonarCloud report
[x] I tested my code in Kubernetes
[x] I tested my code in OpenShift

Aug 21 '24 17:08 adietish

Could you please run mvn spotless:apply to fix style failures?

Aug 22 '24 08:08 rohanKanojia

@manusa, @rohanKanojia: can you please have a look at my implementation and provide feedback? thanks!

The only place I currently consider a change is Config#getFileWithAuthInfo(name). Loading and parsing of the kubeconfig files happens there on each call. An improvement could be to store files & model/configs when configuring the client/config instead (and no further parsing would have to happen later).

PS. DefaultMockServerTest seem to fail in github checks even though they're fine for me locally.

Aug 30 '24 13:08 adietish

PS. DefaultMockServerTest seem to fail in github checks even though they're fine for me locally.

Yes, this is a known issue https://github.com/fabric8io/kubernetes-client/issues/6228 Marc had raised a PR to fix it but looks like it's not properly fixed it.

Aug 30 '24 16:08 rohanKanojia

@rohanKanojia, @manusa: I now only (lazily) parse kubeconfig files once. Please review.

Sep 02 '24 13:09 adietish

build fails again on DefaultMockServerWebSocketTest:

Error:  Failures: 
Error:    DefaultMockServerWebSocketTest.andUpgradeToWebSocket, with configured events, should emit onClose when done:106->andUpgradeToWebSocket, with configured events, should emit onClose when done_closure4:-1->andUpgradeToWebSocket, with configured events, should emit onClose when done_closure4:107 Condition failed with Exception:

Sep 03 '24 14:09 adietish

A unit test seems to be failing on windows :

[INFO] 
Error:  Errors: 
Error:    KubernetesClientImplTest.shouldInstantiateClientUsingSerializeDeserialize:132 � KubernetesClient An error has occurred.
[INFO] 
Error:  Tests run: 240, Failures: 0, Errors: 1, Skipped: 6

Sep 04 '24 04:09 rohanKanojia

@rohanKanojia fixed it. The new class KubeConfigFile needed jackson annotations to allow deserialization.

Sep 04 '24 12:09 adietish

@rohanKanojia: fails again on DefaultMockServerWebSocketTest. And then Sonar complains about a possible NPE in fileName = fileNames[0];. It is wrong, it doesn't get the check that prevents it:

    String[] fileNames = getKubeconfigFilenames();
    if (Utils.isNotNullOrEmpty(fileNames)) {
      fileName = fileNames[0];

Please review, thanks!

Sep 05 '24 08:09 adietish

@rohanKanojia, @manusa: the negative sonar status is imho erroneous. It overlooks a null-check and assertions that are present in a test case (parametrized test), complains about (intentionally introduced and kept) deprecation and blames too many (required) parameters in SundrioConfig constructor.

Sep 09 '24 10:09 adietish

@rohanKanojia: fails again on DefaultMockServerWebSocketTest.

Sep 17 '24 12:09 adietish

@rohanKanojia: fails again on DefaultMockServerWebSocketTest.

This is a known failure.

We'll try to merge this PR once we merge others that might create conflicts on the Config class but need to go first because we need them for a 6.13.4 release.

Sep 18 '24 04:09 manusa

@rohanKanojia, @manusa: could we please progress on this PR, please as it is important for us at intellij-kubernetes?

Please check previous comment.

Note also that this one will only be available in v7.0.0 we can't backport this to 6.13. If it's really urgent or critical we can try to do a 6.14, but that will delay even more the 7.0.0 release. If you do need the backport + 6.14 release, please say so in order to plan it accordingly.

Sep 18 '24 08:09 manusa

@manusa: if possible I'd love this to be available in 6.14 but I can live with it only being available with 7.0.0. The reasoning is that we'd need all our plugins to use the same kubernetes-client (limitations in the jetbrains platform). I assume bumping all these would be substantial work and delay our releases substantially (we're currently on 6.12).

Sep 19 '24 21:09 adietish

I assume bumping all these would be substantial work and delay our releases substantially (we're currently on 6.12).

I don't think this will be the case for v7.0.0 and your use-case. It should be fairly straightforward to bump to the next major and should not require additional work.

You can do a quick check by bumping to 7.0-SNAOSHOT, at this point I don't think this will even require any code changes for the plugins.

Sep 20 '24 10:09 manusa

@manusa, @rohanKanojia: do you have any updates regarding this PR and the release of 7.0.0 please?

Oct 04 '24 12:10 adietish

Hi @adietish

We had a few hiccups with the new model generation (#6130) regarding the OpenShift types which have delayed us around 1 week or more. The problems are now mostly solved, so we should be able to proceed with the rest of tasks in #5778.

Regarding this PR I hope I can merge it by the end of this week so that you can at least try it in the 7.0-SNAPSHOT release. Please take into account that this PR is dealing with one of the most critical and delicate classes and functionalities of the client, so we have to be extra-careful when merging and implementing any additional test to ensure that the client retains its original behavior while adding the new functionality.

Oct 07 '24 14:10 manusa

Hi @manusa

thx for your extensive explanations, makes perfect sense.

Regarding this PR I hope I can merge it by the end of this week so that you can at least try it in the 7.0-SNAPSHOT release. Please take into account that this PR is dealing with one of the most critical and delicate classes and functionalities of the client, so we have to be extra-careful when merging and implementing any additional test to ensure that the client retains its original behavior while adding the new functionality.

Fully agree. I already did some additional tests, maybe I can have one more look into it to see if there's more I can assert.

Oct 08 '24 08:10 adietish

Hi @adietish Before merging this PR I'd like to add some changes on top. Are you done with your work? Is it OK if I take over the PR and add a few commits on top of your branch?

Oct 22 '24 12:10 manusa

Hi @adietish Before merging this PR I'd like to add some changes on top. Are you done with your work? Is it OK if I take over the PR and add a few commits on top of your branch?

@manusa Yes I'm done and confident that stuff works as expected. I had the changes reviewed/tested in our plugin, working as expected for our usecases. Go ahead and take it. Looking forward to have this merged :)

Oct 28 '24 10:10 adietish

@manusa Yes I'm done and confident that stuff works as expected. I had the changes reviewed/tested in our plugin, working as expected for our usecases. Go ahead and take it. Looking forward to have this merged :)

Cool, I'm taking it over now. Let's see if we can merge it soon.

Oct 29 '24 12:10 manusa