From 12f04cfd77dd100a1cc0903f19e62feea10df777 Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Tue, 7 Apr 2026 18:29:50 +0530 Subject: [PATCH 1/7] HELM-683: Add basic auth support for OCI/HTTP chart install and fetch - Add basicAuthSecretName parameter to GetChartFromURL and InstallChartFromURL - Build RegistryClient with basic auth credentials for OCI registry pulls - Extract shared registryClientOptions from GetOCIRegistry for reuse - Add applyBasicAuthFromSecret helper to read credentials from K8s Secret - Strip OCI version tags before LocateChart to prevent duplicate tag errors - Wire basic_auth_secret_name through HandleChartGet and HandleHelmInstallAsync - Add OCI basic auth test cases with zot registry and htpasswd auth --- pkg/helm/actions/get_chart.go | 14 ++++- pkg/helm/actions/get_registry.go | 33 +++++++--- pkg/helm/actions/install_chart.go | 43 ++++++++++++- pkg/helm/actions/install_chart_test.go | 63 +++++++++++++++---- pkg/helm/actions/setup_test.go | 14 +++++ pkg/helm/actions/testdata/htpasswd | 1 + pkg/helm/actions/testdata/uploadOciCharts.sh | 44 ++++++++----- .../testdata/zot-config-basicauth.json | 19 ++++++ pkg/helm/actions/testdata/zot-stop.sh | 3 +- pkg/helm/actions/testdata/zotWithBasicAuth.sh | 9 +++ pkg/helm/handlers/handler_test.go | 4 +- pkg/helm/handlers/handlers.go | 13 ++-- pkg/helm/handlers/request.go | 2 + 13 files changed, 215 insertions(+), 47 deletions(-) create mode 100644 pkg/helm/actions/testdata/htpasswd create mode 100644 pkg/helm/actions/testdata/zot-config-basicauth.json create mode 100755 pkg/helm/actions/testdata/zotWithBasicAuth.sh diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index 18e80ecbf49..5cfb71d8aa3 100644 --- a/pkg/helm/actions/get_chart.go +++ b/pkg/helm/actions/get_chart.go @@ -63,13 +63,25 @@ func GetChart(url string, conf *action.Configuration, repositoryNamespace string return loader.Load(chartPath) } -func GetChartFromURL(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool) (*chart.Chart, error) { +// GetChartFromURL loads a chart from an OCI or direct HTTP(S) URL. basicAuthSecretName names a +// Secret in namespace with username and password keys when the registry requires authentication. +func GetChartFromURL(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool, basicAuthSecretName string) (*chart.Chart, error) { if !isValidChartURL(url) { return nil, fmt.Errorf("invalid chart URL: %s, must be oci:// URL or http(s)://*.tgz", url) } cmd := action.NewInstall(conf) cmd.Namespace = namespace + if err := applyBasicAuthFromSecret(cmd, coreClient, namespace, basicAuthSecretName); err != nil { + return nil, err + } + if basicAuthSecretName != "" { + rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) + if err != nil { + return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) + } + cmd.SetRegistryClient(rc) + } chartLocation, err := cmd.ChartPathOptions.LocateChart(url, settings) if err != nil { return nil, fmt.Errorf("error getting chart from URL: %v", err) diff --git a/pkg/helm/actions/get_registry.go b/pkg/helm/actions/get_registry.go index a1014120d36..6683d85d192 100644 --- a/pkg/helm/actions/get_registry.go +++ b/pkg/helm/actions/get_registry.go @@ -12,14 +12,8 @@ import ( // newRegistryClient is a package-level variable to allow mocking in tests var newRegistryClient = registry.NewClient -func GetDefaultOCIRegistry(conf *action.Configuration) error { - return GetOCIRegistry(conf, false, false) -} - -func GetOCIRegistry(conf *action.Configuration, skipTLSVerify bool, plainHTTP bool) error { - if conf == nil { - return fmt.Errorf("action configuration cannot be nil") - } +// registryClientOptions returns the same options used by GetOCIRegistry for TLS / plain-HTTP behavior. +func registryClientOptions(skipTLSVerify, plainHTTP bool) []registry.ClientOption { opts := []registry.ClientOption{ registry.ClientOptDebug(false), } @@ -33,7 +27,28 @@ func GetOCIRegistry(conf *action.Configuration, skipTLSVerify bool, plainHTTP bo } opts = append(opts, registry.ClientOptHTTPClient(&http.Client{Transport: transport})) } - registryClient, err := newRegistryClient(opts...) + return opts +} + +// RegistryClientWithBasicAuth builds a registry.Client with the same TLS/plain-HTTP settings as +// GetDefaultOCIRegistry (skipTLSVerify=false, plainHTTP=false) plus OCI basic auth. +// Helm's OCI getter uses Configuration.RegistryClient when set and does not apply ChartPathOptions +// username/password to that client; credentials must be set on the registry client via ClientOptBasicAuth. +func RegistryClientWithBasicAuth(skipTLSVerify, plainHTTP bool, username, password string) (*registry.Client, error) { + opts := registryClientOptions(skipTLSVerify, plainHTTP) + opts = append(opts, registry.ClientOptBasicAuth(username, password)) + return newRegistryClient(opts...) +} + +func GetDefaultOCIRegistry(conf *action.Configuration) error { + return GetOCIRegistry(conf, false, false) +} + +func GetOCIRegistry(conf *action.Configuration, skipTLSVerify bool, plainHTTP bool) error { + if conf == nil { + return fmt.Errorf("action configuration cannot be nil") + } + registryClient, err := newRegistryClient(registryClientOptions(skipTLSVerify, plainHTTP)...) if err != nil { return fmt.Errorf("failed to create registry client: %w", err) } diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index dadfd8910b6..1f6792c5996 100644 --- a/pkg/helm/actions/install_chart.go +++ b/pkg/helm/actions/install_chart.go @@ -259,9 +259,33 @@ func InstallChartAsync(ns, name, url string, vals map[string]interface{}, conf * return &secret, nil } +// applyBasicAuthFromSecret sets cmd.Username and cmd.Password from a Secret in ns with +// keys "username" and "password" (same convention as HelmChartRepository connectionConfig). +func applyBasicAuthFromSecret(cmd *action.Install, coreClient corev1client.CoreV1Interface, ns, secretName string) error { + if secretName == "" { + return nil + } + secret, err := coreClient.Secrets(ns).Get(context.TODO(), secretName, v1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get secret %q from namespace %q: %w", secretName, ns, err) + } + u, uok := secret.Data[username] + p, pok := secret.Data[password] + if !uok { + return fmt.Errorf("failed to find %q key in secret %q/%q", username, ns, secretName) + } + if !pok { + return fmt.Errorf("failed to find %q key in secret %q/%q", password, ns, secretName) + } + cmd.Username = string(u) + cmd.Password = string(p) + return nil +} + // InstallChartFromURL installs a chart from an OCI or direct HTTP(S) chart URL. // If not provided, version is extracted from the OCI URL tag when applicable. -func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string) (*kv1.Secret, error) { +// basicAuthSecretName names a Secret in ns containing username and password keys for registry auth. +func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string, basicAuthSecretName string) (*kv1.Secret, error) { if !isValidChartURL(url) { return nil, fmt.Errorf("invalid chart URL: %s, must be oci:// URL or http(s)://*.tgz", url) @@ -271,10 +295,27 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf cmd.ReleaseName = name cmd.Namespace = ns + if err := applyBasicAuthFromSecret(cmd, coreClient, ns, basicAuthSecretName); err != nil { + return nil, err + } + // OCI pulls use conf.RegistryClient when set; the getter does not merge ChartPathOptions username/password + // onto that client (see helm ocigetter). Rebuild the client with basic auth when credentials are supplied. + if basicAuthSecretName != "" { + rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) + if err != nil { + return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) + } + cmd.SetRegistryClient(rc) + } + // Set version so LocateChart (and Helm OCI) resolve the correct chart tag; matches InstallChart behavior. if version == "" { version = chartVersionFromURL(url) } + // Remove version from OCI URLs as LocateChart will use chartPathOptions.Version to resolve tag. + if strings.HasPrefix(url, "oci://") { + url = strings.TrimSuffix(url, ":"+version) + } cmd.ChartPathOptions.Version = version cp, err := cmd.ChartPathOptions.LocateChart(url, settings) diff --git a/pkg/helm/actions/install_chart_test.go b/pkg/helm/actions/install_chart_test.go index dffe9f9b5eb..c83e4755678 100644 --- a/pkg/helm/actions/install_chart_test.go +++ b/pkg/helm/actions/install_chart_test.go @@ -405,14 +405,17 @@ func TestInstallChartAsync(t *testing.T) { func TestInstallChartFromURL(t *testing.T) { tests := []struct { - testName string - releaseName string - chartPath string - chartName string - chartVersion string - plainHTTP bool - skipTLSVerify bool - expectError bool + testName string + releaseName string + chartPath string + chartName string + chartVersion string + plainHTTP bool + skipTLSVerify bool + basicAuthSecretName string + basicAuthUser string + basicAuthPass string + expectError bool }{ { testName: "valid HTTP chart URL", @@ -444,6 +447,32 @@ func TestInstallChartFromURL(t *testing.T) { skipTLSVerify: true, expectError: true, }, + { + testName: "OCI chart with basic auth", + releaseName: "basicauth-oci", + chartPath: "oci://localhost:5001/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "oci-auth-secret", + basicAuthUser: "AzureDiamond", + basicAuthPass: "hunter2", + expectError: false, + }, + { + testName: "OCI chart with wrong basic auth credentials", + releaseName: "badauth-oci", + chartPath: "oci://localhost:5001/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "bad-auth-secret", + basicAuthUser: "wrong-user", + basicAuthPass: "wrong-pass", + expectError: true, + }, } for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { @@ -459,18 +488,28 @@ func TestInstallChartFromURL(t *testing.T) { require.NoError(t, err) objs := []runtime.Object{} + if tt.basicAuthSecretName != "" { + objs = append(objs, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.basicAuthSecretName, + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "username": []byte(tt.basicAuthUser), + "password": []byte(tt.basicAuthPass), + }, + }) + } clientInterface := k8sfake.NewSimpleClientset(objs...) coreClient := clientInterface.CoreV1() if tt.expectError { - rel, err := InstallChartFromURL("test-namespace", tt.releaseName, tt.chartPath, nil, actionConfig, coreClient, tt.chartVersion) + rel, err := InstallChartFromURL("test-namespace", tt.releaseName, tt.chartPath, nil, actionConfig, coreClient, tt.chartVersion, tt.basicAuthSecretName) require.Error(t, err) require.Nil(t, rel) return } - // For valid URLs: create the release secret in a background goroutine - // to simulate what Helm's cmd.Run would do, unblocking getSecret's Watch. secretName := fmt.Sprintf("sh.helm.release.v1.%v.v1", tt.releaseName) go func() { time.Sleep(2 * time.Second) @@ -494,7 +533,7 @@ func TestInstallChartFromURL(t *testing.T) { secretsDriver.Create(secretName, &r) }() - rel, err := InstallChartFromURL("test-namespace", tt.releaseName, tt.chartPath, nil, actionConfig, coreClient, tt.chartVersion) + rel, err := InstallChartFromURL("test-namespace", tt.releaseName, tt.chartPath, nil, actionConfig, coreClient, tt.chartVersion, tt.basicAuthSecretName) require.NoError(t, err) require.NotNil(t, rel) require.Equal(t, secretName, rel.ObjectMeta.Name) diff --git a/pkg/helm/actions/setup_test.go b/pkg/helm/actions/setup_test.go index 2fd9f16bcc7..b797a308478 100644 --- a/pkg/helm/actions/setup_test.go +++ b/pkg/helm/actions/setup_test.go @@ -104,6 +104,9 @@ func startTests(m *testing.M) (exitCode int) { if err := setupTestBasicAuth(); err != nil { panic(err) } + if err := setupTestOCIBasicAuth(); err != nil { + panic(err) + } return m.Run() } @@ -168,6 +171,17 @@ func setupTestBasicAuth() error { return nil } +func setupTestOCIBasicAuth() error { + if err := ExecuteScript("./testdata/zotWithBasicAuth.sh", false); err != nil { + return err + } + time.Sleep(5 * time.Second) + if err := ExecuteScript("./testdata/uploadOciCharts.sh", true, "--basic-auth"); err != nil { + return err + } + return nil +} + func ExecuteScript(filepath string, waitForCompletion bool, args ...string) error { tlsCmd := exec.Command(filepath, args...) tlsCmd.Stdout = os.Stdout diff --git a/pkg/helm/actions/testdata/htpasswd b/pkg/helm/actions/testdata/htpasswd new file mode 100644 index 00000000000..f301fb5dfba --- /dev/null +++ b/pkg/helm/actions/testdata/htpasswd @@ -0,0 +1 @@ +AzureDiamond:$2y$05$R3muloAZfYflQ1dV5i8rZuyddR.X3CsFSBWO4jNy19MaCmiWslt3C diff --git a/pkg/helm/actions/testdata/uploadOciCharts.sh b/pkg/helm/actions/testdata/uploadOciCharts.sh index f4c30bffc5b..923c6165e8f 100755 --- a/pkg/helm/actions/testdata/uploadOciCharts.sh +++ b/pkg/helm/actions/testdata/uploadOciCharts.sh @@ -1,14 +1,12 @@ #!/bin/bash -e -# Upload Helm charts as OCI artifacts to zot registry (with TLS) +# Upload Helm charts as OCI artifacts to zot registry +# Usage: uploadOciCharts.sh --tls | --no-tls | --basic-auth # Change to the script's directory (pkg/helm/actions/testdata/) cd "$(dirname "$0")" - -if [[ $1 == "--tls" ]]; then - REGISTRY="localhost:5443" -else - REGISTRY="localhost:5000" -fi +export HELM_CONFIG_HOME="${TMPDIR:-/tmp}/helm-config" +export HELM_REGISTRY_CONFIG="${HELM_CONFIG_HOME}/registry/config.json" +mkdir -p "${HELM_CONFIG_HOME}/registry" CACERT="../cacert.pem" CHARTS_DIR="../../testdata" @@ -26,12 +24,28 @@ else fi # Push charts to OCI registry using helm push -if [[ $1 == "--tls" ]]; then - echo "Pushing mariadb-7.3.5.tgz to oci://$REGISTRY/helm-charts..." - $HELM push $CHARTS_DIR/mariadb-7.3.5.tgz oci://$REGISTRY/helm-charts --ca-file=$CACERT -else - echo "Pushing mychart-0.1.0.tgz to oci://$REGISTRY/helm-charts..." - $HELM push $CHARTS_DIR/mychart-0.1.0.tgz oci://$REGISTRY/helm-charts --plain-http -fi - +mode="${1:-"--no-tls"}" +case $mode in + "--tls") + REGISTRY="localhost:5443" + echo "Pushing mariadb-7.3.5.tgz to oci://$REGISTRY/helm-charts..." + $HELM push $CHARTS_DIR/mariadb-7.3.5.tgz oci://$REGISTRY/helm-charts --ca-file=$CACERT + ;; + "--basic-auth") + REGISTRY="localhost:5001" + echo "Logging in to oci://$REGISTRY with basic auth..." + echo "hunter2" | $HELM registry login $REGISTRY --username AzureDiamond --password-stdin --plain-http + echo "Pushing mychart-0.1.0.tgz to oci://$REGISTRY/helm-charts..." + $HELM push $CHARTS_DIR/mychart-0.1.0.tgz oci://$REGISTRY/helm-charts --plain-http + ;; + "--no-tls" ) + REGISTRY="localhost:5000" + echo "Pushing mychart-0.1.0.tgz to oci://$REGISTRY/helm-charts..." + $HELM push $CHARTS_DIR/mychart-0.1.0.tgz oci://$REGISTRY/helm-charts --plain-http + ;; + *) + echo "Usage: uploadOciCharts.sh --tls | --no-tls | --basic-auth" >&2 + exit 2 + ;; +esac echo "Charts pushed successfully!" diff --git a/pkg/helm/actions/testdata/zot-config-basicauth.json b/pkg/helm/actions/testdata/zot-config-basicauth.json new file mode 100644 index 00000000000..c557eaeab1b --- /dev/null +++ b/pkg/helm/actions/testdata/zot-config-basicauth.json @@ -0,0 +1,19 @@ +{ + "distSpecVersion": "1.1.0", + "storage": { + "rootDirectory": "./zot-storage-5001", + "gc": false + }, + "http": { + "address": "127.0.0.1", + "port": "5001", + "auth": { + "htpasswd": { + "path": "./testdata/htpasswd" + } + } + }, + "log": { + "level": "debug" + } +} diff --git a/pkg/helm/actions/testdata/zot-stop.sh b/pkg/helm/actions/testdata/zot-stop.sh index 3bd9da392ad..77e4ed9df79 100755 --- a/pkg/helm/actions/testdata/zot-stop.sh +++ b/pkg/helm/actions/testdata/zot-stop.sh @@ -2,4 +2,5 @@ kill -TERM $(< zot.pid) || echo "Zot is not currently running." kill -TERM $(< zot-no-tls.pid) || echo "Zot is not currently running." -rm -f zot.pid zot-no-tls.pid +kill -TERM "$(< zot-basicauth.pid)" || echo "Zot (basic auth) is not currently running." +rm -f zot.pid zot-no-tls.pid zot-basicauth.pid diff --git a/pkg/helm/actions/testdata/zotWithBasicAuth.sh b/pkg/helm/actions/testdata/zotWithBasicAuth.sh new file mode 100755 index 00000000000..5dfbb6b0060 --- /dev/null +++ b/pkg/helm/actions/testdata/zotWithBasicAuth.sh @@ -0,0 +1,9 @@ +#!/bin/bash -e +# Start zot OCI registry server with basic auth (htpasswd) +GOOS=${GOOS:-$(go env GOOS)} +GOARCH=${GOARCH:-$(go env GOARCH)} + +mkdir -p ./zot-storage-5001 + +./$GOOS-$GOARCH/zot serve ./testdata/zot-config-basicauth.json & +echo $! > ./zot-basicauth.pid diff --git a/pkg/helm/handlers/handler_test.go b/pkg/helm/handlers/handler_test.go index 7be5df7ed2d..27b9d5e0def 100644 --- a/pkg/helm/handlers/handler_test.go +++ b/pkg/helm/handlers/handler_test.go @@ -201,8 +201,8 @@ func getFakeActionConfigurations(string, string, string, *http.RoundTripper) *ac } } -func fakeInstallChartFromURL(mockedSecret *kv1.Secret, err error) func(ns string, name string, url string, values map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string) (*kv1.Secret, error) { - return func(ns string, name string, url string, values map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string) (*kv1.Secret, error) { +func fakeInstallChartFromURL(mockedSecret *kv1.Secret, err error) func(ns string, name string, url string, values map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string, basicAuthSecretName string) (*kv1.Secret, error) { + return func(ns string, name string, url string, values map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string, basicAuthSecretName string) (*kv1.Secret, error) { return mockedSecret, err } } diff --git a/pkg/helm/handlers/handlers.go b/pkg/helm/handlers/handlers.go index 7d166cc8f50..6d37df337ec 100644 --- a/pkg/helm/handlers/handlers.go +++ b/pkg/helm/handlers/handlers.go @@ -64,7 +64,7 @@ type helmHandlers struct { renderManifests func(string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, string, string, bool) (string, error) installChartAsync func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*kv1.Secret, error) installChart func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*release.Release, error) - installChartFromURL func(string, string, string, map[string]interface{}, *action.Configuration, corev1client.CoreV1Interface, string) (*kv1.Secret, error) + installChartFromURL func(string, string, string, map[string]interface{}, *action.Configuration, corev1client.CoreV1Interface, string, string) (*kv1.Secret, error) listReleases func(*action.Configuration, bool) ([]*release.Release, error) upgradeReleaseAsync func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*kv1.Secret, error) upgradeRelease func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*release.Release, error) @@ -73,7 +73,7 @@ type helmHandlers struct { rollbackRelease func(string, int, *action.Configuration) (*release.Release, error) getRelease func(string, *action.Configuration) (*release.Release, error) getChart func(chartUrl string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool, indexEntry string) (*chart.Chart, error) - getChartFromURL func(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool) (*chart.Chart, error) + getChartFromURL func(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool, basicAuthSecretName string) (*chart.Chart, error) getReleaseHistory func(releaseName string, conf *action.Configuration) ([]*release.Release, error) newProxy func(bearerToken string) (chartproxy.Proxy, error) } @@ -159,7 +159,7 @@ func (h *helmHandlers) HandleHelmInstallAsync(user *auth.User, w http.ResponseWr } if req.NoRepo { - resp, err := h.installChartFromURL(namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.CoreClient, req.ChartVersion) + resp, err := h.installChartFromURL(namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.CoreClient, req.ChartVersion, req.BasicAuthSecretName) if err != nil { serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to install helm chart: %v", err)}) return @@ -229,12 +229,13 @@ func (h *helmHandlers) HandleChartGet(user *auth.User, w http.ResponseWriter, r namespace := params.Get("namespace") indexEntry := params.Get("indexEntry") noRepo := params.Get("noRepo") == "true" + basicAuthSecretName := params.Get("basic_auth_secret_name") if namespace == "" { namespace = "default" } - conf := h.getActionConfigurations(h.ApiServerHost, "default", user.Token, &h.Transport) + conf := h.getActionConfigurations(h.ApiServerHost, namespace, user.Token, &h.Transport) handlerClients, err := NewHandlerClients(conf) if err != nil { serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: err.Error()}) @@ -247,7 +248,7 @@ func (h *helmHandlers) HandleChartGet(user *auth.User, w http.ResponseWriter, r serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: "chart URL is required"}) return } - resp, err = h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true) + resp, err = h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true, basicAuthSecretName) } else { resp, err = h.getChart(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true, indexEntry) } @@ -467,7 +468,7 @@ func (h *helmHandlers) HandleURLChartGet(user *auth.User, w http.ResponseWriter, serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: err.Error()}) return } - resp, err := h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true) + resp, err := h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true, params.Get("basic_auth_secret_name")) if err != nil { serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to retrieve chart: %v", err)}) return diff --git a/pkg/helm/handlers/request.go b/pkg/helm/handlers/request.go index cfc60ae5cbb..5c9385cf6b5 100644 --- a/pkg/helm/handlers/request.go +++ b/pkg/helm/handlers/request.go @@ -9,6 +9,8 @@ type HelmRequest struct { Version int `json:"version"` IndexEntry string `json:"indexEntry"` NoRepo bool `json:"noRepo"` + // BasicAuthSecretName is optional; names a Secret in Namespace with keys username and password for OCI/HTTP chart pull when NoRepo is true. + BasicAuthSecretName string `json:"basic_auth_secret_name"` } type HelmVerifierRequest struct { From 03eaa6c2f7c806512e7e14b6946850e3e746b11b Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Fri, 1 May 2026 11:18:14 +0530 Subject: [PATCH 2/7] HELM-703: Add secret selector UI for Helm URL chart basic auth - Add typeahead secret dropdown to HelmURLChartForm - Surface errors for invalid secrets with explanatory description - Pass basicAuthSecretName through fetchChartData and install API calls - Append namespace and basic_auth_secret_name to chart fetch query string - Display selected secret as read-only field on the install confirmation step - Add basicAuthSecretName to HelmURLChartFormData type and helm-utils --- .../helm-plugin/locales/en/helm-plugin.json | 4 ++ .../forms/url-chart/HelmURLChartForm.tsx | 59 +++++++++++++++- .../url-chart/HelmURLChartInstallPage.tsx | 68 ++++++++++++------- .../forms/url-chart/HelmURLInstallForm.tsx | 51 ++++++++++++++ .../src/components/forms/url-chart/types.ts | 1 + .../helm-plugin/src/utils/helm-utils.ts | 2 + 6 files changed, 159 insertions(+), 26 deletions(-) diff --git a/frontend/packages/helm-plugin/locales/en/helm-plugin.json b/frontend/packages/helm-plugin/locales/en/helm-plugin.json index df75c1f953e..947a505fdc7 100644 --- a/frontend/packages/helm-plugin/locales/en/helm-plugin.json +++ b/frontend/packages/helm-plugin/locales/en/helm-plugin.json @@ -124,12 +124,16 @@ "The OCI URL or HTTP/HTTPS tar file for the Helm chart; for example - oci://registry.example.com/charts/mychart or https://example.com/chart-1.0.0.tgz.": "The OCI URL or HTTP/HTTPS tar file for the Helm chart; for example - oci://registry.example.com/charts/mychart or https://example.com/chart-1.0.0.tgz.", "Unique name for Helm release.": "Unique name for Helm release.", "The version of chart to install.": "The version of chart to install.", + "Secret for basic authentication.": "Secret for basic authentication.", + "Select a secret": "Select a secret", + "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication": "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication", "Next": "Next", "Install Helm chart from Helm registry.": "Install Helm chart from Helm registry.", "Helm release": "Helm release", "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.": "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.", "Configure Helm release": "Configure Helm release", "Version": "Version", + "Secret for basic authentication": "Secret for basic authentication", "Install": "Install", "Back": "Back", "Display Name": "Display Name", diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx index 85443f2a883..31458acbe85 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx @@ -1,10 +1,21 @@ import type { FC } from 'react'; -import { useEffect } from 'react'; +import { useEffect, useMemo } from 'react'; import { TextInputTypes, Grid, GridItem } from '@patternfly/react-core'; import type { FormikProps } from 'formik'; +import * as fuzzy from 'fuzzysearch'; import { useTranslation } from 'react-i18next'; import FormSection from '@console/dev-console/src/components/import/section/FormSection'; -import { InputField, FormFooter, FormBody, FormHeader, FlexForm } from '@console/shared'; +import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; +import { SecretModel } from '@console/internal/models'; +import type { K8sResourceKind } from '@console/internal/module/k8s'; +import { + InputField, + FormFooter, + FormBody, + FormHeader, + FlexForm, + ResourceDropdownField, +} from '@console/shared'; import type { HelmURLChartFormData } from './types'; export interface HelmURLChartFormProps { @@ -17,6 +28,7 @@ const HelmURLChartForm: FC & HelmURLChartFormP status, isSubmitting, onNext, + namespace, isValid, dirty, values, @@ -25,6 +37,34 @@ const HelmURLChartForm: FC & HelmURLChartFormP }) => { const { t } = useTranslation(); + const autocompleteFilter = (strText: string, item: any): boolean => + fuzzy(strText, item?.props?.name); + + const watchedResources = useK8sWatchResources<{ + secrets: K8sResourceKind[]; + }>({ + secrets: { + isList: true, + kind: SecretModel.kind, + namespace, + optional: true, + }, + }); + const secretResources = useMemo( + () => [ + { + data: watchedResources.secrets?.data, + loaded: watchedResources.secrets?.loaded, + loadError: watchedResources.secrets?.loadError, + kind: SecretModel.kind, + }, + ], + [ + watchedResources.secrets?.data, + watchedResources.secrets?.loaded, + watchedResources.secrets?.loadError, + ], + ); const isNextDisabled = !isValid || !dirty || isSubmitting; // Auto-populate releaseName and chartVersion from URL @@ -116,6 +156,21 @@ const HelmURLChartForm: FC & HelmURLChartFormP data-test="oci-chart-version" /> + + + diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx index 7ec662c6324..f06a06a4d77 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx @@ -65,38 +65,48 @@ const HelmURLChartInstallPage: FunctionComponent = () => { chartURL: '', chartVersion: '', namespace, + basicAuthSecretName: '', }; - const fetchChartData = useCallback(async (chartURL: string, chartVersion: string) => { - setIsLoadingChart(true); - setChartError(null); + const fetchChartData = useCallback( + async (chartURL: string, chartVersion: string, basicAuthSecretName: string) => { + setIsLoadingChart(true); + setChartError(null); - try { - const fullChartURL = getFullChartURL(chartURL, chartVersion); - const apiUrl = `/api/helm/chart?url=${encodeURIComponent(fullChartURL)}&noRepo=true`; + try { + const fullChartURL = getFullChartURL(chartURL, chartVersion); + let authParam = ''; + if (basicAuthSecretName) { + authParam = `&basic_auth_secret_name=${encodeURIComponent(basicAuthSecretName)}`; + } + const apiUrl = `/api/helm/chart?url=${encodeURIComponent( + fullChartURL, + )}&noRepo=true&namespace=${namespace}${authParam}`; - const res = await coFetchJSON(apiUrl); - const chart: HelmChart = res?.chart || res; - const valuesYAML = getChartValuesYAML(chart); - const valuesJSON = chart?.values ?? {}; - const valuesSchema = chart?.schema && JSON.parse(atob(chart?.schema)); + const res = await coFetchJSON(apiUrl); + const chart: HelmChart = res?.chart || res; + const valuesYAML = getChartValuesYAML(chart); + const valuesJSON = chart?.values ?? {}; + const valuesSchema = chart?.schema && JSON.parse(atob(chart?.schema)); - setInitialYamlData(valuesYAML); - setInitialFormData(valuesJSON as Record); - setInitialFormSchema(valuesSchema); - setChartHasValues(!!valuesYAML); - setChartData(chart); - } catch (e) { - setChartError(e as Error); - } finally { - setIsLoadingChart(false); - } - }, []); + setInitialYamlData(valuesYAML); + setInitialFormData(valuesJSON as Record); + setInitialFormSchema(valuesSchema); + setChartHasValues(!!valuesYAML); + setChartData(chart); + } catch (e) { + setChartError(e as Error); + } finally { + setIsLoadingChart(false); + } + }, + [namespace], + ); const handleNextStep = useCallback( (values: HelmURLChartFormData) => { setChartDetails(values); - fetchChartData(values.chartURL, values.chartVersion); + fetchChartData(values.chartURL, values.chartVersion, values.basicAuthSecretName); setCurrentStep(WizardStep.ConfigureInstall); }, [fetchChartData], @@ -112,7 +122,15 @@ const HelmURLChartInstallPage: FunctionComponent = () => { values: HelmURLInstallFormData, actions: FormikHelpers, ) => { - const { releaseName, chartURL, chartVersion, yamlData, formData, editorType } = values; + const { + releaseName, + chartURL, + chartVersion, + yamlData, + formData, + editorType, + basicAuthSecretName, + } = values; let valuesObj: Record | undefined; if (editorType === EditorType.Form) { @@ -153,6 +171,7 @@ const HelmURLChartInstallPage: FunctionComponent = () => { chart_url: fullChartURL, // eslint-disable-line @typescript-eslint/naming-convention ...(chartVersion ? { chart_version: chartVersion } : {}), // eslint-disable-line @typescript-eslint/naming-convention ...(valuesObj ? { values: valuesObj } : {}), + ...(basicAuthSecretName ? { basic_auth_secret_name: basicAuthSecretName } : {}), // eslint-disable-line @typescript-eslint/naming-convention noRepo: true, }; @@ -197,6 +216,7 @@ const HelmURLChartInstallPage: FunctionComponent = () => { chartURL: chartDetails?.chartURL || '', chartVersion: chartDetails?.chartVersion || '', namespace, + basicAuthSecretName: chartDetails?.basicAuthSecretName || '', chartName: chartData?.metadata?.name || '', appVersion: chartData?.metadata?.appVersion || '', chartReadme: getChartReadme(chartData), diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx index 37f5031977b..6c622e21ec2 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx @@ -2,12 +2,17 @@ import type { ReactNode, FC } from 'react'; import { useMemo } from 'react'; import { TextInputTypes, Grid, GridItem, Button, Alert } from '@patternfly/react-core'; import type { FormikProps } from 'formik'; +import * as fuzzy from 'fuzzysearch'; import * as _ from 'lodash'; import { Trans, useTranslation } from 'react-i18next'; import FormSection from '@console/dev-console/src/components/import/section/FormSection'; import { useTheme } from '@console/internal/components/ThemeProvider'; +import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; +import { SecretModel } from '@console/internal/models'; +import type { K8sResourceKind } from '@console/internal/module/k8s'; import { InputField, + ResourceDropdownField, FormFooter, FormBody, CodeEditorField, @@ -37,12 +42,42 @@ const HelmURLInstallForm: FC & HelmURLInstal values, chartMetaDescription, chartError, + namespace, onBack, }) => { const { t } = useTranslation(); const { theme } = useTheme(); const { chartReadme, formData, formSchema } = values; + const autocompleteFilter = (strText: string, item: string): boolean => fuzzy(strText, item); + + const watchedResources = useK8sWatchResources<{ + secrets: K8sResourceKind[]; + }>({ + secrets: { + isList: true, + kind: SecretModel.kind, + namespace, + optional: true, + }, + }); + + const secretResources = useMemo( + () => [ + { + data: watchedResources.secrets?.data, + loaded: watchedResources.secrets?.loaded, + loadError: watchedResources.secrets?.loadError, + kind: SecretModel.kind, + }, + ], + [ + watchedResources.secrets?.data, + watchedResources.secrets?.loaded, + watchedResources.secrets?.loadError, + ], + ); + const helmReadmeModalLauncher = useHelmReadmeModalLauncher({ readme: chartReadme, theme, @@ -147,6 +182,22 @@ const HelmURLInstallForm: FC & HelmURLInstal data-test="chart-version" /> + + + {!chartError && diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/types.ts b/frontend/packages/helm-plugin/src/components/forms/url-chart/types.ts index 06975852dd5..7ae022819a1 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/types.ts +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/types.ts @@ -6,6 +6,7 @@ export interface HelmURLChartFormData { chartURL: string; chartVersion: string; namespace: string; + basicAuthSecretName?: string; } export interface HelmURLInstallFormData extends HelmURLChartFormData { diff --git a/frontend/packages/helm-plugin/src/utils/helm-utils.ts b/frontend/packages/helm-plugin/src/utils/helm-utils.ts index fed34e69228..83f523f5963 100644 --- a/frontend/packages/helm-plugin/src/utils/helm-utils.ts +++ b/frontend/packages/helm-plugin/src/utils/helm-utils.ts @@ -356,6 +356,7 @@ export const installChartFromURL = ( chartURL: string, chartVersion?: string, values?: Record, + basicAuthSecretName?: string, ) => { return coFetchJSON.post('/api/helm/release/async', { namespace, @@ -363,6 +364,7 @@ export const installChartFromURL = ( chart_url: chartURL, // eslint-disable-line @typescript-eslint/naming-convention ...(chartVersion ? { chart_version: chartVersion } : {}), // eslint-disable-line @typescript-eslint/naming-convention ...(values ? { values } : {}), + ...(basicAuthSecretName ? { basic_auth_secret_name: basicAuthSecretName } : {}), // eslint-disable-line @typescript-eslint/naming-convention noRepo: true, }); }; From cba6e35412d500e0924fe5437b8fa797145a03ee Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Fri, 1 May 2026 16:33:40 +0530 Subject: [PATCH 3/7] HELM-703: Remove unused useTheme import and theme prop from HelmURLInstallForm - Remove stale useTheme import that caused TS6133 build error - Drop theme property from useHelmReadmeModalLauncher (not in Props type) - Pass namespace to HelmURLInstallForm for ResourceDropdownField --- .../src/components/forms/url-chart/HelmURLInstallForm.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx index 39cd01bc239..d9427908202 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx @@ -6,7 +6,6 @@ import * as fuzzy from 'fuzzysearch'; import * as _ from 'lodash'; import { Trans, useTranslation } from 'react-i18next'; import FormSection from '@console/dev-console/src/components/import/section/FormSection'; -import { useTheme } from '@console/internal/components/ThemeProvider'; import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; import { SecretModel } from '@console/internal/models'; import type { K8sResourceKind } from '@console/internal/module/k8s'; @@ -79,7 +78,6 @@ const HelmURLInstallForm: FC & HelmURLInstal const helmReadmeModalLauncher = useHelmReadmeModalLauncher({ readme: chartReadme, - theme, }); const isSubmitDisabled = isSubmitting || !_.isEmpty(errors) || !!chartError; From fa68e0684098923c36ec9758a54f53aaa4d09648 Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Fri, 8 May 2026 22:43:48 +0530 Subject: [PATCH 4/7] HELM-683: Improve basic auth error handling and test coverage Move the empty-secret-name guard from applyBasicAuthFromSecret to its call sites in InstallChartFromURL and GetChartFromURL so the function can assume it always has work to do. Add test cases for missing secrets, malformed secrets (missing username/password keys), and wrong credentials over both OCI and HTTP. Replace exact error string matching with ErrorContains for more resilient assertions. --- .../helm-plugin/locales/en/helm-plugin.json | 3 +- .../forms/url-chart/HelmURLChartForm.tsx | 2 +- .../url-chart/HelmURLChartInstallPage.tsx | 7 +- pkg/helm/actions/get_chart.go | 6 +- pkg/helm/actions/get_registry.go | 2 +- pkg/helm/actions/install_chart.go | 11 +- pkg/helm/actions/install_chart_test.go | 129 ++++++++++++++---- pkg/helm/actions/testdata/uploadOciCharts.sh | 3 +- pkg/helm/actions/testdata/zot-stop.sh | 6 +- 9 files changed, 118 insertions(+), 51 deletions(-) diff --git a/frontend/packages/helm-plugin/locales/en/helm-plugin.json b/frontend/packages/helm-plugin/locales/en/helm-plugin.json index dce32e8cc96..17e3969b140 100644 --- a/frontend/packages/helm-plugin/locales/en/helm-plugin.json +++ b/frontend/packages/helm-plugin/locales/en/helm-plugin.json @@ -126,7 +126,7 @@ "The OCI URL or HTTP/HTTPS tar file for the Helm chart; for example - oci://registry.example.com/charts/mychart or https://example.com/chart-1.0.0.tgz.": "The OCI URL or HTTP/HTTPS tar file for the Helm chart; for example - oci://registry.example.com/charts/mychart or https://example.com/chart-1.0.0.tgz.", "Unique name for Helm release.": "Unique name for Helm release.", "The version of chart to install.": "The version of chart to install.", - "Secret for basic authentication.": "Secret for basic authentication.", + "Secret for basic authentication": "Secret for basic authentication", "Select a secret": "Select a secret", "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication": "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication", "Next": "Next", @@ -135,7 +135,6 @@ "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.": "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.", "Configure Helm release": "Configure Helm release", "Version": "Version", - "Secret for basic authentication": "Secret for basic authentication", "Install": "Install", "Back": "Back", "Display Name": "Display Name", diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx index 31458acbe85..2016d9dd39b 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx @@ -159,7 +159,7 @@ const HelmURLChartForm: FC & HelmURLChartFormP { try { const fullChartURL = getFullChartURL(chartURL, chartVersion); - let authParam = ''; - if (basicAuthSecretName) { - authParam = `&basic_auth_secret_name=${encodeURIComponent(basicAuthSecretName)}`; - } + const authParam = basicAuthSecretName + ? `&basic_auth_secret_name=${encodeURIComponent(basicAuthSecretName)}` + : ''; const apiUrl = `/api/helm/chart?url=${encodeURIComponent( fullChartURL, )}&noRepo=true&namespace=${namespace}${authParam}`; diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index 5cfb71d8aa3..aa56d4f3961 100644 --- a/pkg/helm/actions/get_chart.go +++ b/pkg/helm/actions/get_chart.go @@ -72,10 +72,10 @@ func GetChartFromURL(url string, conf *action.Configuration, namespace string, c } cmd := action.NewInstall(conf) cmd.Namespace = namespace - if err := applyBasicAuthFromSecret(cmd, coreClient, namespace, basicAuthSecretName); err != nil { - return nil, err - } if basicAuthSecretName != "" { + if err := applyBasicAuthFromSecret(cmd, coreClient, namespace, basicAuthSecretName); err != nil { + return nil, err + } rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) if err != nil { return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) diff --git a/pkg/helm/actions/get_registry.go b/pkg/helm/actions/get_registry.go index 6683d85d192..0a194e9d286 100644 --- a/pkg/helm/actions/get_registry.go +++ b/pkg/helm/actions/get_registry.go @@ -12,7 +12,7 @@ import ( // newRegistryClient is a package-level variable to allow mocking in tests var newRegistryClient = registry.NewClient -// registryClientOptions returns the same options used by GetOCIRegistry for TLS / plain-HTTP behavior. +// registryClientOptions adds the appropriate registry functions to the client options based on the skipTLSVerify and plainHTTP flags. func registryClientOptions(skipTLSVerify, plainHTTP bool) []registry.ClientOption { opts := []registry.ClientOption{ registry.ClientOptDebug(false), diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index 1f6792c5996..7eaec329fdf 100644 --- a/pkg/helm/actions/install_chart.go +++ b/pkg/helm/actions/install_chart.go @@ -262,18 +262,15 @@ func InstallChartAsync(ns, name, url string, vals map[string]interface{}, conf * // applyBasicAuthFromSecret sets cmd.Username and cmd.Password from a Secret in ns with // keys "username" and "password" (same convention as HelmChartRepository connectionConfig). func applyBasicAuthFromSecret(cmd *action.Install, coreClient corev1client.CoreV1Interface, ns, secretName string) error { - if secretName == "" { - return nil - } secret, err := coreClient.Secrets(ns).Get(context.TODO(), secretName, v1.GetOptions{}) if err != nil { return fmt.Errorf("failed to get secret %q from namespace %q: %w", secretName, ns, err) } u, uok := secret.Data[username] - p, pok := secret.Data[password] if !uok { return fmt.Errorf("failed to find %q key in secret %q/%q", username, ns, secretName) } + p, pok := secret.Data[password] if !pok { return fmt.Errorf("failed to find %q key in secret %q/%q", password, ns, secretName) } @@ -295,12 +292,12 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf cmd.ReleaseName = name cmd.Namespace = ns - if err := applyBasicAuthFromSecret(cmd, coreClient, ns, basicAuthSecretName); err != nil { - return nil, err - } // OCI pulls use conf.RegistryClient when set; the getter does not merge ChartPathOptions username/password // onto that client (see helm ocigetter). Rebuild the client with basic auth when credentials are supplied. if basicAuthSecretName != "" { + if err := applyBasicAuthFromSecret(cmd, coreClient, ns, basicAuthSecretName); err != nil { + return nil, err + } rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) if err != nil { return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) diff --git a/pkg/helm/actions/install_chart_test.go b/pkg/helm/actions/install_chart_test.go index 4c5dfb57bce..8265e1460c7 100644 --- a/pkg/helm/actions/install_chart_test.go +++ b/pkg/helm/actions/install_chart_test.go @@ -416,37 +416,38 @@ func TestInstallChartFromURL(t *testing.T) { basicAuthSecretName string basicAuthUser string basicAuthPass string - expectError bool + secretData map[string][]byte + expectedErrMsg string }{ { - testName: "valid HTTP chart URL", - releaseName: "valid-chart-path", - chartPath: "http://localhost:9181/charts/influxdb-3.0.2.tgz", - chartName: "influxdb", - chartVersion: "3.0.2", - plainHTTP: true, - skipTLSVerify: true, - expectError: false, + testName: "valid HTTP chart URL", + releaseName: "valid-chart-path", + chartPath: "http://localhost:9181/charts/influxdb-3.0.2.tgz", + chartName: "influxdb", + chartVersion: "3.0.2", + plainHTTP: true, + skipTLSVerify: true, + expectedErrMsg: "", }, { - testName: "valid OCI chart URL", - releaseName: "valid-chart-path", - chartPath: "oci://localhost:5000/helm-charts/mychart:0.1.0", - chartName: "mychart", - chartVersion: "0.1.0", - plainHTTP: true, - skipTLSVerify: true, - expectError: false, + testName: "valid OCI chart URL", + releaseName: "valid-chart-path", + chartPath: "oci://localhost:5000/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + expectedErrMsg: "", }, { - testName: "invalid chart URL rejected synchronously", - releaseName: "invalid-chart-path", - chartPath: "http://localhost:9181/charts/influxdb/filename", - chartName: "influxdb", - chartVersion: "3.0.1", - plainHTTP: true, - skipTLSVerify: true, - expectError: true, + testName: "invalid chart URL rejected synchronously", + releaseName: "invalid-chart-path", + chartPath: "http://localhost:9181/charts/influxdb/filename", + chartName: "influxdb", + chartVersion: "3.0.1", + plainHTTP: true, + skipTLSVerify: true, + expectedErrMsg: "invalid chart URL", }, { testName: "OCI chart with basic auth", @@ -459,7 +460,20 @@ func TestInstallChartFromURL(t *testing.T) { basicAuthSecretName: "oci-auth-secret", basicAuthUser: "AzureDiamond", basicAuthPass: "hunter2", - expectError: false, + expectedErrMsg: "", + }, + { + testName: "HTTPS chart with basic auth", + releaseName: "basicauth-http", + chartPath: "http://localhost:8181/charts/mychart-0.1.0.tgz", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "http-auth-secret", + basicAuthUser: "AzureDiamond", + basicAuthPass: "hunter2", + expectedErrMsg: "", }, { testName: "OCI chart with wrong basic auth credentials", @@ -472,7 +486,55 @@ func TestInstallChartFromURL(t *testing.T) { basicAuthSecretName: "bad-auth-secret", basicAuthUser: "wrong-user", basicAuthPass: "wrong-pass", - expectError: true, + expectedErrMsg: "error locating chart", + }, + { + testName: "HTTPS chart with wrong basic auth credentials", + releaseName: "badauth-http", + chartPath: "http://localhost:8181/charts/mychart-0.1.0.tgz", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "bad-auth-secret", + basicAuthUser: "wrong-user", + basicAuthPass: "wrong-pass", + expectedErrMsg: "error locating chart", + }, + { + testName: "basic auth secret not found", + releaseName: "missing-secret", + chartPath: "oci://localhost:5001/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "nonexistent-secret", + expectedErrMsg: "failed to get secret", + }, + { + testName: "secret missing username key", + releaseName: "malformed-no-user", + chartPath: "oci://localhost:5001/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "bad-secret", + secretData: map[string][]byte{"password": []byte("hunter2")}, + expectedErrMsg: "failed to find \"username\" key in secret", + }, + { + testName: "secret missing password key", + releaseName: "malformed-no-pass", + chartPath: "oci://localhost:5001/helm-charts/mychart:0.1.0", + chartName: "mychart", + chartVersion: "0.1.0", + plainHTTP: true, + skipTLSVerify: true, + basicAuthSecretName: "bad-secret", + secretData: map[string][]byte{"username": []byte("AzureDiamond")}, + expectedErrMsg: "failed to find \"password\" key in secret", }, } for _, tt := range tests { @@ -489,7 +551,15 @@ func TestInstallChartFromURL(t *testing.T) { require.NoError(t, err) objs := []runtime.Object{} - if tt.basicAuthSecretName != "" { + if tt.secretData != nil { + objs = append(objs, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.basicAuthSecretName, + Namespace: "test-namespace", + }, + Data: tt.secretData, + }) + } else if tt.basicAuthSecretName != "" && tt.basicAuthUser != "" { objs = append(objs, &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: tt.basicAuthSecretName, @@ -504,9 +574,10 @@ func TestInstallChartFromURL(t *testing.T) { clientInterface := k8sfake.NewSimpleClientset(objs...) coreClient := clientInterface.CoreV1() - if tt.expectError { + if tt.expectedErrMsg != "" { rel, err := InstallChartFromURL("test-namespace", tt.releaseName, tt.chartPath, nil, actionConfig, coreClient, tt.chartVersion, tt.basicAuthSecretName) require.Error(t, err) + require.ErrorContains(t, err, tt.expectedErrMsg) require.Nil(t, rel) return } diff --git a/pkg/helm/actions/testdata/uploadOciCharts.sh b/pkg/helm/actions/testdata/uploadOciCharts.sh index 923c6165e8f..318c7050057 100755 --- a/pkg/helm/actions/testdata/uploadOciCharts.sh +++ b/pkg/helm/actions/testdata/uploadOciCharts.sh @@ -34,7 +34,7 @@ case $mode in "--basic-auth") REGISTRY="localhost:5001" echo "Logging in to oci://$REGISTRY with basic auth..." - echo "hunter2" | $HELM registry login $REGISTRY --username AzureDiamond --password-stdin --plain-http + $HELM registry login $REGISTRY --username AzureDiamond --password "hunter2" --plain-http echo "Pushing mychart-0.1.0.tgz to oci://$REGISTRY/helm-charts..." $HELM push $CHARTS_DIR/mychart-0.1.0.tgz oci://$REGISTRY/helm-charts --plain-http ;; @@ -44,6 +44,7 @@ case $mode in $HELM push $CHARTS_DIR/mychart-0.1.0.tgz oci://$REGISTRY/helm-charts --plain-http ;; *) + echo "Unrecognized argument \"${mode}\"." >&2 echo "Usage: uploadOciCharts.sh --tls | --no-tls | --basic-auth" >&2 exit 2 ;; diff --git a/pkg/helm/actions/testdata/zot-stop.sh b/pkg/helm/actions/testdata/zot-stop.sh index 77e4ed9df79..776fb4af971 100755 --- a/pkg/helm/actions/testdata/zot-stop.sh +++ b/pkg/helm/actions/testdata/zot-stop.sh @@ -1,6 +1,6 @@ #!/bin/bash -kill -TERM $(< zot.pid) || echo "Zot is not currently running." -kill -TERM $(< zot-no-tls.pid) || echo "Zot is not currently running." -kill -TERM "$(< zot-basicauth.pid)" || echo "Zot (basic auth) is not currently running." +kill -TERM $(< zot.pid) || echo "Zot (TLS) is not currently running." +kill -TERM $(< zot-no-tls.pid) || echo "Zot (no TLS) is not currently running." +kill -TERM $(< zot-basicauth.pid) || echo "Zot (basic auth) is not currently running." rm -f zot.pid zot-no-tls.pid zot-basicauth.pid From 6191e866d2f422e72a44984427a30432f52ae0ed Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Tue, 12 May 2026 22:28:47 +0530 Subject: [PATCH 5/7] HELM-683: Refactor registry client and extract useSecretResources hook Decouple GetOCIRegistry from action.Configuration so it returns the registry client directly, keeping GetActionConfigurations focused on REST/namespace plumbing. Consolidate secret-based auth into GetUserCredentials and applyBasicAuthFromUserCredentials for reuse across InstallChartFromURL and GetChartFromURL. Extract duplicated secret- watching logic from HelmURLChartForm and HelmURLInstallForm into a shared useSecretResources hook. Add test coverage for UserCredentials in GetOCIRegistry. --- .../forms/url-chart/HelmURLChartForm.tsx | 32 +-------- .../forms/url-chart/HelmURLInstallForm.tsx | 31 +-------- .../forms/url-chart/useSecretResources.ts | 33 ++++++++++ pkg/helm/actions/config.go | 3 +- pkg/helm/actions/get_chart.go | 9 ++- pkg/helm/actions/get_registry.go | 43 +++++------- pkg/helm/actions/get_registry_test.go | 66 +++++++++---------- pkg/helm/actions/install_chart.go | 42 +++++++----- pkg/helm/actions/install_chart_test.go | 9 ++- pkg/helm/handlers/handlers.go | 3 +- pkg/helm/handlers/request.go | 19 +++--- 11 files changed, 135 insertions(+), 155 deletions(-) create mode 100644 frontend/packages/helm-plugin/src/components/forms/url-chart/useSecretResources.ts diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx index 2016d9dd39b..37a17820da0 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx @@ -1,13 +1,10 @@ import type { FC } from 'react'; -import { useEffect, useMemo } from 'react'; +import { useEffect } from 'react'; import { TextInputTypes, Grid, GridItem } from '@patternfly/react-core'; import type { FormikProps } from 'formik'; import * as fuzzy from 'fuzzysearch'; import { useTranslation } from 'react-i18next'; import FormSection from '@console/dev-console/src/components/import/section/FormSection'; -import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; -import { SecretModel } from '@console/internal/models'; -import type { K8sResourceKind } from '@console/internal/module/k8s'; import { InputField, FormFooter, @@ -17,6 +14,7 @@ import { ResourceDropdownField, } from '@console/shared'; import type { HelmURLChartFormData } from './types'; +import { useSecretResources } from './useSecretResources'; export interface HelmURLChartFormProps { namespace: string; @@ -40,31 +38,7 @@ const HelmURLChartForm: FC & HelmURLChartFormP const autocompleteFilter = (strText: string, item: any): boolean => fuzzy(strText, item?.props?.name); - const watchedResources = useK8sWatchResources<{ - secrets: K8sResourceKind[]; - }>({ - secrets: { - isList: true, - kind: SecretModel.kind, - namespace, - optional: true, - }, - }); - const secretResources = useMemo( - () => [ - { - data: watchedResources.secrets?.data, - loaded: watchedResources.secrets?.loaded, - loadError: watchedResources.secrets?.loadError, - kind: SecretModel.kind, - }, - ], - [ - watchedResources.secrets?.data, - watchedResources.secrets?.loaded, - watchedResources.secrets?.loadError, - ], - ); + const secretResources = useSecretResources(namespace); const isNextDisabled = !isValid || !dirty || isSubmitting; // Auto-populate releaseName and chartVersion from URL diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx index d9427908202..ca643828d26 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx @@ -6,9 +6,6 @@ import * as fuzzy from 'fuzzysearch'; import * as _ from 'lodash'; import { Trans, useTranslation } from 'react-i18next'; import FormSection from '@console/dev-console/src/components/import/section/FormSection'; -import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; -import { SecretModel } from '@console/internal/models'; -import type { K8sResourceKind } from '@console/internal/module/k8s'; import { InputField, ResourceDropdownField, @@ -23,6 +20,7 @@ import { import { getJSONSchemaOrder, prune } from '@console/shared/src/components/dynamic-form/utils'; import { useHelmReadmeModalLauncher } from '../install-upgrade/HelmReadmeModal'; import type { HelmURLInstallFormData } from './types'; +import { useSecretResources } from './useSecretResources'; export interface HelmURLInstallFormProps { chartHasValues: boolean; @@ -49,32 +47,7 @@ const HelmURLInstallForm: FC & HelmURLInstal const autocompleteFilter = (strText: string, item: string): boolean => fuzzy(strText, item); - const watchedResources = useK8sWatchResources<{ - secrets: K8sResourceKind[]; - }>({ - secrets: { - isList: true, - kind: SecretModel.kind, - namespace, - optional: true, - }, - }); - - const secretResources = useMemo( - () => [ - { - data: watchedResources.secrets?.data, - loaded: watchedResources.secrets?.loaded, - loadError: watchedResources.secrets?.loadError, - kind: SecretModel.kind, - }, - ], - [ - watchedResources.secrets?.data, - watchedResources.secrets?.loaded, - watchedResources.secrets?.loadError, - ], - ); + const secretResources = useSecretResources(namespace); const helmReadmeModalLauncher = useHelmReadmeModalLauncher({ readme: chartReadme, diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/useSecretResources.ts b/frontend/packages/helm-plugin/src/components/forms/url-chart/useSecretResources.ts new file mode 100644 index 00000000000..8a751314b14 --- /dev/null +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/useSecretResources.ts @@ -0,0 +1,33 @@ +import { useMemo } from 'react'; +import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; +import { SecretModel } from '@console/internal/models'; +import type { K8sResourceKind } from '@console/internal/module/k8s'; + +export const useSecretResources = (namespace: string) => { + const watchedResources = useK8sWatchResources<{ + secrets: K8sResourceKind[]; + }>({ + secrets: { + isList: true, + kind: SecretModel.kind, + namespace, + optional: true, + }, + }); + + return useMemo( + () => [ + { + data: watchedResources.secrets?.data, + loaded: watchedResources.secrets?.loaded, + loadError: watchedResources.secrets?.loadError, + kind: SecretModel.kind, + }, + ], + [ + watchedResources.secrets?.data, + watchedResources.secrets?.loaded, + watchedResources.secrets?.loadError, + ], + ); +}; diff --git a/pkg/helm/actions/config.go b/pkg/helm/actions/config.go index be5d2c062ab..191dcd6c00b 100644 --- a/pkg/helm/actions/config.go +++ b/pkg/helm/actions/config.go @@ -50,9 +50,10 @@ func GetActionConfigurations(host, ns, token string, transport *http.RoundTrippe } conf := new(action.Configuration) conf.Init(confFlags, ns, "secrets", klog.Infof) - err = GetDefaultOCIRegistry(conf) + registryClient, err := GetDefaultOCIRegistry() if err != nil { klog.V(4).Infof("Failed to get default OCI registry: %v", err) } + conf.RegistryClient = registryClient return conf } diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index aa56d4f3961..bd76446902a 100644 --- a/pkg/helm/actions/get_chart.go +++ b/pkg/helm/actions/get_chart.go @@ -73,14 +73,13 @@ func GetChartFromURL(url string, conf *action.Configuration, namespace string, c cmd := action.NewInstall(conf) cmd.Namespace = namespace if basicAuthSecretName != "" { - if err := applyBasicAuthFromSecret(cmd, coreClient, namespace, basicAuthSecretName); err != nil { + userCredentials, err := GetUserCredentials(coreClient, namespace, basicAuthSecretName) + if err != nil { return nil, err } - rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) - if err != nil { - return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) + if err := applyBasicAuthFromUserCredentials(cmd, userCredentials); err != nil { + return nil, err } - cmd.SetRegistryClient(rc) } chartLocation, err := cmd.ChartPathOptions.LocateChart(url, settings) if err != nil { diff --git a/pkg/helm/actions/get_registry.go b/pkg/helm/actions/get_registry.go index 0a194e9d286..affed8a2712 100644 --- a/pkg/helm/actions/get_registry.go +++ b/pkg/helm/actions/get_registry.go @@ -5,15 +5,18 @@ import ( "fmt" "net/http" - "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/registry" ) // newRegistryClient is a package-level variable to allow mocking in tests var newRegistryClient = registry.NewClient -// registryClientOptions adds the appropriate registry functions to the client options based on the skipTLSVerify and plainHTTP flags. -func registryClientOptions(skipTLSVerify, plainHTTP bool) []registry.ClientOption { +type UserCredentials struct { + Username string + Password string +} + +func GetOCIRegistry(skipTLSVerify bool, plainHTTP bool, userCredentials *UserCredentials) (*registry.Client, error) { opts := []registry.ClientOption{ registry.ClientOptDebug(false), } @@ -27,31 +30,17 @@ func registryClientOptions(skipTLSVerify, plainHTTP bool) []registry.ClientOptio } opts = append(opts, registry.ClientOptHTTPClient(&http.Client{Transport: transport})) } - return opts -} - -// RegistryClientWithBasicAuth builds a registry.Client with the same TLS/plain-HTTP settings as -// GetDefaultOCIRegistry (skipTLSVerify=false, plainHTTP=false) plus OCI basic auth. -// Helm's OCI getter uses Configuration.RegistryClient when set and does not apply ChartPathOptions -// username/password to that client; credentials must be set on the registry client via ClientOptBasicAuth. -func RegistryClientWithBasicAuth(skipTLSVerify, plainHTTP bool, username, password string) (*registry.Client, error) { - opts := registryClientOptions(skipTLSVerify, plainHTTP) - opts = append(opts, registry.ClientOptBasicAuth(username, password)) - return newRegistryClient(opts...) -} - -func GetDefaultOCIRegistry(conf *action.Configuration) error { - return GetOCIRegistry(conf, false, false) -} - -func GetOCIRegistry(conf *action.Configuration, skipTLSVerify bool, plainHTTP bool) error { - if conf == nil { - return fmt.Errorf("action configuration cannot be nil") + if userCredentials != nil { + opts = append(opts, registry.ClientOptBasicAuth(userCredentials.Username, userCredentials.Password)) } - registryClient, err := newRegistryClient(registryClientOptions(skipTLSVerify, plainHTTP)...) + registryClient, err := newRegistryClient(opts...) if err != nil { - return fmt.Errorf("failed to create registry client: %w", err) + return nil, fmt.Errorf("failed to create registry client: %w", err) } - conf.RegistryClient = registryClient - return nil + return registryClient, nil + +} + +func GetDefaultOCIRegistry() (*registry.Client, error) { + return GetOCIRegistry(false, false, nil) } diff --git a/pkg/helm/actions/get_registry_test.go b/pkg/helm/actions/get_registry_test.go index 73ee7e83a37..c9a0d93e17f 100644 --- a/pkg/helm/actions/get_registry_test.go +++ b/pkg/helm/actions/get_registry_test.go @@ -29,9 +29,9 @@ func TestGetDefaultOCIRegistry_Success(t *testing.T) { originalKubeClient := conf.KubeClient originalCapabilities := conf.Capabilities - err := GetDefaultOCIRegistry(conf) + registryClient, err := GetDefaultOCIRegistry() require.NoError(t, err) - require.NotNil(t, conf.RegistryClient, "Registry Client should not be nil") + require.NotNil(t, registryClient, "Registry Client should not be nil") // Verify other configuration fields are not modified. require.Equal(t, originalReleases, conf.Releases, "Releases should not be modified") @@ -40,17 +40,12 @@ func TestGetDefaultOCIRegistry_Success(t *testing.T) { } -func TestGetOCIRegistry_NilConfig(t *testing.T) { - err := GetOCIRegistry(nil, false, false) - require.Error(t, err) - require.Contains(t, err.Error(), "action configuration cannot be nil") -} - func TestGetOCIRegistry_Success(t *testing.T) { tests := []struct { - name string - skipTLSVerify bool - plainHTTP bool + name string + skipTLSVerify bool + plainHTTP bool + userCredentials *UserCredentials }{ { name: "default options", @@ -72,6 +67,21 @@ func TestGetOCIRegistry_Success(t *testing.T) { skipTLSVerify: true, plainHTTP: true, }, + { + name: "with user credentials", + userCredentials: &UserCredentials{Username: "admin", Password: "secret"}, + }, + { + name: "with user credentials and plainHTTP", + plainHTTP: true, + userCredentials: &UserCredentials{Username: "admin", Password: "secret"}, + }, + { + name: "with user credentials, skipTLSVerify, and plainHTTP", + skipTLSVerify: true, + plainHTTP: true, + userCredentials: &UserCredentials{Username: "admin", Password: "secret"}, + }, } originalNewRegistryClient := newRegistryClient defer func() { @@ -80,29 +90,23 @@ func TestGetOCIRegistry_Success(t *testing.T) { for _, tt := range tests { newRegistryClient = func(options ...registry.ClientOption) (*registry.Client, error) { - count := 0 + expectedExtra := 0 if tt.plainHTTP { - count += 1 + expectedExtra++ } if tt.skipTLSVerify { - count += 1 + expectedExtra++ + } + if tt.userCredentials != nil { + expectedExtra++ } - require.Equal(t, count, len(options)-1, "Expected %d options, got %d", count, len(options)) + require.Equal(t, expectedExtra, len(options)-1, "Expected %d extra options, got %d", expectedExtra, len(options)-1) return ®istry.Client{}, nil } t.Run(tt.name, func(t *testing.T) { - store := storage.Init(driver.NewMemory()) - conf := &action.Configuration{ - RESTClientGetter: FakeConfig{}, - Releases: store, - KubeClient: &kubefake.PrintingKubeClient{Out: io.Discard}, - Capabilities: chartutil.DefaultCapabilities, - } - require.Nil(t, conf.RegistryClient, "Registry Client should be nil initially") - - err := GetOCIRegistry(conf, tt.skipTLSVerify, tt.plainHTTP) + registryClient, err := GetOCIRegistry(tt.skipTLSVerify, tt.plainHTTP, tt.userCredentials) require.NoError(t, err) - require.NotNil(t, conf.RegistryClient, "Registry Client should not be nil after GetOCIRegistry") + require.NotNil(t, registryClient, "Registry Client should not be nil after GetOCIRegistry") }) } } @@ -117,15 +121,7 @@ func TestGetOCIRegistry_NewClientError(t *testing.T) { return nil, errors.New("mock registry client error") } - store := storage.Init(driver.NewMemory()) - conf := &action.Configuration{ - RESTClientGetter: FakeConfig{}, - Releases: store, - KubeClient: &kubefake.PrintingKubeClient{Out: io.Discard}, - Capabilities: chartutil.DefaultCapabilities, - } - - err := GetOCIRegistry(conf, false, false) + _, err := GetOCIRegistry(false, false, nil) require.Error(t, err) require.Contains(t, err.Error(), "failed to create registry client") require.Contains(t, err.Error(), "mock registry client error") diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index 7eaec329fdf..f206f440413 100644 --- a/pkg/helm/actions/install_chart.go +++ b/pkg/helm/actions/install_chart.go @@ -259,30 +259,43 @@ func InstallChartAsync(ns, name, url string, vals map[string]interface{}, conf * return &secret, nil } -// applyBasicAuthFromSecret sets cmd.Username and cmd.Password from a Secret in ns with -// keys "username" and "password" (same convention as HelmChartRepository connectionConfig). -func applyBasicAuthFromSecret(cmd *action.Install, coreClient corev1client.CoreV1Interface, ns, secretName string) error { +// GetUserCredentials gets the username and password from a Secret in namespace with keys "username" and "password" +func GetUserCredentials(coreClient corev1client.CoreV1Interface, ns, secretName string) (*UserCredentials, error) { secret, err := coreClient.Secrets(ns).Get(context.TODO(), secretName, v1.GetOptions{}) if err != nil { - return fmt.Errorf("failed to get secret %q from namespace %q: %w", secretName, ns, err) + return nil, fmt.Errorf("failed to get secret %q from namespace %q: %w", secretName, ns, err) } u, uok := secret.Data[username] if !uok { - return fmt.Errorf("failed to find %q key in secret %q/%q", username, ns, secretName) + return nil, fmt.Errorf("failed to find %q key in secret %q/%q", username, ns, secretName) } p, pok := secret.Data[password] if !pok { - return fmt.Errorf("failed to find %q key in secret %q/%q", password, ns, secretName) + return nil, fmt.Errorf("failed to find %q key in secret %q/%q", password, ns, secretName) } - cmd.Username = string(u) - cmd.Password = string(p) + + return &UserCredentials{ + Username: string(u), + Password: string(p), + }, nil +} + +// applyBasicAuthFromSecret sets cmd.Username and cmd.Password from a userCredentials and sets the registry client +func applyBasicAuthFromUserCredentials(cmd *action.Install, userCredentials *UserCredentials) error { + cmd.Username = userCredentials.Username + cmd.Password = userCredentials.Password + rc, err := GetOCIRegistry(false, false, userCredentials) + if err != nil { + return fmt.Errorf("failed to configure OCI registry client: %w", err) + } + cmd.SetRegistryClient(rc) return nil } // InstallChartFromURL installs a chart from an OCI or direct HTTP(S) chart URL. // If not provided, version is extracted from the OCI URL tag when applicable. // basicAuthSecretName names a Secret in ns containing username and password keys for registry auth. -func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version string, basicAuthSecretName string) (*kv1.Secret, error) { +func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version, basicAuthSecretName string) (*kv1.Secret, error) { if !isValidChartURL(url) { return nil, fmt.Errorf("invalid chart URL: %s, must be oci:// URL or http(s)://*.tgz", url) @@ -293,16 +306,15 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf cmd.Namespace = ns // OCI pulls use conf.RegistryClient when set; the getter does not merge ChartPathOptions username/password - // onto that client (see helm ocigetter). Rebuild the client with basic auth when credentials are supplied. + // onto that client. Rebuild the client with basic auth when credentials are supplied. if basicAuthSecretName != "" { - if err := applyBasicAuthFromSecret(cmd, coreClient, ns, basicAuthSecretName); err != nil { + userCredentials, err := GetUserCredentials(coreClient, ns, basicAuthSecretName) + if err != nil { return nil, err } - rc, err := RegistryClientWithBasicAuth(false, false, cmd.Username, cmd.Password) - if err != nil { - return nil, fmt.Errorf("failed to configure OCI registry client: %w", err) + if err := applyBasicAuthFromUserCredentials(cmd, userCredentials); err != nil { + return nil, err } - cmd.SetRegistryClient(rc) } // Set version so LocateChart (and Helm OCI) resolve the correct chart tag; matches InstallChart behavior. diff --git a/pkg/helm/actions/install_chart_test.go b/pkg/helm/actions/install_chart_test.go index 8265e1460c7..fcceb46f5a7 100644 --- a/pkg/helm/actions/install_chart_test.go +++ b/pkg/helm/actions/install_chart_test.go @@ -463,7 +463,7 @@ func TestInstallChartFromURL(t *testing.T) { expectedErrMsg: "", }, { - testName: "HTTPS chart with basic auth", + testName: "HTTP chart with basic auth", releaseName: "basicauth-http", chartPath: "http://localhost:8181/charts/mychart-0.1.0.tgz", chartName: "mychart", @@ -489,7 +489,7 @@ func TestInstallChartFromURL(t *testing.T) { expectedErrMsg: "error locating chart", }, { - testName: "HTTPS chart with wrong basic auth credentials", + testName: "HTTP chart with wrong basic auth credentials", releaseName: "badauth-http", chartPath: "http://localhost:8181/charts/mychart-0.1.0.tgz", chartName: "mychart", @@ -547,8 +547,9 @@ func TestInstallChartFromURL(t *testing.T) { Capabilities: chartutil.DefaultCapabilities, Log: func(format string, v ...interface{}) {}, } - err := GetOCIRegistry(actionConfig, tt.skipTLSVerify, tt.plainHTTP) + registryClient, err := GetOCIRegistry(tt.skipTLSVerify, tt.plainHTTP, nil) require.NoError(t, err) + actionConfig.RegistryClient = registryClient objs := []runtime.Object{} if tt.secretData != nil { @@ -582,6 +583,8 @@ func TestInstallChartFromURL(t *testing.T) { return } + // For valid URLs: create the release secret in a background goroutine + // to simulate what Helm's cmd.Run would do, unblocking getSecret's Watch. secretName := fmt.Sprintf("sh.helm.release.v1.%v.v1", tt.releaseName) go func() { time.Sleep(2 * time.Second) diff --git a/pkg/helm/handlers/handlers.go b/pkg/helm/handlers/handlers.go index 6d37df337ec..1fe109b735e 100644 --- a/pkg/helm/handlers/handlers.go +++ b/pkg/helm/handlers/handlers.go @@ -456,6 +456,7 @@ func (h *helmHandlers) HandleURLChartGet(user *auth.User, w http.ResponseWriter, namespace = "default" } chartUrl := params.Get("url") + basicAuthSecretName := params.Get("basic_auth_secret_name") if chartUrl == "" { serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: "chart URL is required"}) @@ -468,7 +469,7 @@ func (h *helmHandlers) HandleURLChartGet(user *auth.User, w http.ResponseWriter, serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: err.Error()}) return } - resp, err := h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true, params.Get("basic_auth_secret_name")) + resp, err := h.getChartFromURL(chartUrl, conf, namespace, handlerClients.DynamicClient, handlerClients.CoreClient, true, basicAuthSecretName) if err != nil { serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to retrieve chart: %v", err)}) return diff --git a/pkg/helm/handlers/request.go b/pkg/helm/handlers/request.go index 5c9385cf6b5..65969ef6271 100644 --- a/pkg/helm/handlers/request.go +++ b/pkg/helm/handlers/request.go @@ -1,16 +1,15 @@ package handlers type HelmRequest struct { - Name string `json:"name"` - Namespace string `json:"namespace"` - ChartUrl string `json:"chart_url"` - ChartVersion string `json:"chart_version"` // optional; for OCI/direct URL install, used when chart_url has no tag - Values map[string]interface{} `json:"values"` - Version int `json:"version"` - IndexEntry string `json:"indexEntry"` - NoRepo bool `json:"noRepo"` - // BasicAuthSecretName is optional; names a Secret in Namespace with keys username and password for OCI/HTTP chart pull when NoRepo is true. - BasicAuthSecretName string `json:"basic_auth_secret_name"` + Name string `json:"name"` + Namespace string `json:"namespace"` + ChartUrl string `json:"chart_url"` + ChartVersion string `json:"chart_version"` // optional; for OCI/direct URL install, used when chart_url has no tag + Values map[string]interface{} `json:"values"` + Version int `json:"version"` + IndexEntry string `json:"indexEntry"` + NoRepo bool `json:"noRepo"` + BasicAuthSecretName string `json:"basic_auth_secret_name"` // optional; names a Secret in Namespace with keys username and password for OCI/HTTP chart pull when NoRepo is true. } type HelmVerifierRequest struct { From 35e7988addd71e7c8ea0d2a52ab964f9becfc0d2 Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Thu, 14 May 2026 18:35:10 +0530 Subject: [PATCH 6/7] HELM-703: Show "None" as default in disabled secret dropdown on install form The second page (install form) shows the secret field as disabled. When no secret is selected, display "None" instead of "Select a secret" to clearly indicate that authentication is not configured. --- frontend/packages/helm-plugin/locales/en/helm-plugin.json | 1 + .../src/components/forms/url-chart/HelmURLInstallForm.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/packages/helm-plugin/locales/en/helm-plugin.json b/frontend/packages/helm-plugin/locales/en/helm-plugin.json index 17e3969b140..da448572235 100644 --- a/frontend/packages/helm-plugin/locales/en/helm-plugin.json +++ b/frontend/packages/helm-plugin/locales/en/helm-plugin.json @@ -135,6 +135,7 @@ "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.": "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.", "Configure Helm release": "Configure Helm release", "Version": "Version", + "None": "None", "Install": "Install", "Back": "Back", "Display Name": "Display Name", diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx index ca643828d26..a34dea7fc98 100644 --- a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx @@ -159,7 +159,7 @@ const HelmURLInstallForm: FC & HelmURLInstal resources={secretResources} dataSelector={['metadata', 'name']} fullWidth - placeholder={t('helm-plugin~Select a secret')} + placeholder={t('helm-plugin~None')} showBadge autocompleteFilter={autocompleteFilter} disabled From 92da9e3fade5ebed07cd3279e9b1e07b438a1eee Mon Sep 17 00:00:00 2001 From: sowmya-sl Date: Tue, 12 May 2026 13:47:09 +0530 Subject: [PATCH 7/7] HELM-728: Persist auth secret across installs and upgrades Use a RegistryClientSetter interface so applyBasicAuthFromUserCredentials works with both action.Install and action.Upgrade via their shared ChartPathOptions. Persist the basic-auth secret name as a chart annotation (helm.openshift.io/auth-secret) during install and propagate it on upgrade so authenticated registries remain accessible across the release lifecycle. --- pkg/helm/actions/get_chart.go | 2 +- pkg/helm/actions/install_chart.go | 24 ++++++++++++-- pkg/helm/actions/upgrade_release.go | 50 +++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index bd76446902a..7ae03cd7422 100644 --- a/pkg/helm/actions/get_chart.go +++ b/pkg/helm/actions/get_chart.go @@ -77,7 +77,7 @@ func GetChartFromURL(url string, conf *action.Configuration, namespace string, c if err != nil { return nil, err } - if err := applyBasicAuthFromUserCredentials(cmd, userCredentials); err != nil { + if err := applyBasicAuthFromUserCredentials(&cmd.ChartPathOptions, cmd, userCredentials); err != nil { return nil, err } } diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index f206f440413..ed8472fa5f1 100644 --- a/pkg/helm/actions/install_chart.go +++ b/pkg/helm/actions/install_chart.go @@ -14,6 +14,7 @@ import ( "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/registry" "helm.sh/helm/v3/pkg/release" kv1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -48,6 +49,14 @@ var ( httpURLRe = regexp.MustCompile(`(?i)^https?://` + hostPort + `/.+\.(?:tar\.gz|tgz)$`) ) +const ( + helmAuthSecretAnnotation = "helm.openshift.io/auth-secret" +) + +type RegistryClientSetter interface { + SetRegistryClient(rc *registry.Client) +} + // isValidChartURL validates chart URLs using RFC-compliant hostname labels. // Accepts oci:/// and http(s):///.tgz|tar.gz URLs. func isValidChartURL(raw string) bool { @@ -281,17 +290,25 @@ func GetUserCredentials(coreClient corev1client.CoreV1Interface, ns, secretName } // applyBasicAuthFromSecret sets cmd.Username and cmd.Password from a userCredentials and sets the registry client -func applyBasicAuthFromUserCredentials(cmd *action.Install, userCredentials *UserCredentials) error { +func applyBasicAuthFromUserCredentials(cmd *action.ChartPathOptions, setter RegistryClientSetter, userCredentials *UserCredentials) error { cmd.Username = userCredentials.Username cmd.Password = userCredentials.Password rc, err := GetOCIRegistry(false, false, userCredentials) if err != nil { return fmt.Errorf("failed to configure OCI registry client: %w", err) } - cmd.SetRegistryClient(rc) + setter.SetRegistryClient(rc) return nil } +// addAuthSecretAnnotation adds the auth secret reference to the release annotations via chart metadata. +func addAuthSecretAnnotation(ch *chart.Chart, secretName string) { + if secretName == "" { + return + } + ch.Metadata.Annotations[helmAuthSecretAnnotation] = secretName +} + // InstallChartFromURL installs a chart from an OCI or direct HTTP(S) chart URL. // If not provided, version is extracted from the OCI URL tag when applicable. // basicAuthSecretName names a Secret in ns containing username and password keys for registry auth. @@ -312,7 +329,7 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf if err != nil { return nil, err } - if err := applyBasicAuthFromUserCredentials(cmd, userCredentials); err != nil { + if err := applyBasicAuthFromUserCredentials(&cmd.ChartPathOptions, cmd, userCredentials); err != nil { return nil, err } } @@ -345,6 +362,7 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf } ch.Metadata.Annotations["chart_url"] = url ch.Metadata.Annotations["installation"] = "url_install" + addAuthSecretAnnotation(ch, basicAuthSecretName) go func() { _, err := cmd.Run(ch, vals) if err == nil { diff --git a/pkg/helm/actions/upgrade_release.go b/pkg/helm/actions/upgrade_release.go index 1b4cb9a99de..2b21618f739 100644 --- a/pkg/helm/actions/upgrade_release.go +++ b/pkg/helm/actions/upgrade_release.go @@ -17,6 +17,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/klog/v2" ) func UpgradeRelease( @@ -132,6 +133,36 @@ func UpgradeRelease( return rel, nil } +// applyBasicAuthFromSecretForUpgrade sets client.Username and client.Password from a Secret. +func applyBasicAuthFromSecretForUpgrade(client *action.Upgrade, coreClient corev1client.CoreV1Interface, ns, secretName string) error { + if secretName == "" { + return nil + } + secret, err := coreClient.Secrets(ns).Get(context.TODO(), secretName, v1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get secret %q from namespace %q: %w", secretName, ns, err) + } + u, uok := secret.Data["username"] + p, pok := secret.Data["password"] + if !uok { + klog.Warningf("Secret %q/%q missing %q key for upgrade auth", ns, secretName, "username") + } + if !pok { + klog.Warningf("Secret %q/%q missing %q key for upgrade auth", ns, secretName, "password") + } + client.Username = string(u) + client.Password = string(p) + return nil +} + +// retrieveAuthSecretFromRelease extracts the auth secret name from release annotations. +func retrieveAuthSecretFromRelease(rel *release.Release) string { + if rel == nil || rel.Chart == nil || rel.Chart.Metadata == nil || rel.Chart.Metadata.Annotations == nil { + return "" + } + return rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation] +} + func UpgradeReleaseAsync( releaseNamespace string, releaseName string, @@ -158,11 +189,27 @@ func UpgradeReleaseAsync( return nil, err } + auth_secret := "" // Before proceeding, check if chart URL is present as an annotation if rel.Chart.Metadata.Annotations != nil { if chart_url, ok := rel.Chart.Metadata.Annotations["chart_url"]; chartUrl == "" && ok { chartUrl = chart_url } + if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok { + auth_secret = authSecret + } + + } + + if auth_secret != "" { + klog.Infof("Found persisted auth secret %s for release %s/%s, applying credentials for upgrade", auth_secret, releaseNamespace, releaseName) + userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret) + if err != nil { + klog.Errorf("Failed to get user credentials for release upgrade %s/%s: %v", releaseNamespace, releaseName, err) + } + if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil { + return nil, err + } } var tlsFiles []*os.File @@ -224,6 +271,9 @@ func UpgradeReleaseAsync( } ch.Metadata.Annotations["chart_url"] = chartUrl } + if auth_secret != "" { + addAuthSecretAnnotation(ch, auth_secret) + } go func() { _, err := client.Run(releaseName, ch, vals) if err != nil {