From 28662b3ea466939517fb1cdab3b4ac6e3de1babc Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Wed, 22 Apr 2026 23:53:22 +0530 Subject: [PATCH 01/10] fall back to kubeconfig loader behavior (exec plugin capable) Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 110 ++++++++++++++------------- 1 file changed, 58 insertions(+), 52 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index fc2e357e2..437559fe0 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -54,12 +54,10 @@ const ( Latest = ">0.0.0-0" ) -var ( - // downloadLocaton is the location where downloaded helm charts - // will be stored. os.TempDir will ensure that the path is cross - // platform - downloadLocation = os.TempDir() -) +// downloadLocaton is the location where downloaded helm charts +// will be stored. os.TempDir will ensure that the path is cross +// platform +var downloadLocation = os.TempDir() // HelmIndex holds the index.yaml data in the struct format type HelmIndex struct { @@ -421,57 +419,64 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config // KubeConfig setup kubeConfig := genericclioptions.NewConfigFlags(false) - // Set KubeConfig to DevNull to prevent read from local kubeconfig - // to prevent conflicts between "data" and "files" properties (CAFile, CAData and KeyFile, KeyData) - // ConfigFlags only allows setting CAFile, KeyFile but not CAData, KeyData. - // When the library reads the original kubeconfig containing cert data / key data AND we specify cert file / key file, these configurations conflict - devNull := os.DevNull - kubeConfig.KubeConfig = &devNull - kubeConfig.APIServer = &c.RestConfig.Host - kubeConfig.BearerToken = &c.RestConfig.BearerToken - kubeConfig.Insecure = &c.RestConfig.Insecure - - // Set username and password for basic auth if available - if c.RestConfig.Username != "" { - kubeConfig.Username = &c.RestConfig.Username - } - if c.RestConfig.Password != "" { - kubeConfig.Password = &c.RestConfig.Password - } - - // Only set CA file if not running in insecure mode - if !c.RestConfig.Insecure { - if len(c.RestConfig.CAData) > 0 { - caFileName, err := setDataAndReturnFilename(c.RestConfig.CAData) - if err != nil { - cleanup() // Clean up any files created so far - return nil, nil, err + + // Exec-auth kubeconfigs (eg: aws eks get-token) typically do not have a static bearer token. + // In that case, do not force /dev/null; let client-go load kubeconfig and execute the auth plugin. + useKubeconfigAuth := c.RestConfig.ExecProvider != nil && c.RestConfig.BearerToken == "" + + if !useKubeconfigAuth { + // Set KubeConfig to DevNull to prevent read from local kubeconfig + // to prevent conflicts between "data" and "files" properties (CAFile, CAData and KeyFile, KeyData) + // ConfigFlags only allows setting CAFile, KeyFile but not CAData, KeyData. + // When the library reads the original kubeconfig containing cert data / key data AND we specify cert file / key file, these configurations conflict + devNull := os.DevNull + kubeConfig.KubeConfig = &devNull + kubeConfig.APIServer = &c.RestConfig.Host + kubeConfig.BearerToken = &c.RestConfig.BearerToken + kubeConfig.Insecure = &c.RestConfig.Insecure + + // Set username and password for basic auth if available + if c.RestConfig.Username != "" { + kubeConfig.Username = &c.RestConfig.Username + } + if c.RestConfig.Password != "" { + kubeConfig.Password = &c.RestConfig.Password + } + + // Only set CA file if not running in insecure mode + if !c.RestConfig.Insecure { + if len(c.RestConfig.CAData) > 0 { + caFileName, err := setDataAndReturnFilename(c.RestConfig.CAData) + if err != nil { + cleanup() // Clean up any files created so far + return nil, nil, err + } + tempFiles = append(tempFiles, caFileName) + kubeConfig.CAFile = &caFileName } - tempFiles = append(tempFiles, caFileName) - kubeConfig.CAFile = &caFileName } - } - // Set client certificate data if available - if len(c.RestConfig.CertData) > 0 { - certFileName, err := setDataAndReturnFilename(c.RestConfig.CertData) - if err != nil { - cleanup() - return nil, nil, err + // Set client certificate data if available + if len(c.RestConfig.CertData) > 0 { + certFileName, err := setDataAndReturnFilename(c.RestConfig.CertData) + if err != nil { + cleanup() + return nil, nil, err + } + tempFiles = append(tempFiles, certFileName) + kubeConfig.CertFile = &certFileName } - tempFiles = append(tempFiles, certFileName) - kubeConfig.CertFile = &certFileName - } - // Set client key data if available - if len(c.RestConfig.KeyData) > 0 { - keyFileName, err := setDataAndReturnFilename(c.RestConfig.KeyData) - if err != nil { - cleanup() // Clean up any files created so far - return nil, nil, err + // Set client key data if available + if len(c.RestConfig.KeyData) > 0 { + keyFileName, err := setDataAndReturnFilename(c.RestConfig.KeyData) + if err != nil { + cleanup() // Clean up any files created so far + return nil, nil, err + } + tempFiles = append(tempFiles, keyFileName) + kubeConfig.KeyFile = &keyFileName } - tempFiles = append(tempFiles, keyFileName) - kubeConfig.KeyFile = &keyFileName } actionConfig := new(action.Configuration) @@ -554,7 +559,8 @@ func createHelmPathFromHelmChartLocation(loc HelmChartLocation) (string, error) getter.Provider{ Schemes: []string{"http", "https"}, New: getter.NewHTTPGetter, - }}, + }, + }, ) if err != nil { return "", ErrApplyHelmChart(err) From bcdca61089868f6fc30ff90058e19a23e93a04f3 Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Sun, 26 Apr 2026 22:29:58 +0530 Subject: [PATCH 02/10] serializing the in-memory kubeconfig to a temp file Signed-off-by: reaper8055 Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 47 ++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 437559fe0..95e99cede 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -15,6 +15,8 @@ import ( "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) // HelmDriver is the type for helm drivers @@ -422,9 +424,9 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config // Exec-auth kubeconfigs (eg: aws eks get-token) typically do not have a static bearer token. // In that case, do not force /dev/null; let client-go load kubeconfig and execute the auth plugin. - useKubeconfigAuth := c.RestConfig.ExecProvider != nil && c.RestConfig.BearerToken == "" + useKubeConfigAuth := c.RestConfig.ExecProvider != nil && c.RestConfig.BearerToken == "" - if !useKubeconfigAuth { + if !useKubeConfigAuth { // Set KubeConfig to DevNull to prevent read from local kubeconfig // to prevent conflicts between "data" and "files" properties (CAFile, CAData and KeyFile, KeyData) // ConfigFlags only allows setting CAFile, KeyFile but not CAData, KeyData. @@ -479,6 +481,47 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config } } + const ( + clusterName = "meshery-cluster" + authInfo = "meshkit-helm-user" + ) + + // eksKubeConfig is just placeholder, will change these changes are reviewed + eksKubeConfig := clientcmdapi.NewConfig() + + eksKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ + Server: c.RestConfig.Host, + TLSServerName: c.RestConfig.ServerName, + InsecureSkipTLSVerify: c.RestConfig.Insecure, + CertificateAuthority: c.RestConfig.CAFile, + CertificateAuthorityData: c.RestConfig.CAData, + } + + eksKubeConfig.AuthInfos[clusterName] = &clientcmdapi.AuthInfo{ + Exec: c.RestConfig.ExecProvider, + AuthProvider: c.RestConfig.AuthProvider, + } + + eksKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ + Cluster: clusterName, + AuthInfo: authInfo, + } + + eksKubeConfig.CurrentContext = clusterName + + configBytes, err := clientcmd.Write(*eksKubeConfig) + if err != nil { + return nil, nil, fmt.Errorf("failed to write kubeconfig %v", err) + } + + configFile, err := setDataAndReturnFilename(configBytes) + if err != nil { + return nil, nil, fmt.Errorf("failed to get kubeconfig file %v", err) + } + tempFiles = append(tempFiles, configFile) + + kubeConfig.KubeConfig = &configFile + actionConfig := new(action.Configuration) if err := actionConfig.Init(kubeConfig, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { cleanup() // Clean up any files created so far From b7763e8eecbe56a4328330131d74dc0949311d0d Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Mon, 27 Apr 2026 19:38:39 +0530 Subject: [PATCH 03/10] rename eksKubeConfig to helmKubeConfig Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 95e99cede..4c055d032 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -486,10 +486,9 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config authInfo = "meshkit-helm-user" ) - // eksKubeConfig is just placeholder, will change these changes are reviewed - eksKubeConfig := clientcmdapi.NewConfig() + helmKubeConfig := clientcmdapi.NewConfig() - eksKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ + helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ Server: c.RestConfig.Host, TLSServerName: c.RestConfig.ServerName, InsecureSkipTLSVerify: c.RestConfig.Insecure, @@ -497,19 +496,20 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config CertificateAuthorityData: c.RestConfig.CAData, } - eksKubeConfig.AuthInfos[clusterName] = &clientcmdapi.AuthInfo{ + helmKubeConfig.AuthInfos[clusterName] = &clientcmdapi.AuthInfo{ Exec: c.RestConfig.ExecProvider, AuthProvider: c.RestConfig.AuthProvider, } - eksKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ + helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ Cluster: clusterName, AuthInfo: authInfo, } - eksKubeConfig.CurrentContext = clusterName + // explicitly setting kube context may not be required + helmKubeConfig.CurrentContext = clusterName - configBytes, err := clientcmd.Write(*eksKubeConfig) + configBytes, err := clientcmd.Write(*helmKubeConfig) if err != nil { return nil, nil, fmt.Errorf("failed to write kubeconfig %v", err) } From d0efa856e15ae59f1b43b545987146657c660ae7 Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Mon, 4 May 2026 00:52:28 +0530 Subject: [PATCH 04/10] fixed logic error where kubeConfig.KubeConfig was never unset Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 67 ++++++++++++++-------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 4c055d032..7aca91f7f 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -479,48 +479,49 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config tempFiles = append(tempFiles, keyFileName) kubeConfig.KeyFile = &keyFileName } - } + } else { - const ( - clusterName = "meshery-cluster" - authInfo = "meshkit-helm-user" - ) + const ( + clusterName = "meshery-cluster" + authInfo = "meshkit-helm-user" + ) - helmKubeConfig := clientcmdapi.NewConfig() + helmKubeConfig := clientcmdapi.NewConfig() - helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ - Server: c.RestConfig.Host, - TLSServerName: c.RestConfig.ServerName, - InsecureSkipTLSVerify: c.RestConfig.Insecure, - CertificateAuthority: c.RestConfig.CAFile, - CertificateAuthorityData: c.RestConfig.CAData, - } + helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ + Server: c.RestConfig.Host, + TLSServerName: c.RestConfig.ServerName, + InsecureSkipTLSVerify: c.RestConfig.Insecure, + CertificateAuthority: c.RestConfig.CAFile, + CertificateAuthorityData: c.RestConfig.CAData, + } - helmKubeConfig.AuthInfos[clusterName] = &clientcmdapi.AuthInfo{ - Exec: c.RestConfig.ExecProvider, - AuthProvider: c.RestConfig.AuthProvider, - } + helmKubeConfig.AuthInfos[authInfo] = &clientcmdapi.AuthInfo{ + Exec: c.RestConfig.ExecProvider, + AuthProvider: c.RestConfig.AuthProvider, + } - helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ - Cluster: clusterName, - AuthInfo: authInfo, - } + helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ + Cluster: clusterName, + AuthInfo: authInfo, + } - // explicitly setting kube context may not be required - helmKubeConfig.CurrentContext = clusterName + // explicitly setting kube context may not be required + helmKubeConfig.CurrentContext = clusterName - configBytes, err := clientcmd.Write(*helmKubeConfig) - if err != nil { - return nil, nil, fmt.Errorf("failed to write kubeconfig %v", err) - } + configBytes, err := clientcmd.Write(*helmKubeConfig) + if err != nil { + return nil, nil, fmt.Errorf("failed to write kubeconfig %v", err) + } - configFile, err := setDataAndReturnFilename(configBytes) - if err != nil { - return nil, nil, fmt.Errorf("failed to get kubeconfig file %v", err) - } - tempFiles = append(tempFiles, configFile) + configFile, err := setDataAndReturnFilename(configBytes) + if err != nil { + return nil, nil, fmt.Errorf("failed to get kubeconfig file %v", err) + } + tempFiles = append(tempFiles, configFile) - kubeConfig.KubeConfig = &configFile + kubeConfig.KubeConfig = &configFile + } actionConfig := new(action.Configuration) if err := actionConfig.Init(kubeConfig, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { From 54d18c793ef2a9b21588246eaad18007d19beb26 Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Sun, 17 May 2026 19:53:42 +0530 Subject: [PATCH 05/10] refactor: added setupKubeConfig method Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 105 +++++++++++----------- utils/kubernetes/apply-helm-chart_test.go | 29 ++++++ 2 files changed, 84 insertions(+), 50 deletions(-) create mode 100644 utils/kubernetes/apply-helm-chart_test.go diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 7aca91f7f..99bdfe96e 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -241,7 +241,7 @@ type ApplyHelmChartConfig struct { // }, // OverrideValues: vals, // }) -func (client *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { +func (c *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { setupDefaults(&cfg) if err := setupChartVersion(&cfg); err != nil { @@ -264,8 +264,14 @@ func (client *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { return ErrApplyHelmChart(err) } - actionConfig, cleanup, err := createHelmActionConfig(client, cfg) + kubeConfig, cleanup, err := c.setupKubeConfig() if err != nil { + cleanup() + return err + } + actionConfig, err := c.createHelmActionConfig(cfg, kubeConfig) + if err != nil { + cleanup() return ErrApplyHelmChart(err) } defer cleanup() @@ -408,10 +414,19 @@ func checkIfInstallable(ch *chart.Chart) error { } // createHelmActionConfig generates the actionConfig with the appropriate defaults -func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Configuration, func(), error) { +func (c *Client) createHelmActionConfig(cfg ApplyHelmChartConfig, kubeConfig *genericclioptions.ConfigFlags) (*action.Configuration, error) { // Set the environment variable needed by the Init methods _ = os.Setenv("HELM_DRIVER_SQL_CONNECTION_STRING", cfg.SQLConnectionString) + actionConfig := new(action.Configuration) + if err := actionConfig.Init(kubeConfig, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { + return nil, ErrApplyHelmChart(err) + } + + return actionConfig, nil +} + +func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), error) { var tempFiles []string cleanup := func() { for _, f := range tempFiles { @@ -450,8 +465,7 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config if len(c.RestConfig.CAData) > 0 { caFileName, err := setDataAndReturnFilename(c.RestConfig.CAData) if err != nil { - cleanup() // Clean up any files created so far - return nil, nil, err + return nil, cleanup, err } tempFiles = append(tempFiles, caFileName) kubeConfig.CAFile = &caFileName @@ -462,8 +476,7 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config if len(c.RestConfig.CertData) > 0 { certFileName, err := setDataAndReturnFilename(c.RestConfig.CertData) if err != nil { - cleanup() - return nil, nil, err + return nil, cleanup, err } tempFiles = append(tempFiles, certFileName) kubeConfig.CertFile = &certFileName @@ -473,63 +486,55 @@ func createHelmActionConfig(c *Client, cfg ApplyHelmChartConfig) (*action.Config if len(c.RestConfig.KeyData) > 0 { keyFileName, err := setDataAndReturnFilename(c.RestConfig.KeyData) if err != nil { - cleanup() // Clean up any files created so far - return nil, nil, err + return nil, cleanup, err } tempFiles = append(tempFiles, keyFileName) kubeConfig.KeyFile = &keyFileName } - } else { - - const ( - clusterName = "meshery-cluster" - authInfo = "meshkit-helm-user" - ) - - helmKubeConfig := clientcmdapi.NewConfig() + return kubeConfig, cleanup, nil + } - helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ - Server: c.RestConfig.Host, - TLSServerName: c.RestConfig.ServerName, - InsecureSkipTLSVerify: c.RestConfig.Insecure, - CertificateAuthority: c.RestConfig.CAFile, - CertificateAuthorityData: c.RestConfig.CAData, - } + const ( + clusterName = "meshery-cluster" + authInfo = "meshkit-helm-user" + ) - helmKubeConfig.AuthInfos[authInfo] = &clientcmdapi.AuthInfo{ - Exec: c.RestConfig.ExecProvider, - AuthProvider: c.RestConfig.AuthProvider, - } + helmKubeConfig := clientcmdapi.NewConfig() - helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ - Cluster: clusterName, - AuthInfo: authInfo, - } + helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ + Server: c.RestConfig.Host, + TLSServerName: c.RestConfig.ServerName, + InsecureSkipTLSVerify: c.RestConfig.Insecure, + CertificateAuthority: c.RestConfig.CAFile, + CertificateAuthorityData: c.RestConfig.CAData, + } - // explicitly setting kube context may not be required - helmKubeConfig.CurrentContext = clusterName + helmKubeConfig.AuthInfos[authInfo] = &clientcmdapi.AuthInfo{ + Exec: c.RestConfig.ExecProvider, + AuthProvider: c.RestConfig.AuthProvider, + } - configBytes, err := clientcmd.Write(*helmKubeConfig) - if err != nil { - return nil, nil, fmt.Errorf("failed to write kubeconfig %v", err) - } + helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ + Cluster: clusterName, + AuthInfo: authInfo, + } - configFile, err := setDataAndReturnFilename(configBytes) - if err != nil { - return nil, nil, fmt.Errorf("failed to get kubeconfig file %v", err) - } - tempFiles = append(tempFiles, configFile) + // explicitly setting kube context may not be required + helmKubeConfig.CurrentContext = clusterName - kubeConfig.KubeConfig = &configFile + configBytes, err := clientcmd.Write(*helmKubeConfig) + if err != nil { + return nil, cleanup, fmt.Errorf("failed to write kubeconfig %v", err) } - actionConfig := new(action.Configuration) - if err := actionConfig.Init(kubeConfig, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { - cleanup() // Clean up any files created so far - return nil, nil, ErrApplyHelmChart(err) + configFile, err := setDataAndReturnFilename(configBytes) + if err != nil { + return nil, cleanup, fmt.Errorf("failed to get kubeconfig file %v", err) } + tempFiles = append(tempFiles, configFile) - return actionConfig, cleanup, nil + kubeConfig.KubeConfig = &configFile + return kubeConfig, cleanup, nil } // Populates a file in temp directory with the passed data and returns the filename @@ -693,7 +698,7 @@ func (helmEntries HelmEntries) GetEntryWithAppVersion(entry, appVersion string) return HelmEntryMetadata{}, false } -// GetEntryWithAppVersion takes in the entry name and the appversion and returns the corresponding +// GetEntryWithChartVersion takes in the entry name and the appversion and returns the corresponding // metadata for the parameters if it exists func (helmEntries HelmEntries) GetEntryWithChartVersion(entry, chartVersion string) (HelmEntryMetadata, bool) { hem, ok := helmEntries[entry] diff --git a/utils/kubernetes/apply-helm-chart_test.go b/utils/kubernetes/apply-helm-chart_test.go new file mode 100644 index 000000000..96c1015e8 --- /dev/null +++ b/utils/kubernetes/apply-helm-chart_test.go @@ -0,0 +1,29 @@ +package kubernetes + +import ( + "testing" + // "k8s.io/cli-runtime/pkg/genericclioptions" + // "k8s.io/client-go/rest" +) + +func Test_setupKubeConfig(t *testing.T) { + // tests := []struct { + // name string + // client *Client + // wantKubeConfi *genericclioptions.ConfigFlags + // wantCleanupFunc func() + // wantErr error + // }{ + // { + // name: "use exec auth in kubeconfig", + // client: &Client{ + // RestConfig: rest.Config{ + // ExecProvider: nil, + // BearerToken: "", + // }, + // }, + // wantKubeConfi: *genericclioptions.ConfigFlags{ + // }, + // }, + // } +} From 42b9046dd8caf1d6b48c6d1cef466fc002ed6017 Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Sun, 17 May 2026 22:21:58 +0530 Subject: [PATCH 06/10] added test for setupKubeConfig method Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 17 +- utils/kubernetes/apply-helm-chart_test.go | 736 +++++++++++++++++++++- 2 files changed, 727 insertions(+), 26 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 99bdfe96e..41c025105 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -61,6 +61,13 @@ const ( // platform var downloadLocation = os.TempDir() +var ( + // writeKubeConfig marshals kubeconfig bytes. Kept as a variable to enable deterministic test stubs. + writeKubeConfig = clientcmd.Write + // writeTempData writes byte payloads to temp files. Kept as a variable to enable deterministic test stubs. + writeTempData = setDataAndReturnFilename +) + // HelmIndex holds the index.yaml data in the struct format type HelmIndex struct { APIVersion string `yaml:"apiVersion"` @@ -463,7 +470,7 @@ func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), erro // Only set CA file if not running in insecure mode if !c.RestConfig.Insecure { if len(c.RestConfig.CAData) > 0 { - caFileName, err := setDataAndReturnFilename(c.RestConfig.CAData) + caFileName, err := writeTempData(c.RestConfig.CAData) if err != nil { return nil, cleanup, err } @@ -474,7 +481,7 @@ func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), erro // Set client certificate data if available if len(c.RestConfig.CertData) > 0 { - certFileName, err := setDataAndReturnFilename(c.RestConfig.CertData) + certFileName, err := writeTempData(c.RestConfig.CertData) if err != nil { return nil, cleanup, err } @@ -484,7 +491,7 @@ func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), erro // Set client key data if available if len(c.RestConfig.KeyData) > 0 { - keyFileName, err := setDataAndReturnFilename(c.RestConfig.KeyData) + keyFileName, err := writeTempData(c.RestConfig.KeyData) if err != nil { return nil, cleanup, err } @@ -522,12 +529,12 @@ func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), erro // explicitly setting kube context may not be required helmKubeConfig.CurrentContext = clusterName - configBytes, err := clientcmd.Write(*helmKubeConfig) + configBytes, err := writeKubeConfig(*helmKubeConfig) if err != nil { return nil, cleanup, fmt.Errorf("failed to write kubeconfig %v", err) } - configFile, err := setDataAndReturnFilename(configBytes) + configFile, err := writeTempData(configBytes) if err != nil { return nil, cleanup, fmt.Errorf("failed to get kubeconfig file %v", err) } diff --git a/utils/kubernetes/apply-helm-chart_test.go b/utils/kubernetes/apply-helm-chart_test.go index 96c1015e8..c9565ab8d 100644 --- a/utils/kubernetes/apply-helm-chart_test.go +++ b/utils/kubernetes/apply-helm-chart_test.go @@ -1,29 +1,723 @@ package kubernetes import ( + "errors" + "os" + "strings" "testing" - // "k8s.io/cli-runtime/pkg/genericclioptions" - // "k8s.io/client-go/rest" + + "k8s.io/client-go/rest" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) +// Test design note: +// +// We intentionally assert behavior-level invariants for setupKubeConfig() +// instead of doing a full DeepEqual on *genericclioptions.ConfigFlags for the +// following reasons +// +// 1. the function produces runtime-generated artifacts (temp file paths) +// for CA/Cert/Key material. Those paths are nondeterministic by design, so exact +// object equality would make tests brittle and OS/environment-dependent. +// +// 2. the function returns a cleanup closure. Function values are not +// meaningfully comparable for equality in Go, so "exact output object" checking +// is not a stable assertion model for this API shape. +// +// 3. the real contract of this branch is not "exact struct bytes match"; +// it is: select non-exec mode, force KubeConfig=/dev/null, propagate auth/TLS +// fields correctly, create material files only when required, and provide a +// cleanup hook that removes created files. +// +// This test therefore checks deterministic outcomes that represent that contract: +// field propagation, branch selection, file-presence/file-absence expectations, +// and cleanup side effects. +// +// If we later introduce dependency injection for temp-file creation, we can add +// stricter deterministic assertions (including exact file names/paths) without +// changing the behavior contract validated here. func Test_setupKubeConfig(t *testing.T) { - // tests := []struct { - // name string - // client *Client - // wantKubeConfi *genericclioptions.ConfigFlags - // wantCleanupFunc func() - // wantErr error - // }{ - // { - // name: "use exec auth in kubeconfig", - // client: &Client{ - // RestConfig: rest.Config{ - // ExecProvider: nil, - // BearerToken: "", - // }, - // }, - // wantKubeConfi: *genericclioptions.ConfigFlags{ - // }, - // }, - // } + type ( + branchKind string + pathKind string + // only for deterministic error injection in tests + failPoint string + ) + + const ( + branchNonExec branchKind = "non-exec" + branchExec branchKind = "exec" + pathSkip pathKind = "skip" + pathNil pathKind = "nil" + pathDevNull pathKind = "devnull" + pathTempFile pathKind = "tempFile" + + // only for deterministic error injection in tests + failNone failPoint = "" + failCADataTempFile failPoint = "ca-tempfile" + failCertDataTempFile failPoint = "cert-tempfile" + failKeyDataTempFile failPoint = "key-tempfile" + failExecConfigSerialize failPoint = "exec-config-serialize" + failExecConfigTempFile failPoint = "exec-config-tempfile" + ) + + type opt[T any] struct { + enabled bool + value T + } + + type pathWant struct { + enabled bool + kind pathKind + } + + type wantErr struct { + want bool + contains string // optional substring check + } + + type want struct { + branch branchKind + err wantErr + + apiServer opt[string] + bearerToken opt[string] + insecure opt[bool] + username opt[string] + password opt[string] + + kubeConfig pathWant + caFile pathWant + certFile pathWant + keyFile pathWant + } + + setupFailPoint := func(t *testing.T, fp failPoint) func() { + t.Helper() + + originalWriteKubeConfig := writeKubeConfig + originalWriteTempData := writeTempData + restore := func() { + writeKubeConfig = originalWriteKubeConfig + writeTempData = originalWriteTempData + } + + switch fp { + case failNone: + return restore + case failExecConfigSerialize: + writeKubeConfig = func(_ clientcmdapi.Config) ([]byte, error) { + return nil, errors.New("injected kubeconfig serialization error") + } + case failCADataTempFile, failCertDataTempFile, failKeyDataTempFile, failExecConfigTempFile: + targetCall := 1 + switch fp { + case failCertDataTempFile: + targetCall = 2 + case failKeyDataTempFile: + targetCall = 3 + } + callCount := 0 + writeTempData = func(data []byte) (string, error) { + callCount++ + if callCount == targetCall { + return "", errors.New("injected temp file error") + } + return originalWriteTempData(data) + } + default: + t.Fatalf("unknown failpoint %q", fp) + } + + return restore + } + + tests := []struct { + name string + client *Client + failAt failPoint // requires DI hooks in test harness; keep failNone for normal cases + want want + }{ + { + name: "non-exec branch selected when exec provider is nil and bearer token is empty", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.empty-token.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.empty-token.local:6443"}, + bearerToken: opt[string]{enabled: true, value: ""}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch propagates api server and bearer token", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://127.0.0.1:6443", + BearerToken: "token-123", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://127.0.0.1:6443"}, + bearerToken: opt[string]{enabled: true, value: "token-123"}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch is still selected when exec provider exists but bearer token is non-empty", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.nonexec.local:6443", + BearerToken: "static-token", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token"}, + APIVersion: "client.authentication.k8s.io/v1", + }, + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.nonexec.local:6443"}, + bearerToken: opt[string]{enabled: true, value: "static-token"}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch treats whitespace bearer token as non-empty and avoids exec branch", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.whitespace-token.local:6443", + BearerToken: " ", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token"}, + APIVersion: "client.authentication.k8s.io/v1", + }, + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.whitespace-token.local:6443"}, + bearerToken: opt[string]{enabled: true, value: " "}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch sets username only when password is empty", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.user-only.local:6443", + Username: "alice", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.user-only.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + username: opt[string]{enabled: true, value: "alice"}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch sets password only when username is empty", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.pass-only.local:6443", + Password: "super-secret", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.pass-only.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + password: opt[string]{enabled: true, value: "super-secret"}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch sets both username and password when both are provided", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.basic-auth.local:6443", + Username: "alice", + Password: "secret", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.basic-auth.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + username: opt[string]{enabled: true, value: "alice"}, + password: opt[string]{enabled: true, value: "secret"}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch creates CA file when insecure is false and CAData is present", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.ca.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte("ca-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.ca.local:6443"}, + insecure: opt[bool]{enabled: true, value: false}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathTempFile}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch does not create CA file when insecure is true even if CAData is present", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.ca-ignored.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + CAData: []byte("ca-data-ignored"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.ca-ignored.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch creates client cert file when CertData is present", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.cert.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + CertData: []byte("cert-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.cert.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathTempFile}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch creates client key file when KeyData is present", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.key.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + KeyData: []byte("key-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.key.local:6443"}, + insecure: opt[bool]{enabled: true, value: true}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathTempFile}, + }, + }, + { + name: "non-exec branch creates CA cert and key files when all tls data is present and insecure is false", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.full-tls.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte("ca-data"), + CertData: []byte("cert-data"), + KeyData: []byte("key-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: false}, + apiServer: opt[string]{enabled: true, value: "https://api.full-tls.local:6443"}, + insecure: opt[bool]{enabled: true, value: false}, + kubeConfig: pathWant{enabled: true, kind: pathDevNull}, + caFile: pathWant{enabled: true, kind: pathTempFile}, + certFile: pathWant{enabled: true, kind: pathTempFile}, + keyFile: pathWant{enabled: true, kind: pathTempFile}, + }, + }, + { + name: "exec branch selected when exec provider exists and bearer token is empty", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.exec.local:6443", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token"}, + APIVersion: "client.authentication.k8s.io/v1", + }, + BearerToken: "", + }, + }, + want: want{ + branch: branchExec, + err: wantErr{want: false}, + kubeConfig: pathWant{enabled: true, kind: pathTempFile}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "exec branch preserves servername and auth provider configuration while still returning kubeconfig temp file", + failAt: failNone, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.exec-authprovider.local:6443", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "kubelogin", + Args: []string{"get-token"}, + APIVersion: "client.authentication.k8s.io/v1beta1", + }, + AuthProvider: &clientcmdapi.AuthProviderConfig{ + Name: "gcp", + Config: map[string]string{"scopes": "https://www.googleapis.com/auth/cloud-platform"}, + }, + BearerToken: "", + TLSClientConfig: rest.TLSClientConfig{ + ServerName: "kubernetes.default.svc", + Insecure: false, + CAFile: "/etc/ssl/custom-ca.pem", + CAData: []byte("cluster-ca"), + }, + }, + }, + want: want{ + branch: branchExec, + err: wantErr{want: false}, + kubeConfig: pathWant{enabled: true, kind: pathTempFile}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch returns error when CA temp file creation fails", + failAt: failCADataTempFile, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.fail-ca.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte("ca-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: true, contains: "temp"}, + kubeConfig: pathWant{enabled: true, kind: pathNil}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch returns error when cert temp file creation fails after CA file creation", + failAt: failCertDataTempFile, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.fail-cert.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte("ca-data"), + CertData: []byte("cert-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: true, contains: "temp"}, + kubeConfig: pathWant{enabled: true, kind: pathNil}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "non-exec branch returns error when key temp file creation fails after CA and cert creation", + failAt: failKeyDataTempFile, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.fail-key.local:6443", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: false, + CAData: []byte("ca-data"), + CertData: []byte("cert-data"), + KeyData: []byte("key-data"), + }, + }, + }, + want: want{ + branch: branchNonExec, + err: wantErr{want: true, contains: "temp"}, + kubeConfig: pathWant{enabled: true, kind: pathNil}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "exec branch returns error when kubeconfig serialization fails", + failAt: failExecConfigSerialize, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.fail-serialize.local:6443", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token"}, + APIVersion: "client.authentication.k8s.io/v1", + }, + BearerToken: "", + }, + }, + want: want{ + branch: branchExec, + err: wantErr{want: true, contains: "failed to write kubeconfig"}, + kubeConfig: pathWant{enabled: true, kind: pathNil}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + { + name: "exec branch returns error when kubeconfig temp file creation fails", + failAt: failExecConfigTempFile, + client: &Client{ + RestConfig: rest.Config{ + Host: "https://api.fail-exec-tempfile.local:6443", + ExecProvider: &clientcmdapi.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token"}, + APIVersion: "client.authentication.k8s.io/v1", + }, + BearerToken: "", + }, + }, + want: want{ + branch: branchExec, + err: wantErr{want: true, contains: "failed to get kubeconfig file"}, + kubeConfig: pathWant{enabled: true, kind: pathNil}, + caFile: pathWant{enabled: true, kind: pathNil}, + certFile: pathWant{enabled: true, kind: pathNil}, + keyFile: pathWant{enabled: true, kind: pathNil}, + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + // If you implemented DI for error injection, this should be a no-op for failNone. + restore := setupFailPoint(t, test.failAt) + defer restore() + + got, cleanup, err := test.client.setupKubeConfig() + + // contract: cleanup should always be non-nil (even on error paths) + if cleanup == nil { + t.Fatalf("cleanup func is nil") + } + defer cleanup() + + // error assertions + if test.want.err.want { + if err == nil { + t.Fatalf("expected error, got nil") + } + if test.want.err.contains != "" && !strings.Contains(err.Error(), test.want.err.contains) { + t.Fatalf("error %q does not contain %q", err.Error(), test.want.err.contains) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got == nil { + t.Fatalf("got nil kubeConfig") + } + + // branch assertion + actualBranch := branchExec + if got.KubeConfig != nil && *got.KubeConfig == os.DevNull { + actualBranch = branchNonExec + } + if actualBranch != test.want.branch { + t.Fatalf("branch mismatch: got %q want %q", actualBranch, test.want.branch) + } + + assertOptString := func(field string, gotPtr *string, exp opt[string]) { + t.Helper() + if !exp.enabled { + return + } + if gotPtr == nil { + t.Fatalf("%s is nil; want %q", field, exp.value) + } + if *gotPtr != exp.value { + t.Fatalf("%s mismatch: got %q want %q", field, *gotPtr, exp.value) + } + } + + assertOptBool := func(field string, gotPtr *bool, exp opt[bool]) { + t.Helper() + if !exp.enabled { + return + } + if gotPtr == nil { + t.Fatalf("%s is nil; want %v", field, exp.value) + } + if *gotPtr != exp.value { + t.Fatalf("%s mismatch: got %v want %v", field, *gotPtr, exp.value) + } + } + + assertPath := func(field string, gotPtr *string, exp pathWant) { + t.Helper() + if !exp.enabled || exp.kind == pathSkip { + return + } + switch exp.kind { + case pathNil: + if gotPtr == nil { + return + } + if *gotPtr != "" { + t.Fatalf("%s expected nil/empty, got %q", field, *gotPtr) + } + case pathDevNull: + if gotPtr == nil { + t.Fatalf("%s expected %q, got nil", field, os.DevNull) + } + if *gotPtr != os.DevNull { + t.Fatalf("%s expected %q, got %q", field, os.DevNull, *gotPtr) + } + case pathTempFile: + if gotPtr == nil { + t.Fatalf("%s expected temp file path, got nil", field) + } + if strings.TrimSpace(*gotPtr) == "" { + t.Fatalf("%s expected non-empty temp file path", field) + } + if *gotPtr == os.DevNull { + t.Fatalf("%s expected temp file path, got %q", field, os.DevNull) + } + if _, statErr := os.Stat(*gotPtr); statErr != nil { + t.Fatalf("%s expected existing file at %q: %v", field, *gotPtr, statErr) + } + default: + t.Fatalf("unknown path kind %q for field %s", exp.kind, field) + } + } + + assertOptString("APIServer", got.APIServer, test.want.apiServer) + assertOptString("BearerToken", got.BearerToken, test.want.bearerToken) + assertOptBool("Insecure", got.Insecure, test.want.insecure) + assertOptString("Username", got.Username, test.want.username) + assertOptString("Password", got.Password, test.want.password) + + assertPath("KubeConfig", got.KubeConfig, test.want.kubeConfig) + assertPath("CAFile", got.CAFile, test.want.caFile) + assertPath("CertFile", got.CertFile, test.want.certFile) + assertPath("KeyFile", got.KeyFile, test.want.keyFile) + }) + } } From ae1d8b41452569b04d28a14683d1a7944410d6b5 Mon Sep 17 00:00:00 2001 From: reaper8055 <11490705+reaper8055@users.noreply.github.com> Date: Sun, 14 Jun 2026 19:22:55 +0530 Subject: [PATCH 07/10] returned meshkit error in ApplyHelmChart Signed-off-by: reaper8055 <11490705+reaper8055@users.noreply.github.com> Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 41c025105..957ee0533 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -56,15 +56,16 @@ const ( Latest = ">0.0.0-0" ) -// downloadLocaton is the location where downloaded helm charts -// will be stored. os.TempDir will ensure that the path is cross -// platform -var downloadLocation = os.TempDir() - var ( - // writeKubeConfig marshals kubeconfig bytes. Kept as a variable to enable deterministic test stubs. + // downloadLocaton is the location where downloaded helm charts + // will be stored. os.TempDir will ensure that the path is cross + // platform + downloadLocation = os.TempDir() + // writeKubeConfig marshals kubeconfig bytes. Kept as a variable to enable + // deterministic test stubs. writeKubeConfig = clientcmd.Write - // writeTempData writes byte payloads to temp files. Kept as a variable to enable deterministic test stubs. + // writeTempData writes byte payloads to temp files. Kept as a variable to + // enable deterministic test stubs. writeTempData = setDataAndReturnFilename ) @@ -274,7 +275,7 @@ func (c *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { kubeConfig, cleanup, err := c.setupKubeConfig() if err != nil { cleanup() - return err + return ErrApplyHelmChart(err) } actionConfig, err := c.createHelmActionConfig(cfg, kubeConfig) if err != nil { From 578331276eb52274a8e8193bc3d4998776e46a30 Mon Sep 17 00:00:00 2001 From: reaper8055 Date: Sat, 20 Jun 2026 11:52:57 +0530 Subject: [PATCH 08/10] removed ambigious comment Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 957ee0533..ed6673697 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -527,7 +527,6 @@ func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), erro AuthInfo: authInfo, } - // explicitly setting kube context may not be required helmKubeConfig.CurrentContext = clusterName configBytes, err := writeKubeConfig(*helmKubeConfig) From 27d0d19416380256e29c73fc79898153581f645a Mon Sep 17 00:00:00 2001 From: reaper8055 Date: Sat, 18 Jul 2026 13:58:55 +0530 Subject: [PATCH 09/10] use original kubeconfig loader to allow helm to execute renewable credential plugins Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 16 ++- utils/kubernetes/client-config-getter.go | 53 ++++++++ utils/kubernetes/client-config-getter_test.go | 118 ++++++++++++++++++ utils/kubernetes/client.go | 41 ++++-- utils/kubernetes/kubernetes.go | 9 +- 5 files changed, 221 insertions(+), 16 deletions(-) create mode 100644 utils/kubernetes/client-config-getter.go create mode 100644 utils/kubernetes/client-config-getter_test.go diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index ed6673697..6467fa724 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -272,12 +272,12 @@ func (c *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { return ErrApplyHelmChart(err) } - kubeConfig, cleanup, err := c.setupKubeConfig() + restClientGetter, cleanup, err := c.helmRESTClientGetter() if err != nil { cleanup() return ErrApplyHelmChart(err) } - actionConfig, err := c.createHelmActionConfig(cfg, kubeConfig) + actionConfig, err := c.createHelmActionConfig(cfg, restClientGetter) if err != nil { cleanup() return ErrApplyHelmChart(err) @@ -422,18 +422,26 @@ func checkIfInstallable(ch *chart.Chart) error { } // createHelmActionConfig generates the actionConfig with the appropriate defaults -func (c *Client) createHelmActionConfig(cfg ApplyHelmChartConfig, kubeConfig *genericclioptions.ConfigFlags) (*action.Configuration, error) { +func (c *Client) createHelmActionConfig(cfg ApplyHelmChartConfig, restClientGetter genericclioptions.RESTClientGetter) (*action.Configuration, error) { // Set the environment variable needed by the Init methods _ = os.Setenv("HELM_DRIVER_SQL_CONNECTION_STRING", cfg.SQLConnectionString) actionConfig := new(action.Configuration) - if err := actionConfig.Init(kubeConfig, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { + if err := actionConfig.Init(restClientGetter, cfg.Namespace, string(cfg.HelmDriver), cfg.Logger); err != nil { return nil, ErrApplyHelmChart(err) } return actionConfig, nil } +func (c *Client) helmRESTClientGetter() (genericclioptions.RESTClientGetter, func(), error) { + if loader := c.rawKubeConfigLoader(); loader != nil { + return newClientConfigRESTClientGetter(loader), func() {}, nil + } + + return c.setupKubeConfig() +} + func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), error) { var tempFiles []string cleanup := func() { diff --git a/utils/kubernetes/client-config-getter.go b/utils/kubernetes/client-config-getter.go new file mode 100644 index 000000000..77df6eb45 --- /dev/null +++ b/utils/kubernetes/client-config-getter.go @@ -0,0 +1,53 @@ +package kubernetes + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" + "k8s.io/client-go/tools/clientcmd" +) + +type clientConfigRESTClientGetter struct { + clientConfig clientcmd.ClientConfig +} + +var _ genericclioptions.RESTClientGetter = (*clientConfigRESTClientGetter)(nil) + +func newClientConfigRESTClientGetter(clientConfig clientcmd.ClientConfig) genericclioptions.RESTClientGetter { + return &clientConfigRESTClientGetter{clientConfig: clientConfig} +} + +func (g *clientConfigRESTClientGetter) ToRESTConfig() (*rest.Config, error) { + return g.clientConfig.ClientConfig() +} + +func (g *clientConfigRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + config, err := g.ToRESTConfig() + if err != nil { + return nil, err + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return nil, err + } + + return memory.NewMemCacheClient(discoveryClient), nil +} + +func (g *clientConfigRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { + discoveryClient, err := g.ToDiscoveryClient() + if err != nil { + return nil, err + } + + mapper := restmapper.NewDeferredDiscoveryRESTMapper(discoveryClient) + return restmapper.NewShortcutExpander(mapper, discoveryClient, func(string) {}), nil +} + +func (g *clientConfigRESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig { + return g.clientConfig +} diff --git a/utils/kubernetes/client-config-getter_test.go b/utils/kubernetes/client-config-getter_test.go new file mode 100644 index 000000000..a6bcf3fd3 --- /dev/null +++ b/utils/kubernetes/client-config-getter_test.go @@ -0,0 +1,118 @@ +package kubernetes + +import ( + "os" + "testing" + + "k8s.io/cli-runtime/pkg/genericclioptions" +) + +func testExecKubeConfig() []byte { + return []byte(`apiVersion: v1 +kind: Config +clusters: +- name: test-cluster + cluster: + server: https://cluster.example.com + insecure-skip-tls-verify: true +contexts: +- name: test-context + context: + cluster: test-cluster + user: test-user +current-context: test-context +users: +- name: test-user + user: + exec: + apiVersion: client.authentication.k8s.io/v1 + command: credential-plugin + interactiveMode: Never +`) +} + +func TestNewRetainsKubeConfigLoader(t *testing.T) { + client, err := New(testExecKubeConfig()) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + loader := client.rawKubeConfigLoader() + if loader == nil { + t.Fatal("New() did not retain the kubeconfig loader") + } + + rawConfig, err := loader.RawConfig() + if err != nil { + t.Fatalf("RawConfig() error = %v", err) + } + authInfo := rawConfig.AuthInfos["test-user"] + if authInfo == nil || authInfo.Exec == nil { + t.Fatal("retained kubeconfig loader lost exec authentication") + } + if authInfo.Exec.Command != "credential-plugin" { + t.Fatalf("exec command = %q, want %q", authInfo.Exec.Command, "credential-plugin") + } +} + +func TestClientConfigRESTClientGetterPreservesLoader(t *testing.T) { + _, loader, err := detectKubeConfig(testExecKubeConfig()) + if err != nil { + t.Fatalf("detectKubeConfig() error = %v", err) + } + + getter := newClientConfigRESTClientGetter(loader) + if getter.ToRawKubeConfigLoader() != loader { + t.Fatal("RESTClientGetter did not return the retained kubeconfig loader") + } + + config, err := getter.ToRESTConfig() + if err != nil { + t.Fatalf("ToRESTConfig() error = %v", err) + } + if config.ExecProvider == nil || config.ExecProvider.Command != "credential-plugin" { + t.Fatal("RESTClientGetter did not preserve exec authentication") + } +} + +func TestHelmRESTClientGetterPrefersRetainedLoader(t *testing.T) { + config, loader, err := detectKubeConfig(testExecKubeConfig()) + if err != nil { + t.Fatalf("detectKubeConfig() error = %v", err) + } + client := &Client{ + RestConfig: *config, + kubeConfigLoader: loader, + } + + getter, cleanup, err := client.helmRESTClientGetter() + if err != nil { + t.Fatalf("helmRESTClientGetter() error = %v", err) + } + defer cleanup() + + if getter.ToRawKubeConfigLoader() != loader { + t.Fatal("Helm did not receive the retained kubeconfig loader") + } + if _, ok := getter.(*genericclioptions.ConfigFlags); ok { + t.Fatal("Helm reconstructed ConfigFlags despite a retained kubeconfig loader") + } +} + +func TestHelmRESTClientGetterFallsBackForDirectClient(t *testing.T) { + client := &Client{} + + getter, cleanup, err := client.helmRESTClientGetter() + if err != nil { + t.Fatalf("helmRESTClientGetter() error = %v", err) + } + defer cleanup() + + configFlags, ok := getter.(*genericclioptions.ConfigFlags) + if !ok { + t.Fatalf("fallback getter type = %T, want *genericclioptions.ConfigFlags", getter) + } + if configFlags.KubeConfig == nil || *configFlags.KubeConfig != os.DevNull { + t.Fatalf("fallback kubeconfig = %v, want %q", configFlags.KubeConfig, os.DevNull) + } +} diff --git a/utils/kubernetes/client.go b/utils/kubernetes/client.go index dd0e19423..3d526405c 100644 --- a/utils/kubernetes/client.go +++ b/utils/kubernetes/client.go @@ -12,22 +12,27 @@ import ( // DetectKubeConfig detects the kubeconfig for the kubernetes cluster and returns it func DetectKubeConfig(configfile []byte) (config *rest.Config, err error) { + config, _, err = detectKubeConfig(configfile) + return config, err +} + +func detectKubeConfig(configfile []byte) (config *rest.Config, loader clientcmd.ClientConfig, err error) { if len(configfile) > 0 { var cfgFile []byte _, cfgFile, err = ProcessConfig(configfile, "") if err != nil { - return nil, err + return nil, nil, err } - if config, err = clientcmd.RESTConfigFromKubeConfig(cfgFile); err == nil { - return config, nil + if config, loader, err = loadClientConfigFromKubeconfig(cfgFile); err == nil { + return config, loader, nil } } // If deployed within the cluster if config, err = rest.InClusterConfig(); err == nil { - return config, nil + return config, nil, nil } // Look for kubeconfig from the path mentioned in $KUBECONFIG @@ -35,10 +40,10 @@ func DetectKubeConfig(configfile []byte) (config *rest.Config, err error) { if kubeconfig != "" { _, cfgFile, err := ProcessConfig(kubeconfig, "") if err != nil { - return nil, err + return nil, nil, err } - if config, err = clientcmd.RESTConfigFromKubeConfig(cfgFile); err == nil { - return config, nil + if config, loader, err = loadClientConfigFromKubeconfig(cfgFile); err == nil { + return config, loader, nil } } @@ -46,13 +51,27 @@ func DetectKubeConfig(configfile []byte) (config *rest.Config, err error) { path := filepath.Join(utils.GetHome(), ".kube", "config") _, cfgFile, err := ProcessConfig(path, "") if err != nil { - return nil, err + return nil, nil, err + } + if config, loader, err = loadClientConfigFromKubeconfig(cfgFile); err == nil { + return config, loader, nil + } + + return nil, nil, ErrRestConfigFromKubeConfig(err) +} + +func loadClientConfigFromKubeconfig(kubeconfig []byte) (*rest.Config, clientcmd.ClientConfig, error) { + loader, err := clientcmd.NewClientConfigFromBytes(kubeconfig) + if err != nil { + return nil, nil, err } - if config, err = clientcmd.RESTConfigFromKubeConfig(cfgFile); err == nil { - return config, nil + + config, err := loader.ClientConfig() + if err != nil { + return nil, nil, err } - return nil, ErrRestConfigFromKubeConfig(err) + return config, loader, nil } // ProcessConfig handles loading, validating, and optionally saving or returning a kubeconfig diff --git a/utils/kubernetes/kubernetes.go b/utils/kubernetes/kubernetes.go index 5454730f7..3adf082d5 100644 --- a/utils/kubernetes/kubernetes.go +++ b/utils/kubernetes/kubernetes.go @@ -4,16 +4,18 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" ) type Client struct { RestConfig rest.Config `json:"restconfig,omitempty"` KubeClient *kubernetes.Clientset `json:"kubeclient,omitempty"` DynamicKubeClient dynamic.Interface `json:"dynamicKubeClient,omitempty"` + kubeConfigLoader clientcmd.ClientConfig } func New(kubeconfig []byte) (*Client, error) { - restConfig, err := DetectKubeConfig(kubeconfig) + restConfig, kubeConfigLoader, err := detectKubeConfig(kubeconfig) if err != nil { return nil, err } @@ -41,5 +43,10 @@ func New(kubeconfig []byte) (*Client, error) { RestConfig: *restConfig, DynamicKubeClient: dyclient, KubeClient: kclient, + kubeConfigLoader: kubeConfigLoader, }, nil } + +func (c *Client) rawKubeConfigLoader() clientcmd.ClientConfig { + return c.kubeConfigLoader +} From 9afa07cd2d47fb16afa4fa84194155505f085023 Mon Sep 17 00:00:00 2001 From: reaper8055 Date: Sun, 19 Jul 2026 22:00:07 +0530 Subject: [PATCH 10/10] centralize exec-aware client configuration Signed-off-by: reaper8055 --- utils/kubernetes/apply-helm-chart.go | 152 +--- utils/kubernetes/apply-helm-chart_test.go | 723 ------------------ utils/kubernetes/client-config-getter.go | 50 +- utils/kubernetes/client-config-getter_test.go | 201 ++++- utils/kubernetes/kubernetes.go | 33 +- 5 files changed, 252 insertions(+), 907 deletions(-) delete mode 100644 utils/kubernetes/apply-helm-chart_test.go diff --git a/utils/kubernetes/apply-helm-chart.go b/utils/kubernetes/apply-helm-chart.go index 6467fa724..7c25f0cf2 100644 --- a/utils/kubernetes/apply-helm-chart.go +++ b/utils/kubernetes/apply-helm-chart.go @@ -15,8 +15,6 @@ import ( "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" "k8s.io/cli-runtime/pkg/genericclioptions" - "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) // HelmDriver is the type for helm drivers @@ -61,12 +59,6 @@ var ( // will be stored. os.TempDir will ensure that the path is cross // platform downloadLocation = os.TempDir() - // writeKubeConfig marshals kubeconfig bytes. Kept as a variable to enable - // deterministic test stubs. - writeKubeConfig = clientcmd.Write - // writeTempData writes byte payloads to temp files. Kept as a variable to - // enable deterministic test stubs. - writeTempData = setDataAndReturnFilename ) // HelmIndex holds the index.yaml data in the struct format @@ -272,17 +264,10 @@ func (c *Client) ApplyHelmChart(cfg ApplyHelmChartConfig) error { return ErrApplyHelmChart(err) } - restClientGetter, cleanup, err := c.helmRESTClientGetter() + actionConfig, err := c.createHelmActionConfig(cfg, c.getRESTClientGetter()) if err != nil { - cleanup() return ErrApplyHelmChart(err) } - actionConfig, err := c.createHelmActionConfig(cfg, restClientGetter) - if err != nil { - cleanup() - return ErrApplyHelmChart(err) - } - defer cleanup() // Before installing a helm chart, check if it already exists in the cluster // this is a workaround make the helm chart installation idempotent @@ -434,141 +419,6 @@ func (c *Client) createHelmActionConfig(cfg ApplyHelmChartConfig, restClientGett return actionConfig, nil } -func (c *Client) helmRESTClientGetter() (genericclioptions.RESTClientGetter, func(), error) { - if loader := c.rawKubeConfigLoader(); loader != nil { - return newClientConfigRESTClientGetter(loader), func() {}, nil - } - - return c.setupKubeConfig() -} - -func (c *Client) setupKubeConfig() (*genericclioptions.ConfigFlags, func(), error) { - var tempFiles []string - cleanup := func() { - for _, f := range tempFiles { - _ = os.Remove(f) - } - } - - // KubeConfig setup - kubeConfig := genericclioptions.NewConfigFlags(false) - - // Exec-auth kubeconfigs (eg: aws eks get-token) typically do not have a static bearer token. - // In that case, do not force /dev/null; let client-go load kubeconfig and execute the auth plugin. - useKubeConfigAuth := c.RestConfig.ExecProvider != nil && c.RestConfig.BearerToken == "" - - if !useKubeConfigAuth { - // Set KubeConfig to DevNull to prevent read from local kubeconfig - // to prevent conflicts between "data" and "files" properties (CAFile, CAData and KeyFile, KeyData) - // ConfigFlags only allows setting CAFile, KeyFile but not CAData, KeyData. - // When the library reads the original kubeconfig containing cert data / key data AND we specify cert file / key file, these configurations conflict - devNull := os.DevNull - kubeConfig.KubeConfig = &devNull - kubeConfig.APIServer = &c.RestConfig.Host - kubeConfig.BearerToken = &c.RestConfig.BearerToken - kubeConfig.Insecure = &c.RestConfig.Insecure - - // Set username and password for basic auth if available - if c.RestConfig.Username != "" { - kubeConfig.Username = &c.RestConfig.Username - } - if c.RestConfig.Password != "" { - kubeConfig.Password = &c.RestConfig.Password - } - - // Only set CA file if not running in insecure mode - if !c.RestConfig.Insecure { - if len(c.RestConfig.CAData) > 0 { - caFileName, err := writeTempData(c.RestConfig.CAData) - if err != nil { - return nil, cleanup, err - } - tempFiles = append(tempFiles, caFileName) - kubeConfig.CAFile = &caFileName - } - } - - // Set client certificate data if available - if len(c.RestConfig.CertData) > 0 { - certFileName, err := writeTempData(c.RestConfig.CertData) - if err != nil { - return nil, cleanup, err - } - tempFiles = append(tempFiles, certFileName) - kubeConfig.CertFile = &certFileName - } - - // Set client key data if available - if len(c.RestConfig.KeyData) > 0 { - keyFileName, err := writeTempData(c.RestConfig.KeyData) - if err != nil { - return nil, cleanup, err - } - tempFiles = append(tempFiles, keyFileName) - kubeConfig.KeyFile = &keyFileName - } - return kubeConfig, cleanup, nil - } - - const ( - clusterName = "meshery-cluster" - authInfo = "meshkit-helm-user" - ) - - helmKubeConfig := clientcmdapi.NewConfig() - - helmKubeConfig.Clusters[clusterName] = &clientcmdapi.Cluster{ - Server: c.RestConfig.Host, - TLSServerName: c.RestConfig.ServerName, - InsecureSkipTLSVerify: c.RestConfig.Insecure, - CertificateAuthority: c.RestConfig.CAFile, - CertificateAuthorityData: c.RestConfig.CAData, - } - - helmKubeConfig.AuthInfos[authInfo] = &clientcmdapi.AuthInfo{ - Exec: c.RestConfig.ExecProvider, - AuthProvider: c.RestConfig.AuthProvider, - } - - helmKubeConfig.Contexts[clusterName] = &clientcmdapi.Context{ - Cluster: clusterName, - AuthInfo: authInfo, - } - - helmKubeConfig.CurrentContext = clusterName - - configBytes, err := writeKubeConfig(*helmKubeConfig) - if err != nil { - return nil, cleanup, fmt.Errorf("failed to write kubeconfig %v", err) - } - - configFile, err := writeTempData(configBytes) - if err != nil { - return nil, cleanup, fmt.Errorf("failed to get kubeconfig file %v", err) - } - tempFiles = append(tempFiles, configFile) - - kubeConfig.KubeConfig = &configFile - return kubeConfig, cleanup, nil -} - -// Populates a file in temp directory with the passed data and returns the filename -func setDataAndReturnFilename(data []byte) (string, error) { - f, err := os.CreateTemp("", "") - if err != nil { - return "", err - } - defer func() { _ = f.Close() }() // Close file immediately after writing - - _, err = f.Write(data) - if err != nil { - _ = os.Remove(f.Name()) // Clean up on write error - return "", err - } - - return f.Name(), nil -} - // generateAction generates an action function using action.Configuration // and ApplyHelmChartConfig and returns it // diff --git a/utils/kubernetes/apply-helm-chart_test.go b/utils/kubernetes/apply-helm-chart_test.go deleted file mode 100644 index c9565ab8d..000000000 --- a/utils/kubernetes/apply-helm-chart_test.go +++ /dev/null @@ -1,723 +0,0 @@ -package kubernetes - -import ( - "errors" - "os" - "strings" - "testing" - - "k8s.io/client-go/rest" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" -) - -// Test design note: -// -// We intentionally assert behavior-level invariants for setupKubeConfig() -// instead of doing a full DeepEqual on *genericclioptions.ConfigFlags for the -// following reasons -// -// 1. the function produces runtime-generated artifacts (temp file paths) -// for CA/Cert/Key material. Those paths are nondeterministic by design, so exact -// object equality would make tests brittle and OS/environment-dependent. -// -// 2. the function returns a cleanup closure. Function values are not -// meaningfully comparable for equality in Go, so "exact output object" checking -// is not a stable assertion model for this API shape. -// -// 3. the real contract of this branch is not "exact struct bytes match"; -// it is: select non-exec mode, force KubeConfig=/dev/null, propagate auth/TLS -// fields correctly, create material files only when required, and provide a -// cleanup hook that removes created files. -// -// This test therefore checks deterministic outcomes that represent that contract: -// field propagation, branch selection, file-presence/file-absence expectations, -// and cleanup side effects. -// -// If we later introduce dependency injection for temp-file creation, we can add -// stricter deterministic assertions (including exact file names/paths) without -// changing the behavior contract validated here. -func Test_setupKubeConfig(t *testing.T) { - type ( - branchKind string - pathKind string - // only for deterministic error injection in tests - failPoint string - ) - - const ( - branchNonExec branchKind = "non-exec" - branchExec branchKind = "exec" - pathSkip pathKind = "skip" - pathNil pathKind = "nil" - pathDevNull pathKind = "devnull" - pathTempFile pathKind = "tempFile" - - // only for deterministic error injection in tests - failNone failPoint = "" - failCADataTempFile failPoint = "ca-tempfile" - failCertDataTempFile failPoint = "cert-tempfile" - failKeyDataTempFile failPoint = "key-tempfile" - failExecConfigSerialize failPoint = "exec-config-serialize" - failExecConfigTempFile failPoint = "exec-config-tempfile" - ) - - type opt[T any] struct { - enabled bool - value T - } - - type pathWant struct { - enabled bool - kind pathKind - } - - type wantErr struct { - want bool - contains string // optional substring check - } - - type want struct { - branch branchKind - err wantErr - - apiServer opt[string] - bearerToken opt[string] - insecure opt[bool] - username opt[string] - password opt[string] - - kubeConfig pathWant - caFile pathWant - certFile pathWant - keyFile pathWant - } - - setupFailPoint := func(t *testing.T, fp failPoint) func() { - t.Helper() - - originalWriteKubeConfig := writeKubeConfig - originalWriteTempData := writeTempData - restore := func() { - writeKubeConfig = originalWriteKubeConfig - writeTempData = originalWriteTempData - } - - switch fp { - case failNone: - return restore - case failExecConfigSerialize: - writeKubeConfig = func(_ clientcmdapi.Config) ([]byte, error) { - return nil, errors.New("injected kubeconfig serialization error") - } - case failCADataTempFile, failCertDataTempFile, failKeyDataTempFile, failExecConfigTempFile: - targetCall := 1 - switch fp { - case failCertDataTempFile: - targetCall = 2 - case failKeyDataTempFile: - targetCall = 3 - } - callCount := 0 - writeTempData = func(data []byte) (string, error) { - callCount++ - if callCount == targetCall { - return "", errors.New("injected temp file error") - } - return originalWriteTempData(data) - } - default: - t.Fatalf("unknown failpoint %q", fp) - } - - return restore - } - - tests := []struct { - name string - client *Client - failAt failPoint // requires DI hooks in test harness; keep failNone for normal cases - want want - }{ - { - name: "non-exec branch selected when exec provider is nil and bearer token is empty", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.empty-token.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.empty-token.local:6443"}, - bearerToken: opt[string]{enabled: true, value: ""}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch propagates api server and bearer token", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://127.0.0.1:6443", - BearerToken: "token-123", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://127.0.0.1:6443"}, - bearerToken: opt[string]{enabled: true, value: "token-123"}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch is still selected when exec provider exists but bearer token is non-empty", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.nonexec.local:6443", - BearerToken: "static-token", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "aws", - Args: []string{"eks", "get-token"}, - APIVersion: "client.authentication.k8s.io/v1", - }, - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.nonexec.local:6443"}, - bearerToken: opt[string]{enabled: true, value: "static-token"}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch treats whitespace bearer token as non-empty and avoids exec branch", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.whitespace-token.local:6443", - BearerToken: " ", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "aws", - Args: []string{"eks", "get-token"}, - APIVersion: "client.authentication.k8s.io/v1", - }, - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.whitespace-token.local:6443"}, - bearerToken: opt[string]{enabled: true, value: " "}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch sets username only when password is empty", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.user-only.local:6443", - Username: "alice", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.user-only.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - username: opt[string]{enabled: true, value: "alice"}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch sets password only when username is empty", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.pass-only.local:6443", - Password: "super-secret", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.pass-only.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - password: opt[string]{enabled: true, value: "super-secret"}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch sets both username and password when both are provided", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.basic-auth.local:6443", - Username: "alice", - Password: "secret", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.basic-auth.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - username: opt[string]{enabled: true, value: "alice"}, - password: opt[string]{enabled: true, value: "secret"}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch creates CA file when insecure is false and CAData is present", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.ca.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: false, - CAData: []byte("ca-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.ca.local:6443"}, - insecure: opt[bool]{enabled: true, value: false}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathTempFile}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch does not create CA file when insecure is true even if CAData is present", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.ca-ignored.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - CAData: []byte("ca-data-ignored"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.ca-ignored.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch creates client cert file when CertData is present", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.cert.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - CertData: []byte("cert-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.cert.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathTempFile}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch creates client key file when KeyData is present", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.key.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: true, - KeyData: []byte("key-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.key.local:6443"}, - insecure: opt[bool]{enabled: true, value: true}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathTempFile}, - }, - }, - { - name: "non-exec branch creates CA cert and key files when all tls data is present and insecure is false", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.full-tls.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: false, - CAData: []byte("ca-data"), - CertData: []byte("cert-data"), - KeyData: []byte("key-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: false}, - apiServer: opt[string]{enabled: true, value: "https://api.full-tls.local:6443"}, - insecure: opt[bool]{enabled: true, value: false}, - kubeConfig: pathWant{enabled: true, kind: pathDevNull}, - caFile: pathWant{enabled: true, kind: pathTempFile}, - certFile: pathWant{enabled: true, kind: pathTempFile}, - keyFile: pathWant{enabled: true, kind: pathTempFile}, - }, - }, - { - name: "exec branch selected when exec provider exists and bearer token is empty", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.exec.local:6443", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "aws", - Args: []string{"eks", "get-token"}, - APIVersion: "client.authentication.k8s.io/v1", - }, - BearerToken: "", - }, - }, - want: want{ - branch: branchExec, - err: wantErr{want: false}, - kubeConfig: pathWant{enabled: true, kind: pathTempFile}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "exec branch preserves servername and auth provider configuration while still returning kubeconfig temp file", - failAt: failNone, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.exec-authprovider.local:6443", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "kubelogin", - Args: []string{"get-token"}, - APIVersion: "client.authentication.k8s.io/v1beta1", - }, - AuthProvider: &clientcmdapi.AuthProviderConfig{ - Name: "gcp", - Config: map[string]string{"scopes": "https://www.googleapis.com/auth/cloud-platform"}, - }, - BearerToken: "", - TLSClientConfig: rest.TLSClientConfig{ - ServerName: "kubernetes.default.svc", - Insecure: false, - CAFile: "/etc/ssl/custom-ca.pem", - CAData: []byte("cluster-ca"), - }, - }, - }, - want: want{ - branch: branchExec, - err: wantErr{want: false}, - kubeConfig: pathWant{enabled: true, kind: pathTempFile}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch returns error when CA temp file creation fails", - failAt: failCADataTempFile, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.fail-ca.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: false, - CAData: []byte("ca-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: true, contains: "temp"}, - kubeConfig: pathWant{enabled: true, kind: pathNil}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch returns error when cert temp file creation fails after CA file creation", - failAt: failCertDataTempFile, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.fail-cert.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: false, - CAData: []byte("ca-data"), - CertData: []byte("cert-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: true, contains: "temp"}, - kubeConfig: pathWant{enabled: true, kind: pathNil}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "non-exec branch returns error when key temp file creation fails after CA and cert creation", - failAt: failKeyDataTempFile, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.fail-key.local:6443", - TLSClientConfig: rest.TLSClientConfig{ - Insecure: false, - CAData: []byte("ca-data"), - CertData: []byte("cert-data"), - KeyData: []byte("key-data"), - }, - }, - }, - want: want{ - branch: branchNonExec, - err: wantErr{want: true, contains: "temp"}, - kubeConfig: pathWant{enabled: true, kind: pathNil}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "exec branch returns error when kubeconfig serialization fails", - failAt: failExecConfigSerialize, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.fail-serialize.local:6443", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "aws", - Args: []string{"eks", "get-token"}, - APIVersion: "client.authentication.k8s.io/v1", - }, - BearerToken: "", - }, - }, - want: want{ - branch: branchExec, - err: wantErr{want: true, contains: "failed to write kubeconfig"}, - kubeConfig: pathWant{enabled: true, kind: pathNil}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - { - name: "exec branch returns error when kubeconfig temp file creation fails", - failAt: failExecConfigTempFile, - client: &Client{ - RestConfig: rest.Config{ - Host: "https://api.fail-exec-tempfile.local:6443", - ExecProvider: &clientcmdapi.ExecConfig{ - Command: "aws", - Args: []string{"eks", "get-token"}, - APIVersion: "client.authentication.k8s.io/v1", - }, - BearerToken: "", - }, - }, - want: want{ - branch: branchExec, - err: wantErr{want: true, contains: "failed to get kubeconfig file"}, - kubeConfig: pathWant{enabled: true, kind: pathNil}, - caFile: pathWant{enabled: true, kind: pathNil}, - certFile: pathWant{enabled: true, kind: pathNil}, - keyFile: pathWant{enabled: true, kind: pathNil}, - }, - }, - } - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { - // If you implemented DI for error injection, this should be a no-op for failNone. - restore := setupFailPoint(t, test.failAt) - defer restore() - - got, cleanup, err := test.client.setupKubeConfig() - - // contract: cleanup should always be non-nil (even on error paths) - if cleanup == nil { - t.Fatalf("cleanup func is nil") - } - defer cleanup() - - // error assertions - if test.want.err.want { - if err == nil { - t.Fatalf("expected error, got nil") - } - if test.want.err.contains != "" && !strings.Contains(err.Error(), test.want.err.contains) { - t.Fatalf("error %q does not contain %q", err.Error(), test.want.err.contains) - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got == nil { - t.Fatalf("got nil kubeConfig") - } - - // branch assertion - actualBranch := branchExec - if got.KubeConfig != nil && *got.KubeConfig == os.DevNull { - actualBranch = branchNonExec - } - if actualBranch != test.want.branch { - t.Fatalf("branch mismatch: got %q want %q", actualBranch, test.want.branch) - } - - assertOptString := func(field string, gotPtr *string, exp opt[string]) { - t.Helper() - if !exp.enabled { - return - } - if gotPtr == nil { - t.Fatalf("%s is nil; want %q", field, exp.value) - } - if *gotPtr != exp.value { - t.Fatalf("%s mismatch: got %q want %q", field, *gotPtr, exp.value) - } - } - - assertOptBool := func(field string, gotPtr *bool, exp opt[bool]) { - t.Helper() - if !exp.enabled { - return - } - if gotPtr == nil { - t.Fatalf("%s is nil; want %v", field, exp.value) - } - if *gotPtr != exp.value { - t.Fatalf("%s mismatch: got %v want %v", field, *gotPtr, exp.value) - } - } - - assertPath := func(field string, gotPtr *string, exp pathWant) { - t.Helper() - if !exp.enabled || exp.kind == pathSkip { - return - } - switch exp.kind { - case pathNil: - if gotPtr == nil { - return - } - if *gotPtr != "" { - t.Fatalf("%s expected nil/empty, got %q", field, *gotPtr) - } - case pathDevNull: - if gotPtr == nil { - t.Fatalf("%s expected %q, got nil", field, os.DevNull) - } - if *gotPtr != os.DevNull { - t.Fatalf("%s expected %q, got %q", field, os.DevNull, *gotPtr) - } - case pathTempFile: - if gotPtr == nil { - t.Fatalf("%s expected temp file path, got nil", field) - } - if strings.TrimSpace(*gotPtr) == "" { - t.Fatalf("%s expected non-empty temp file path", field) - } - if *gotPtr == os.DevNull { - t.Fatalf("%s expected temp file path, got %q", field, os.DevNull) - } - if _, statErr := os.Stat(*gotPtr); statErr != nil { - t.Fatalf("%s expected existing file at %q: %v", field, *gotPtr, statErr) - } - default: - t.Fatalf("unknown path kind %q for field %s", exp.kind, field) - } - } - - assertOptString("APIServer", got.APIServer, test.want.apiServer) - assertOptString("BearerToken", got.BearerToken, test.want.bearerToken) - assertOptBool("Insecure", got.Insecure, test.want.insecure) - assertOptString("Username", got.Username, test.want.username) - assertOptString("Password", got.Password, test.want.password) - - assertPath("KubeConfig", got.KubeConfig, test.want.kubeConfig) - assertPath("CAFile", got.CAFile, test.want.caFile) - assertPath("CertFile", got.CertFile, test.want.certFile) - assertPath("KeyFile", got.KeyFile, test.want.keyFile) - }) - } -} diff --git a/utils/kubernetes/client-config-getter.go b/utils/kubernetes/client-config-getter.go index 77df6eb45..ab76fa175 100644 --- a/utils/kubernetes/client-config-getter.go +++ b/utils/kubernetes/client-config-getter.go @@ -8,20 +8,37 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) type clientConfigRESTClientGetter struct { clientConfig clientcmd.ClientConfig } +type restConfigClientConfig struct { + restConfig *rest.Config +} + var _ genericclioptions.RESTClientGetter = (*clientConfigRESTClientGetter)(nil) +var _ clientcmd.ClientConfig = (*restConfigClientConfig)(nil) func newClientConfigRESTClientGetter(clientConfig clientcmd.ClientConfig) genericclioptions.RESTClientGetter { return &clientConfigRESTClientGetter{clientConfig: clientConfig} } +func newRESTConfigRESTClientGetter(config *rest.Config) genericclioptions.RESTClientGetter { + return newClientConfigRESTClientGetter(&restConfigClientConfig{ + restConfig: rest.CopyConfig(config), + }) +} + func (g *clientConfigRESTClientGetter) ToRESTConfig() (*rest.Config, error) { - return g.clientConfig.ClientConfig() + config, err := g.clientConfig.ClientConfig() + if err != nil { + return nil, err + } + configureRESTConfig(config) + return config, nil } func (g *clientConfigRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { @@ -51,3 +68,34 @@ func (g *clientConfigRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { func (g *clientConfigRESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig { return g.clientConfig } + +func (c *restConfigClientConfig) RawConfig() (clientcmdapi.Config, error) { + const connectionName = "meshkit-connection" + + config := clientcmdapi.NewConfig() + config.Clusters[connectionName] = &clientcmdapi.Cluster{ + Server: c.restConfig.Host, + TLSServerName: c.restConfig.ServerName, + InsecureSkipTLSVerify: c.restConfig.Insecure, + CertificateAuthority: c.restConfig.CAFile, + CertificateAuthorityData: c.restConfig.CAData, + DisableCompression: c.restConfig.DisableCompression, + } + config.Contexts[connectionName] = &clientcmdapi.Context{ + Cluster: connectionName, + } + config.CurrentContext = connectionName + return *config, nil +} + +func (c *restConfigClientConfig) ClientConfig() (*rest.Config, error) { + return rest.CopyConfig(c.restConfig), nil +} + +func (c *restConfigClientConfig) Namespace() (string, bool, error) { + return "default", false, nil +} + +func (c *restConfigClientConfig) ConfigAccess() clientcmd.ConfigAccess { + return nil +} diff --git a/utils/kubernetes/client-config-getter_test.go b/utils/kubernetes/client-config-getter_test.go index a6bcf3fd3..f80f953a0 100644 --- a/utils/kubernetes/client-config-getter_test.go +++ b/utils/kubernetes/client-config-getter_test.go @@ -1,12 +1,20 @@ package kubernetes import ( + "fmt" + "net/http" + "net/http/httptest" "os" + "path/filepath" "testing" - "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) +const execCredentialHelperEnv = "MESHKIT_EXEC_CREDENTIAL_HELPER" + func testExecKubeConfig() []byte { return []byte(`apiVersion: v1 kind: Config @@ -31,13 +39,72 @@ users: `) } +func renewableExecKubeConfig(t *testing.T, serverURL, stateFile string) []byte { + t.Helper() + + const ( + clusterName = "test-cluster" + contextName = "test-context" + userName = "test-user" + ) + config := clientcmdapi.NewConfig() + config.Clusters[clusterName] = &clientcmdapi.Cluster{ + Server: serverURL, + InsecureSkipTLSVerify: true, + } + config.AuthInfos[userName] = &clientcmdapi.AuthInfo{ + Exec: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: os.Args[0], + Args: []string{"-test.run=TestExecCredentialHelperProcess"}, + Env: []clientcmdapi.ExecEnvVar{ + {Name: execCredentialHelperEnv, Value: "1"}, + {Name: "MESHKIT_EXEC_CREDENTIAL_STATE_FILE", Value: stateFile}, + }, + InteractiveMode: clientcmdapi.NeverExecInteractiveMode, + }, + } + config.Contexts[contextName] = &clientcmdapi.Context{ + Cluster: clusterName, + AuthInfo: userName, + } + config.CurrentContext = contextName + + data, err := clientcmd.Write(*config) + if err != nil { + t.Fatalf("clientcmd.Write() error = %v", err) + } + return data +} + +func TestExecCredentialHelperProcess(t *testing.T) { + if os.Getenv(execCredentialHelperEnv) != "1" { + return + } + + stateFile := os.Getenv("MESHKIT_EXEC_CREDENTIAL_STATE_FILE") + token := "expired-token" + state := "1" + if _, err := os.Stat(stateFile); err == nil { + token = "fresh-token" + state = "2" + } + if err := os.WriteFile(stateFile, []byte(state), 0o600); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "write exec credential state: %v", err) + os.Exit(1) + } + + _, _ = fmt.Fprintf(os.Stdout, `{"apiVersion":"client.authentication.k8s.io/v1","kind":"ExecCredential","status":{"token":%q}}`, token) + os.Exit(0) +} + func TestNewRetainsKubeConfigLoader(t *testing.T) { client, err := New(testExecKubeConfig()) if err != nil { t.Fatalf("New() error = %v", err) } - loader := client.rawKubeConfigLoader() + loader := client.getRESTClientGetter().ToRawKubeConfigLoader() if loader == nil { t.Fatal("New() did not retain the kubeconfig loader") } @@ -73,46 +140,130 @@ func TestClientConfigRESTClientGetterPreservesLoader(t *testing.T) { if config.ExecProvider == nil || config.ExecProvider.Command != "credential-plugin" { t.Fatal("RESTClientGetter did not preserve exec authentication") } + if config.QPS != 50 || config.Burst != 100 { + t.Fatalf("REST config rate limits = (%v, %d), want (50, 100)", config.QPS, config.Burst) + } } -func TestHelmRESTClientGetterPrefersRetainedLoader(t *testing.T) { - config, loader, err := detectKubeConfig(testExecKubeConfig()) +func TestNewInitializesRESTClientGetter(t *testing.T) { + client, err := New(testExecKubeConfig()) if err != nil { - t.Fatalf("detectKubeConfig() error = %v", err) + t.Fatalf("New() error = %v", err) } - client := &Client{ - RestConfig: *config, - kubeConfigLoader: loader, + + getter := client.getRESTClientGetter() + if getter == nil { + t.Fatal("New() did not initialize a RESTClientGetter") } - getter, cleanup, err := client.helmRESTClientGetter() + config, err := getter.ToRESTConfig() if err != nil { - t.Fatalf("helmRESTClientGetter() error = %v", err) + t.Fatalf("ToRESTConfig() error = %v", err) + } + if config.ExecProvider == nil || config.ExecProvider.Command != "credential-plugin" { + t.Fatal("initialized RESTClientGetter lost exec authentication") + } + if client.RestConfig.QPS != 50 || client.RestConfig.Burst != 100 { + t.Fatalf("client rate limits = (%v, %d), want (50, 100)", client.RestConfig.QPS, client.RestConfig.Burst) } - defer cleanup() +} - if getter.ToRawKubeConfigLoader() != loader { - t.Fatal("Helm did not receive the retained kubeconfig loader") +func TestRESTClientGetterFallsBackForDirectClient(t *testing.T) { + client := &Client{RestConfig: rest.Config{ + Host: "https://cluster.example.com", + BearerToken: "test-token", + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + }, + QPS: 1, + Burst: 2, + }} + + getter := client.getRESTClientGetter() + config, err := getter.ToRESTConfig() + if err != nil { + t.Fatalf("ToRESTConfig() error = %v", err) + } + if config.Host != client.RestConfig.Host || config.BearerToken != client.RestConfig.BearerToken { + t.Fatalf("REST config = (%q, %q), want (%q, %q)", config.Host, config.BearerToken, client.RestConfig.Host, client.RestConfig.BearerToken) + } + if config.QPS != 50 || config.Burst != 100 { + t.Fatalf("REST config rate limits = (%v, %d), want (50, 100)", config.QPS, config.Burst) + } + + namespace, explicit, err := getter.ToRawKubeConfigLoader().Namespace() + if err != nil { + t.Fatalf("Namespace() error = %v", err) } - if _, ok := getter.(*genericclioptions.ConfigFlags); ok { - t.Fatal("Helm reconstructed ConfigFlags despite a retained kubeconfig loader") + if namespace != "default" || explicit { + t.Fatalf("Namespace() = (%q, %v), want (%q, false)", namespace, explicit, "default") + } + + rawConfig, err := getter.ToRawKubeConfigLoader().RawConfig() + if err != nil { + t.Fatalf("RawConfig() error = %v", err) + } + cluster := rawConfig.Clusters["meshkit-connection"] + if cluster == nil || cluster.Server != client.RestConfig.Host { + t.Fatalf("raw cluster = %#v, want server %q", cluster, client.RestConfig.Host) } } -func TestHelmRESTClientGetterFallsBackForDirectClient(t *testing.T) { - client := &Client{} +func TestRESTConfigGetterReturnsIndependentCopies(t *testing.T) { + getter := newRESTConfigRESTClientGetter(&rest.Config{ + Host: "https://cluster.example.com", + BearerToken: "test-token", + }) - getter, cleanup, err := client.helmRESTClientGetter() + first, err := getter.ToRESTConfig() if err != nil { - t.Fatalf("helmRESTClientGetter() error = %v", err) + t.Fatalf("first ToRESTConfig() error = %v", err) } - defer cleanup() + first.Host = "https://different.example.com" + first.BearerToken = "different-token" - configFlags, ok := getter.(*genericclioptions.ConfigFlags) - if !ok { - t.Fatalf("fallback getter type = %T, want *genericclioptions.ConfigFlags", getter) + second, err := getter.ToRESTConfig() + if err != nil { + t.Fatalf("second ToRESTConfig() error = %v", err) + } + if second.Host != "https://cluster.example.com" || second.BearerToken != "test-token" { + t.Fatalf("second REST config = (%q, %q), want original values", second.Host, second.BearerToken) + } +} + +func TestRESTClientGetterExecutesAndRenewsCredentials(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer fresh-token" { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"gitVersion":"v1.34.0"}`)) + })) + defer server.Close() + + stateFile := filepath.Join(t.TempDir(), "exec-credential-state") + client, err := New(renewableExecKubeConfig(t, server.URL, stateFile)) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + discoveryClient, err := client.getRESTClientGetter().ToDiscoveryClient() + if err != nil { + t.Fatalf("ToDiscoveryClient() error = %v", err) + } + if _, err := discoveryClient.ServerVersion(); err == nil { + t.Fatal("first ServerVersion() unexpectedly succeeded with the rejected credential") + } + if _, err := discoveryClient.ServerVersion(); err != nil { + t.Fatalf("second ServerVersion() error after credential renewal = %v", err) + } + + state, err := os.ReadFile(stateFile) + if err != nil { + t.Fatalf("ReadFile(%q) error = %v", stateFile, err) } - if configFlags.KubeConfig == nil || *configFlags.KubeConfig != os.DevNull { - t.Fatalf("fallback kubeconfig = %v, want %q", configFlags.KubeConfig, os.DevNull) + if string(state) != "2" { + t.Fatalf("exec credential helper state = %q, want %q", state, "2") } } diff --git a/utils/kubernetes/kubernetes.go b/utils/kubernetes/kubernetes.go index 3adf082d5..abd5f84ec 100644 --- a/utils/kubernetes/kubernetes.go +++ b/utils/kubernetes/kubernetes.go @@ -1,17 +1,17 @@ package kubernetes import ( + "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" ) type Client struct { RestConfig rest.Config `json:"restconfig,omitempty"` KubeClient *kubernetes.Clientset `json:"kubeclient,omitempty"` DynamicKubeClient dynamic.Interface `json:"dynamicKubeClient,omitempty"` - kubeConfigLoader clientcmd.ClientConfig + restClientGetter genericclioptions.RESTClientGetter } func New(kubeconfig []byte) (*Client, error) { @@ -19,8 +19,17 @@ func New(kubeconfig []byte) (*Client, error) { if err != nil { return nil, err } - restConfig.QPS = float32(50) - restConfig.Burst = int(100) + + var restClientGetter genericclioptions.RESTClientGetter + if kubeConfigLoader != nil { + restClientGetter = newClientConfigRESTClientGetter(kubeConfigLoader) + } else { + restClientGetter = newRESTConfigRESTClientGetter(restConfig) + } + restConfig, err = restClientGetter.ToRESTConfig() + if err != nil { + return nil, err + } // if insecure variable is kept true, allow that if restConfig.TLSClientConfig.Insecure { //nolint:staticcheck @@ -43,10 +52,20 @@ func New(kubeconfig []byte) (*Client, error) { RestConfig: *restConfig, DynamicKubeClient: dyclient, KubeClient: kclient, - kubeConfigLoader: kubeConfigLoader, + restClientGetter: restClientGetter, }, nil } -func (c *Client) rawKubeConfigLoader() clientcmd.ClientConfig { - return c.kubeConfigLoader +func configureRESTConfig(config *rest.Config) { + config.QPS = float32(50) + config.Burst = int(100) +} + +func (c *Client) getRESTClientGetter() genericclioptions.RESTClientGetter { + if c.restClientGetter != nil { + return c.restClientGetter + } + + // Preserve compatibility for clients constructed directly instead of through New. + return newRESTConfigRESTClientGetter(&c.RestConfig) }