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.

Sign up for GitHub

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

Jump to bottom

use kubeconfigs listed in KUBECONFIG env var #6295

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

use kubeconfigs listed in KUBECONFIG env var #6295

wants to merge 1 commit into from

Conversation

adietish
Copy link
Contributor

@adietish adietish commented Aug 21, 2024 •
edited by manusa
Loading

Description

Fixes #6240

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • 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

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

@adietish adietish changed the title Issue 6240 use kubeconfigs listed in KUBECONFIG env var Aug 21, 2024
@rohanKanojia
Copy link
Member

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

@adietish adietish force-pushed the issue-6240 branch 25 times, most recently from 7feddd2 to 81f1f83 Compare August 29, 2024 08:06
@adietish adietish force-pushed the issue-6240 branch 3 times, most recently from 7d4eddc to 1755e3e Compare August 29, 2024 17:54
@adietish
Copy link
Contributor Author

adietish commented Sep 9, 2024 •
edited
Loading

@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.

@adietish adietish mentioned this pull request Sep 12, 2024
Closed
@adietish adietish force-pushed the issue-6240 branch 3 times, most recently from 7da63b2 to 818ee41 Compare September 17, 2024 11:56
@adietish
Copy link
Contributor Author

@rohanKanojia: fails again on DefaultMockServerWebSocketTest.

@manusa
Copy link
Member

manusa commented Sep 18, 2024

@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.

@manusa
Copy link
Member

manusa commented Sep 18, 2024

@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.

@adietish
Copy link
Contributor Author

@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).

@manusa
Copy link
Member

manusa commented Sep 20, 2024

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.

@adietish adietish force-pushed the issue-6240 branch 3 times, most recently from 9e41115 to 1b1d4f4 Compare September 26, 2024 09:25
@adietish
Copy link
Contributor Author

adietish commented Oct 4, 2024

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

@manusa
Copy link
Member

manusa commented Oct 7, 2024

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.

@adietish
Copy link
Contributor Author

adietish commented Oct 8, 2024

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.

@manusa
Copy link
Member

manusa commented Oct 22, 2024

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?

@adietish
Copy link
Contributor Author

adietish commented Oct 28, 2024 •
edited
Loading

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 :)

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@manusa
Copy link
Member

manusa commented Oct 29, 2024

@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.

@manusa manusa self-assigned this Oct 29, 2024
@adietish @manusa
use kubeconfigs listed in KUBECONFIG env var (fabric8io#6240)
0c5ff29 
* update token in file listed in KUBECONFIG env var (fabric8io#6240)
* only parse configs once (fabric8io#6240)
* update file with auth info when merging authinfos
* parametrized KubeConfigUtilsTest for #hasAuthInfoNamed
* expose Config#getFileWithCurrentContext & #getFileWithContext to consumers

Signed-off-by: Andre Dietisheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manusa manusa Awaiting requested review from manusa manusa is a code owner

@oscerd oscerd Awaiting requested review from oscerd oscerd is a code owner

@shawkins shawkins Awaiting requested review from shawkins shawkins is a code owner

@rohanKanojia rohanKanojia Awaiting requested review from rohanKanojia rohanKanojia is a code owner

Assignees

@manusa manusa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

merge multiple files listed in KUBECONFIG environment var
3 participants
@adietish @rohanKanojia @manusa