Skip to content

fix: support multi-file KUBECONFIG with three-way merge #970

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

Merged
yxxhero merged 2 commits into from
Apr 10, 2026
+131 −7
Merged

fix: support multi-file KUBECONFIG with three-way merge #970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89356c4
fix: support multi-file KUBECONFIG with three-way merge (#969)
yxxhero Apr 10, 2026
ddf0832
fix: remove unused envSettings global variable causing lint failure
Copilot Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions cmd/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"log"
"os"
"path/filepath"
"slices"
"strconv"
"strings"
Expand Down Expand Up @@ -112,8 +113,6 @@ This can be used to visualize what changes a helm upgrade will
perform.
`

var envSettings = cli.New()

func newChartCommand() *cobra.Command {
diff := diffCmd{
namespace: os.Getenv("HELM_NAMESPACE"),
Expand Down Expand Up @@ -297,11 +296,7 @@ func (d *diffCmd) runHelm3() error {
var actionConfig *action.Configuration
if d.threeWayMerge || d.takeOwnership {
actionConfig = new(action.Configuration)
localEnv := cli.New()
*localEnv = *envSettings
if d.kubeContext != "" {
localEnv.KubeContext = d.kubeContext
}
localEnv := prepareEnvSettings(d.kubeContext)
if err := actionConfig.Init(localEnv.RESTClientGetter(), localEnv.Namespace(), os.Getenv("HELM_DRIVER")); err != nil {
log.Fatalf("%+v", err)
}
Expand Down Expand Up @@ -409,3 +404,14 @@ func checkOwnership(d *diffCmd, resources kube.ResourceList, currentSpecs map[st
})
return newOwnedReleases, err
}

func prepareEnvSettings(kubeContext string) *cli.EnvSettings {
localEnv := cli.New()
if len(filepath.SplitList(localEnv.KubeConfig)) > 1 {
localEnv.KubeConfig = ""
}
if kubeContext != "" {
localEnv.KubeContext = kubeContext
}
return localEnv
}
118 changes: 118 additions & 0 deletions cmd/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cmd

import (
"os"
"path/filepath"
"testing"
)

Expand Down Expand Up @@ -63,3 +65,119 @@ func TestIsRemoteAccessAllowed(t *testing.T) {
})
}
}

func TestPrepareEnvSettings_MultiFileKubeconfig(t *testing.T) {
original := os.Getenv("KUBECONFIG")
defer os.Setenv("KUBECONFIG", original)

Comment on lines +69 to +72
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The test restores KUBECONFIG via original := os.Getenv(...) + defer os.Setenv(...), which can leave KUBECONFIG set to an empty string even if it was originally unset. Prefer t.Setenv (per subtest) or use os.LookupEnv and os.Unsetenv to restore the pre-test state accurately (and avoid ignoring os.Setenv errors).

Copilot uses AI. Check for mistakes.
cases := []struct {
name string
kubeconfig string
kubeContext string
wantKubeConfig string
wantKubeContext string
}{
{
name: "single file kubeconfig is preserved",
kubeconfig: "/path/to/config",
kubeContext: "",
wantKubeConfig: "/path/to/config",
wantKubeContext: "",
},
{
name: "multi-file kubeconfig is cleared",
kubeconfig: "/path/to/file1" + string(filepath.ListSeparator) + "/path/to/file2",
kubeContext: "",
wantKubeConfig: "",
wantKubeContext: "",
},
{
name: "multi-file kubeconfig with three files is cleared",
kubeconfig: "/a" + string(filepath.ListSeparator) + "/b" + string(filepath.ListSeparator) + "/c",
kubeContext: "",
wantKubeConfig: "",
wantKubeContext: "",
},
{
name: "empty kubeconfig is preserved",
kubeconfig: "",
kubeContext: "",
wantKubeConfig: "",
wantKubeContext: "",
},
{
name: "kube-context override is applied",
kubeconfig: "/path/to/config",
kubeContext: "my-context",
wantKubeConfig: "/path/to/config",
wantKubeContext: "my-context",
},
{
name: "multi-file kubeconfig with kube-context override",
kubeconfig: "/path/to/file1" + string(filepath.ListSeparator) + "/path/to/file2",
kubeContext: "my-context",
wantKubeConfig: "",
wantKubeContext: "my-context",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
os.Setenv("KUBECONFIG", tc.kubeconfig)

env := prepareEnvSettings(tc.kubeContext)

if env.KubeConfig != tc.wantKubeConfig {
t.Errorf("KubeConfig: got %q, want %q", env.KubeConfig, tc.wantKubeConfig)
}
if env.KubeContext != tc.wantKubeContext {
t.Errorf("KubeContext: got %q, want %q", env.KubeContext, tc.wantKubeContext)
}
})
}
}

func TestPrepareEnvSettings_ConfigFlagsPointToCorrectFields(t *testing.T) {
original := os.Getenv("KUBECONFIG")
defer os.Setenv("KUBECONFIG", original)

t.Run("config flags reflect kube-context override", func(t *testing.T) {
os.Setenv("KUBECONFIG", "/some/config")
env := prepareEnvSettings("my-override-context")

if env.KubeContext != "my-override-context" {
t.Errorf("env.KubeContext = %q, want %q", env.KubeContext, "my-override-context")
}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

Subtest name suggests it verifies ConfigFlags/RESTClientGetter wiring, but it only asserts env.KubeContext. To catch the original regression, also assert that the context used by the REST client loader reflects the override (e.g., env.ConfigFlags.Context points to the right value and/or the raw kubeconfig loader resolves the expected current context).

Suggested change
}
}
if env.ConfigFlags == nil {
t.Fatal("env.ConfigFlags is nil")
}
if env.ConfigFlags.Context == nil {
t.Fatal("env.ConfigFlags.Context is nil")
}
if got := *env.ConfigFlags.Context; got != "my-override-context" {
t.Errorf("env.ConfigFlags.Context = %q, want %q", got, "my-override-context")
}
getter := env.RESTClientGetter()
rawLoader := getter.ToRawKubeConfigLoader()
rawCfg, err := rawLoader.RawConfig()
if err != nil {
t.Fatalf("RawConfig() returned error: %v", err)
}
if rawCfg.CurrentContext != "my-override-context" {
t.Errorf("raw kubeconfig CurrentContext = %q, want %q", rawCfg.CurrentContext, "my-override-context")
}

Copilot uses AI. Check for mistakes.
})

t.Run("multi-file kubeconfig does not set ExplicitPath", func(t *testing.T) {
multiPath := "/tmp/file1" + string(filepath.ListSeparator) + "/tmp/file2"
os.Setenv("KUBECONFIG", multiPath)

env := prepareEnvSettings("")

if env.KubeConfig != "" {
t.Errorf("env.KubeConfig = %q, want empty string for multi-file KUBECONFIG", env.KubeConfig)
}

getter := env.RESTClientGetter()
rawConfig := getter.ToRawKubeConfigLoader()
loadingRules := rawConfig.ConfigAccess()

if loadingRules != nil {
if explicitPath := loadingRules.GetExplicitFile(); explicitPath != "" {
t.Errorf("ExplicitPath = %q, want empty string for multi-file KUBECONFIG", explicitPath)
}
}
})

t.Run("single file kubeconfig preserves ExplicitPath", func(t *testing.T) {
os.Setenv("KUBECONFIG", "/tmp/single-config")

env := prepareEnvSettings("")

if env.KubeConfig != "/tmp/single-config" {
t.Errorf("env.KubeConfig = %q, want %q", env.KubeConfig, "/tmp/single-config")
}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This subtest says it preserves ExplicitPath, but it only checks env.KubeConfig. Consider also asserting that env.RESTClientGetter().ToRawKubeConfigLoader().ConfigAccess().GetExplicitFile() equals the single kubeconfig path so the test actually validates ExplicitPath behavior.

Suggested change
}
}
getter := env.RESTClientGetter()
rawConfig := getter.ToRawKubeConfigLoader()
loadingRules := rawConfig.ConfigAccess()
if loadingRules == nil {
t.Fatal("ConfigAccess() = nil, want non-nil for single-file KUBECONFIG")
}
if explicitPath := loadingRules.GetExplicitFile(); explicitPath != "/tmp/single-config" {
t.Errorf("ExplicitPath = %q, want %q", explicitPath, "/tmp/single-config")
}

Copilot uses AI. Check for mistakes.
})
}
Loading