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

use kubeconfigs listed in KUBECONFIG env var #6295

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

adietish
Copy link
Contributor

@adietish adietish commented Aug 21, 2024

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

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

@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
@manusa manusa force-pushed the issue-6240 branch 2 times, most recently from 8d0a22b to 0c5ff29 Compare October 30, 2024 10:30
adietish and others added 2 commits November 8, 2024 10:11
* 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]>
@manusa manusa added this to the 7.0.0 milestone Nov 8, 2024 — with automated-tasks
Copy link

sonarcloud bot commented Nov 8, 2024

Copy link
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx!

@manusa manusa merged commit 0a8d711 into fabric8io:main Nov 8, 2024
20 checks passed
@adietish adietish deleted the issue-6240 branch November 11, 2024 11:13
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 this pull request may close these issues.

merge multiple files listed in KUBECONFIG environment var
3 participants