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

[Fix] - Kubeconfig merge needs user and cluster #59

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Ramilito
Copy link
Owner

@Ramilito Ramilito commented Feb 10, 2024

Description

When merging and not using kubeconfig env variable, the resulting cached files don't have cluster and user in them.
This PR adds those so they can be more complete.
Results:
default

Fixes # (issue)
#46 Fixes part of this

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Tested locally by using it for a while

@Ramilito Ramilito changed the title Fix/include user cluster in merge [Fix] - Kubeconfig merge needs user and cluster Feb 10, 2024
@Ramilito Ramilito requested a review from nicmr February 13, 2024 09:51
#[derive(Default, Clone, PartialEq, Debug, Serialize, Deserialize)]
pub struct Users {
#[serde(default)]
pub user: HashMap<String, Value>,
Copy link
Owner Author

Choose a reason for hiding this comment

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

Using hashmap here because we aren't interested in the structure and they can vary from providers (AWS,k3s,GKE)

@nicmr
Copy link
Collaborator

nicmr commented Feb 14, 2024

Hi, thanks for adding me as a reviewer 😊 I'm quite busy this week but I'll have a look this Saturday!

Copy link
Collaborator

@nicmr nicmr left a comment

Choose a reason for hiding this comment

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

Didn't quite manage Saturday 😅 I had a look and compared the generated cached kubeconfigs with and without the change, definitely a good step to make it possible to use them independently 👍
Code changes also look completely solid!

@Ramilito
Copy link
Owner Author

Ramilito commented Mar 7, 2024

Ive been running with this for a while and only noticed one issue, the kcd command doesn't permanently set it, I'm guessing it's because the merged files isn't known to kubectl.
Will try setting the --kubeconfig command dynamically and see if it helps

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.

None yet

2 participants