diff --git a/frontend/packages/helm-plugin/locales/en/helm-plugin.json b/frontend/packages/helm-plugin/locales/en/helm-plugin.json index 7c52d917308..7b35ec9c304 100644 --- a/frontend/packages/helm-plugin/locales/en/helm-plugin.json +++ b/frontend/packages/helm-plugin/locales/en/helm-plugin.json @@ -6,7 +6,6 @@ "**Helm Charts** are packages for deploying an Application or components of a larger Application.": "**Helm Charts** are packages for deploying an Application or components of a larger Application.", "A description for the Helm Chart repository.": "A description for the Helm Chart repository.", "A display name for the Helm Chart repository.": "A display name for the Helm Chart repository.", - "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication": "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication", "A unique name for the Helm Chart repository.": "A unique name for the Helm Chart repository.", "A unique name for the Helm Release.": "A unique name for the Helm Release.", "Add a Helm Chart Repository to extend the Developer Catalog": "Add a Helm Chart Repository to extend the Developer Catalog", @@ -39,9 +38,11 @@ "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", "Create": "Create", + "Create authentication Secret": "Create authentication Secret", "Create Helm Chart Repository": "Create Helm Chart Repository", "Create Helm Release": "Create Helm Release", "Create ProjectHelmChartRepository": "Create ProjectHelmChartRepository", + "Create Secret": "Create Secret", "Created": "Created", "Delete": "Delete", "Delete Helm Release": "Delete Helm Release", @@ -55,6 +56,7 @@ "Edit Helm Chart Repository": "Edit Helm Chart Repository", "Edit ProjectHelmChartRepository": "Edit ProjectHelmChartRepository", "Errors in the form data.": "Errors in the form data.", + "Failed to create Secret.": "Failed to create Secret.", "False": "False", "Filter by status": "Filter by status", "For more information on the chart, refer to this <2>README": "For more information on the chart, refer to this <2>README", @@ -100,6 +102,7 @@ "None": "None", "of": "of", "Partner": "Partner", + "Password or token for OCI/HTTP(S) authentication.": "Password or token for OCI/HTTP(S) authentication.", "per page": "per page", "Please enter a URL that is less then 2048 characters.": "Please enter a URL that is less then 2048 characters.", "Pods": "Pods", @@ -126,7 +129,12 @@ "Rollback to Revision {{revision}}": "Rollback to Revision {{revision}}", "Save": "Save", "Scope type": "Scope type", + "Secret \"{{secretName}}\" was not found in this namespace. Select an existing secret or create a new one.": "Secret \"{{secretName}}\" was not found in this namespace. Select an existing secret or create a new one.", "Secret for Basic authentication": "Secret for Basic authentication", + "Secret name": "Secret name", + "Secret password or token": "Secret password or token", + "Secret username": "Secret username", + "Secret with \"{{username}}\" and \"{{password}}\" keys for OCI/HTTP(S) authentication.": "Secret with \"{{username}}\" and \"{{password}}\" keys for OCI/HTTP(S) authentication.", "Select": "Select", "Select a Project to view its details<1>.": "Select a Project to view its details<1>.", "Select a secret": "Select a secret", @@ -146,6 +154,7 @@ "The repository name cannot exceed than 2048 characters.": "The repository name cannot exceed than 2048 characters.", "The version of chart to install.": "The version of chart to install.", "This Helm Chart is provided by a trusted partner and has been verified for ease of integration.": "This Helm Chart is provided by a trusted partner and has been verified for ease of integration.", + "This name is reserved. Choose a different name.": "This name is reserved. Choose a different name.", "TLS Client config": "TLS Client config", "To install a Helm chart, enter the chart URL - Open Container Initiative (OCI) URL or HTTP/HTTPS tar file and version.": "To install a Helm chart, enter the chart URL - Open Container Initiative (OCI) URL or HTTP/HTTPS tar file and version.", "True": "True", @@ -154,6 +163,7 @@ "Unable to load Helm Release history": "Unable to load Helm Release history", "Unable to load Helm Releases": "Unable to load Helm Releases", "Unique name for Helm release.": "Unique name for Helm release.", + "Unique name of the Secret": "Unique name of the Secret", "Unknown error submitting": "Unknown error submitting", "Update helm chart repository in the namespace.": "Update helm chart repository in the namespace.", "Update the helm chart repository.": "Update the helm chart repository.", @@ -163,6 +173,7 @@ "Upgrade by selecting a new chart version or manually changing YAML.": "Upgrade by selecting a new chart version or manually changing YAML.", "Upgrade Helm Release": "Upgrade Helm Release", "URL": "URL", + "Username for OCI/HTTP(S) authentication.": "Username for OCI/HTTP(S) authentication.", "Values from your current release are merged with the new chart's defaults. Review the YAML or form before upgrading.": "Values from your current release are merged with the new chart's defaults. Review the YAML or form before upgrading.", "Version": "Version" } diff --git a/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx b/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx index 50d40c31ee4..58056d0af78 100644 --- a/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx @@ -2,6 +2,7 @@ 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 type { JSONSchema7 } from 'json-schema'; import * as _ from 'lodash'; import { Trans, useTranslation } from 'react-i18next'; @@ -14,11 +15,18 @@ import { FormHeader } from '@console/shared/src/components/form-utils/FormHeader import { CodeEditorField } from '@console/shared/src/components/formik-fields/CodeEditorField'; import { DynamicFormField } from '@console/shared/src/components/formik-fields/DynamicFormField'; import { InputField } from '@console/shared/src/components/formik-fields/InputField'; +import { ResourceDropdownField } from '@console/shared/src/components/formik-fields/ResourceDropdownField'; import { SyncedEditorField } from '@console/shared/src/components/formik-fields/SyncedEditorField'; import type { EditorType } from '@console/shared/src/components/synced-editor/editor-toggle'; import type { HelmChart, HelmActionConfigType } from '../../../types/helm-types'; import { HelmActionType } from '../../../types/helm-types'; import { helmActionString } from '../../../utils/helm-utils'; +import { + useBasicAuthSecretDropdown, + CREATE_SECRET_KEY, + NONE_SECRET_KEY, +} from '../url-chart/useBasicAuthSecretDropdown'; +import { useSecretResources } from '../url-chart/useSecretResources'; import HelmChartVersionDropdown from './HelmChartVersionDropdown'; import { useHelmReadmeModalLauncher } from './HelmReadmeModal'; @@ -35,6 +43,8 @@ export type HelmInstallUpgradeFormData = { formData: any; formSchema: JSONSchema7; editorType: EditorType; + basicAuthSecretName?: string; + isURLInstall?: boolean; }; interface HelmInstallUpgradeFormProps { @@ -68,11 +78,33 @@ const HelmInstallUpgradeForm: FC< chartIndexEntry, annotatedName, providerName, + setFieldValue, }) => { const { t } = useTranslation('helm-plugin'); + const { handleSecretChange } = useBasicAuthSecretDropdown({ + namespace, + currentSecretName: values.basicAuthSecretName, + setFieldValue, + }); const { chartName, chartVersion, chartReadme, formData, formSchema, editorType } = values; const { type: helmAction, title, subTitle } = helmActionConfig; const helmReadmeModalLauncher = useHelmReadmeModalLauncher({ readme: chartReadme }); + const showAuthSecret = values.isURLInstall; + const secretResources = useSecretResources(namespace); + const autocompleteFilter = (strText: string, item: any, key?: string): boolean => + fuzzy(strText, item?.props?.name || (typeof item === 'string' ? item : key) || ''); + const secretMissing = useMemo(() => { + if ( + !showAuthSecret || + !values.basicAuthSecretName || + values.basicAuthSecretName === NONE_SECRET_KEY || + !secretResources[0]?.loaded + ) { + return false; + } + const secrets = secretResources[0]?.data ?? []; + return !secrets.some((s) => s?.metadata?.name === values.basicAuthSecretName); + }, [showAuthSecret, secretResources, values.basicAuthSecretName]); const isSubmitDisabled = (helmAction === HelmActionType.Upgrade && !dirty) || isSubmitting || @@ -162,6 +194,48 @@ const HelmInstallUpgradeForm: FC< providerName={providerName} /> + {showAuthSecret && ( + + + {secretMissing && ( + + )} + + )} {!chartError && diff --git a/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradePage.tsx b/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradePage.tsx index 378e17679a0..59389595c71 100644 --- a/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradePage.tsx +++ b/frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradePage.tsx @@ -33,6 +33,7 @@ import { isGoingToTopology, } from '../../../utils/helm-utils'; import { getHelmActionValidationSchema } from '../../../utils/helm-validation-utils'; +import { NONE_SECRET_KEY } from '../url-chart/useBasicAuthSecretDropdown'; import HelmChartMetaDescription from './HelmChartMetaDescription'; import type { HelmInstallUpgradeFormData } from './HelmInstallUpgradeForm'; import HelmInstallUpgradeForm from './HelmInstallUpgradeForm'; @@ -65,6 +66,7 @@ const HelmInstallUpgradePage: FC = () => { const [initialYamlData, setInitialYamlData] = useState(''); const [initialFormData, setInitialFormData] = useState(); const [initialFormSchema, setInitialFormSchema] = useState(); + const [initialBasicAuthSecretName, setInitialBasicAuthSecretName] = useState(''); const helmAction: HelmActionType = initialChartURL ? HelmActionType.Create : HelmActionType.Upgrade; @@ -101,6 +103,9 @@ const HelmInstallUpgradePage: FC = () => { const valuesYAML = releaseValues || chartValues; const valuesJSON = (res?.config || chart?.values) ?? {}; const valuesSchema = chart?.schema && JSON.parse(atob(chart?.schema)); + const basicAuthSecretName = + chart?.metadata?.annotations?.['helm.openshift.io/auth-secret'] ?? ''; + setInitialBasicAuthSecretName(basicAuthSecretName); setInitialYamlData(valuesYAML); setInitialFormData(valuesJSON); setInitialFormSchema(valuesSchema); @@ -120,6 +125,8 @@ const HelmInstallUpgradePage: FC = () => { }; }, [config.helmReleaseApi, helmAction]); + const isURLInstall = chartData?.metadata?.annotations?.installation === 'url_install'; + const initialValues: HelmInstallUpgradeFormData = { releaseName: initialReleaseName || helmChartName || '', chartURL: initialChartURL, @@ -133,6 +140,8 @@ const HelmInstallUpgradePage: FC = () => { formData: initialFormData, formSchema: initialFormSchema, editorType: initialFormSchema ? EditorType.Form : EditorType.YAML, + basicAuthSecretName: initialBasicAuthSecretName, + isURLInstall, }; const handleSubmit = (values, actions) => { @@ -143,6 +152,7 @@ const HelmInstallUpgradePage: FC = () => { yamlData, formData, editorType, + basicAuthSecretName, }: HelmInstallUpgradeFormData = values; let valuesObj; @@ -182,6 +192,14 @@ const HelmInstallUpgradePage: FC = () => { ...(chartURL ? { chart_url: chartURL } : {}), // eslint-disable-line @typescript-eslint/naming-convention ...(indexEntry ? { indexEntry } : { indexEntry: chartIndexEntry }), ...(valuesObj ? { values: valuesObj } : {}), + ...(values.isURLInstall + ? { + basic_auth_secret_name: + helmAction === HelmActionType.Create && basicAuthSecretName === NONE_SECRET_KEY + ? '' + : basicAuthSecretName, + } + : {}), // eslint-disable-line @typescript-eslint/naming-convention }; return config diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx new file mode 100644 index 00000000000..59acadd6151 --- /dev/null +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx @@ -0,0 +1,200 @@ +import { useCallback, useState } from 'react'; +import type { FormEvent } from 'react'; +import { + Button, + ButtonVariant, + Form, + FormGroup, + FormHelperText, + HelperText, + HelperTextItem, + Modal, + ModalBody, + ModalHeader, + ModalVariant, + TextInput, +} from '@patternfly/react-core'; +import { useTranslation } from 'react-i18next'; +import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider'; +import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay'; +import { SecretModel } from '@console/internal/models'; +import { k8sCreate } from '@console/internal/module/k8s'; +import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts'; + +interface HelmCreateBasicAuthSecretModalProps { + namespace: string; + save?: (name: string) => void; + onClose?: () => void; +} + +export const HelmCreateBasicAuthSecretModal: OverlayComponent = ({ + closeOverlay, + namespace, + save, + onClose, +}) => { + const { t } = useTranslation('helm-plugin'); + const [secretName, setSecretName] = useState(''); + const [username, setUsername] = useState(''); + const [password, setPassword] = useState(''); + const [inProgress, setInProgress] = useState(false); + const [errorMessage, setErrorMessage] = useState(); + + const isCreateDisabled = !secretName.trim() || !username.trim() || !password; + + const closeModal = (force = false) => { + if (inProgress && !force) { + return; + } + if (document.activeElement instanceof HTMLElement) { + document.activeElement.blur(); + } + onClose?.(); + closeOverlay(); + }; + + const handleCreate = async () => { + if (isCreateDisabled || inProgress) { + return; + } + + setInProgress(true); + setErrorMessage(undefined); + + const trimmedSecretName = secretName.trim(); + + // "__none__" is a sentinel used by the upgrade path to clear auth; reject it as a secret name. + if (trimmedSecretName === '__none__') { + setErrorMessage(t('This name is reserved. Choose a different name.')); + setInProgress(false); + return; + } + + try { + await k8sCreate(SecretModel, { + apiVersion: 'v1', + kind: 'Secret', + metadata: { + name: trimmedSecretName, + namespace, + }, + type: 'kubernetes.io/basic-auth', + stringData: { + ...(username ? { username } : {}), + password, + }, + }); + closeModal(true); + // Keep form update separate so a parent callback failure cannot block modal close. + save?.(trimmedSecretName); + } catch (err) { + const message = err instanceof Error ? err.message : t('Failed to create Secret.'); + setErrorMessage(message); + } finally { + setInProgress(false); + } + }; + + const onSubmit = async (e: FormEvent) => { + e.preventDefault(); + await handleCreate(); + }; + + return ( + closeModal(true)} + variant={ModalVariant.medium} + aria-labelledby="helm-create-auth-secret-modal-title" + > + + +
+ + setSecretName(value)} + isRequired + /> + + + {t('Unique name of the Secret')} + + + + + + setUsername(value)} + isRequired + /> + + + {t('Username for OCI/HTTP(S) authentication.')} + + + + + + setPassword(value)} + isRequired + /> + + + + {t('Password or token for OCI/HTTP(S) authentication.')} + + + + +
+ + + + + + + ); +}; + +export type HelmCreateBasicAuthSecretModalLauncher = ( + props?: HelmCreateBasicAuthSecretModalProps, +) => void; + +export const useHelmCreateBasicAuthSecretModal = (): HelmCreateBasicAuthSecretModalLauncher => { + const launcher = useOverlay(); + return useCallback((props) => launcher(HelmCreateBasicAuthSecretModal, props), [launcher]); +}; 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 ec31445c752..d336e975905 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 @@ -12,6 +12,7 @@ import { FormHeader } from '@console/shared/src/components/form-utils/FormHeader import { InputField } from '@console/shared/src/components/formik-fields/InputField'; import { ResourceDropdownField } from '@console/shared/src/components/formik-fields/ResourceDropdownField'; import type { HelmURLChartFormData } from './types'; +import { useBasicAuthSecretDropdown, CREATE_SECRET_KEY } from './useBasicAuthSecretDropdown'; import { useSecretResources } from './useSecretResources'; interface HelmURLChartFormProps { @@ -32,9 +33,14 @@ const HelmURLChartForm: FC & HelmURLChartFormP setFieldError, }) => { const { t } = useTranslation('helm-plugin'); + const { handleSecretChange } = useBasicAuthSecretDropdown({ + namespace, + currentSecretName: values.basicAuthSecretName, + setFieldValue, + }); - const autocompleteFilter = (strText: string, item: any): boolean => - fuzzy(strText, item?.props?.name); + const autocompleteFilter = (strText: string, item: any, key?: string): boolean => + fuzzy(strText, item?.props?.name || (typeof item === 'string' ? item : key) || ''); const secretResources = useSecretResources(namespace); const isNextDisabled = !isValid || !dirty || isSubmitting; @@ -138,8 +144,16 @@ const HelmURLChartForm: FC & HelmURLChartFormP placeholder={t('Select a secret')} showBadge autocompleteFilter={autocompleteFilter} + actionItems={[ + { + actionTitle: t('Create Secret'), + actionKey: CREATE_SECRET_KEY, + }, + ]} + onChange={handleSecretChange} helpText={t( - 'A secret with "username" and "password" keys for OCI/HTTP(S) authentication', + 'Secret with "{{username}}" and "{{password}}" keys for OCI/HTTP(S) authentication.', + { username: 'username', password: 'password' }, )} /> 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 9910c4f8306..90c2044bd11 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 @@ -158,7 +158,8 @@ const HelmURLInstallForm: FC & HelmURLInstal autocompleteFilter={autocompleteFilter} disabled helpText={t( - 'A secret with "username" and "password" keys for OCI/HTTP(S) authentication', + 'Secret with "{{username}}" and "{{password}}" keys for OCI/HTTP(S) authentication.', + { username: 'username', password: 'password' }, )} /> diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/__tests__/HelmCreateBasicAuthSecretModal.spec.tsx b/frontend/packages/helm-plugin/src/components/forms/url-chart/__tests__/HelmCreateBasicAuthSecretModal.spec.tsx new file mode 100644 index 00000000000..c0b5151109a --- /dev/null +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/__tests__/HelmCreateBasicAuthSecretModal.spec.tsx @@ -0,0 +1,121 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { SecretModel } from '@console/internal/models'; +import { k8sCreate } from '@console/internal/module/k8s'; +import { HelmCreateBasicAuthSecretModal } from '../HelmCreateBasicAuthSecretModal'; + +jest.mock('@console/internal/module/k8s', () => ({ + ...jest.requireActual('@console/internal/module/k8s'), + k8sCreate: jest.fn(), +})); + +const mockCloseOverlay = jest.fn(); +const mockSave = jest.fn(); +const mockOnClose = jest.fn(); + +const defaultProps = { + closeOverlay: mockCloseOverlay, + namespace: 'test-ns', + save: mockSave, + onClose: mockOnClose, +}; + +describe('HelmCreateBasicAuthSecretModal', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should render the modal with all form fields and buttons', () => { + render(); + + expect(screen.getByText('Secret name')).toBeVisible(); + expect(screen.getByText('Secret username')).toBeVisible(); + expect(screen.getByText('Secret password or token')).toBeVisible(); + expect(screen.getByRole('button', { name: 'Create' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeVisible(); + }); + + it('should disable the Create button when required fields are empty', () => { + render(); + + const createButton = screen.getByRole('button', { name: 'Create' }); + expect(createButton).toBeDisabled(); + }); + + it('should enable the Create button when all required fields are filled', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByTestId('helm-secret-name'), 'my-secret'); + await user.type(screen.getByTestId('helm-secret-username'), 'admin'); + await user.type(screen.getByTestId('helm-secret-password'), 'pass123'); + + expect(screen.getByRole('button', { name: 'Create' })).toBeEnabled(); + }); + + it('should call k8sCreate with correct Secret shape and invoke save callback on success', async () => { + const user = userEvent.setup(); + (k8sCreate as jest.Mock).mockResolvedValue({}); + render(); + + await user.type(screen.getByTestId('helm-secret-name'), ' my-secret '); + await user.type(screen.getByTestId('helm-secret-username'), 'admin'); + await user.type(screen.getByTestId('helm-secret-password'), 'pass123'); + await user.click(screen.getByRole('button', { name: 'Create' })); + + await waitFor(() => { + expect(k8sCreate).toHaveBeenCalledWith(SecretModel, { + apiVersion: 'v1', + kind: 'Secret', + metadata: { + name: 'my-secret', + namespace: 'test-ns', + }, + type: 'kubernetes.io/basic-auth', + stringData: { + username: 'admin', + password: 'pass123', + }, + }); + }); + + expect(mockSave).toHaveBeenCalledWith('my-secret'); + expect(mockCloseOverlay).toHaveBeenCalled(); + }); + + it('should display error message when k8sCreate fails', async () => { + const user = userEvent.setup(); + (k8sCreate as jest.Mock).mockRejectedValue(new Error('Secret already exists')); + render(); + + await user.type(screen.getByTestId('helm-secret-name'), 'my-secret'); + await user.type(screen.getByTestId('helm-secret-username'), 'admin'); + await user.type(screen.getByTestId('helm-secret-password'), 'pass123'); + await user.click(screen.getByRole('button', { name: 'Create' })); + + expect(await screen.findByText('Secret already exists')).toBeVisible(); + expect(mockSave).not.toHaveBeenCalled(); + expect(mockCloseOverlay).not.toHaveBeenCalled(); + }); + + it('should call closeOverlay and onClose when Cancel button is clicked', async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mockCloseOverlay).toHaveBeenCalled(); + expect(mockOnClose).toHaveBeenCalled(); + expect(mockSave).not.toHaveBeenCalled(); + }); + + it('should keep Create button disabled when only secret name and password are filled but username is empty', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByTestId('helm-secret-name'), 'my-secret'); + await user.type(screen.getByTestId('helm-secret-password'), 'pass123'); + + expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); + }); +}); diff --git a/frontend/packages/helm-plugin/src/components/forms/url-chart/useBasicAuthSecretDropdown.ts b/frontend/packages/helm-plugin/src/components/forms/url-chart/useBasicAuthSecretDropdown.ts new file mode 100644 index 00000000000..34825846500 --- /dev/null +++ b/frontend/packages/helm-plugin/src/components/forms/url-chart/useBasicAuthSecretDropdown.ts @@ -0,0 +1,46 @@ +import { useState, useCallback } from 'react'; +import { useHelmCreateBasicAuthSecretModal } from './HelmCreateBasicAuthSecretModal'; + +const CREATE_SECRET_KEY = '__create_secret__'; +const NONE_SECRET_KEY = '__none__'; + +export { CREATE_SECRET_KEY, NONE_SECRET_KEY }; + +interface UseBasicAuthSecretDropdownOptions { + namespace: string; + currentSecretName: string; + setFieldValue: (field: string, value: string) => void; +} + +export const useBasicAuthSecretDropdown = ({ + namespace, + currentSecretName, + setFieldValue, +}: UseBasicAuthSecretDropdownOptions) => { + const launchModal = useHelmCreateBasicAuthSecretModal(); + const [isCreateSecretModalOpen, setIsCreateSecretModalOpen] = useState(false); + + const handleSecretChange = useCallback( + (key: string) => { + if (key === NONE_SECRET_KEY) { + window.setTimeout(() => setFieldValue('basicAuthSecretName', NONE_SECRET_KEY), 0); + return; + } + if (key === CREATE_SECRET_KEY && !isCreateSecretModalOpen) { + window.setTimeout(() => setFieldValue('basicAuthSecretName', currentSecretName || ''), 0); + setIsCreateSecretModalOpen(true); + launchModal({ + namespace, + save: (name) => { + setFieldValue('basicAuthSecretName', name); + setIsCreateSecretModalOpen(false); + }, + onClose: () => setIsCreateSecretModalOpen(false), + }); + } + }, + [isCreateSecretModalOpen, launchModal, namespace, currentSecretName, setFieldValue], + ); + + return { handleSecretChange }; +}; diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index d82dab25411..8ef617a871c 100644 --- a/pkg/helm/actions/get_chart.go +++ b/pkg/helm/actions/get_chart.go @@ -3,6 +3,7 @@ package actions import ( "fmt" "os" + "strings" "github.com/openshift/api/helm/v1beta1" "helm.sh/helm/v4/pkg/action" @@ -83,6 +84,9 @@ func GetChartFromURL(url string, conf *action.Configuration, namespace string, c } chartLocation, err := cmd.ChartPathOptions.LocateChart(url, settings) if err != nil { + if basicAuthSecretName == "" && (strings.Contains(err.Error(), "401") || strings.Contains(strings.ToLower(err.Error()), "unauthorized")) { + return nil, fmt.Errorf("error getting chart from URL: %w; registry requires authentication - select a Secret with \"username\" and \"password\" keys for basic authentication", err) + } return nil, fmt.Errorf("error getting chart from URL: %v", err) } return loader.Load(chartLocation) diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index 9cb08a05339..00b069c8ff0 100644 --- a/pkg/helm/actions/install_chart.go +++ b/pkg/helm/actions/install_chart.go @@ -358,6 +358,9 @@ func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf cp, err := cmd.ChartPathOptions.LocateChart(url, settings) if err != nil { + if basicAuthSecretName == "" && (strings.Contains(err.Error(), "401") || strings.Contains(strings.ToLower(err.Error()), "unauthorized")) { + return nil, fmt.Errorf("error locating chart: %w; registry requires authentication - select a Secret with \"username\" and \"password\" keys for basic authentication", err) + } return nil, fmt.Errorf("error locating chart: %v", err) } ch, err := loader.Load(cp) diff --git a/pkg/helm/actions/upgrade_release.go b/pkg/helm/actions/upgrade_release.go index 25d007d38aa..7d75e9fdce5 100644 --- a/pkg/helm/actions/upgrade_release.go +++ b/pkg/helm/actions/upgrade_release.go @@ -19,7 +19,6 @@ 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( @@ -107,12 +106,17 @@ func UpgradeRelease( return nil, err } - // Ensure chart URL is properly set in the upgrade chart + // Ensure chart URL and installation method are properly set in the upgrade chart if chartUrl != "" { if ch.Metadata.Annotations == nil { ch.Metadata.Annotations = make(map[string]string) } ch.Metadata.Annotations["chart_url"] = chartUrl + if rel.Chart.Metadata != nil && rel.Chart.Metadata.Annotations != nil { + if inst, ok := rel.Chart.Metadata.Annotations["installation"]; ok { + ch.Metadata.Annotations["installation"] = inst + } + } } result, err := client.Run(releaseName, ch, vals) @@ -149,6 +153,7 @@ func UpgradeReleaseAsync( coreClient corev1client.CoreV1Interface, fileCleanUp bool, indexEntry string, + basicAuthSecretName string, ) (*kv1.Secret, error) { client := action.NewUpgrade(conf) client.ServerSideApply = "false" @@ -167,14 +172,21 @@ func UpgradeReleaseAsync( return nil, err } - auth_secret := "" + auth_secret := basicAuthSecretName + // "__none__" is a sentinel from the frontend meaning the user explicitly cleared the secret. + explicitlyClearedSecret := auth_secret == "__none__" + if explicitlyClearedSecret { + auth_secret = "" + } // Before proceeding, check if chart URL is present as an annotation - if rel.Chart.Metadata.Annotations != nil { + if rel.Chart.Metadata != nil && 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 == "" && !explicitlyClearedSecret { + if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok { + auth_secret = authSecret + } } } @@ -215,10 +227,10 @@ func UpgradeReleaseAsync( if auth_secret != "" { userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret) if err != nil { - klog.Errorf("Failed to get user credentials Secret %s for release upgrade %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) + return nil, fmt.Errorf("failed to get user credentials Secret %s for release upgrade %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) } else { if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil { - klog.Errorf("Failed to apply auth from Secret %s for release upgrade %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) + return nil, fmt.Errorf("failed to apply auth from Secret %s for release upgrade %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) } } } @@ -226,6 +238,9 @@ func UpgradeReleaseAsync( client.ChartPathOptions.Version = chartInfo.Version cp, err = client.ChartPathOptions.LocateChart(chartLocation, settings) if err != nil { + if auth_secret == "" && (strings.Contains(err.Error(), "401") || strings.Contains(strings.ToLower(err.Error()), "unauthorized")) { + return nil, fmt.Errorf("failed to upgrade helm release: %w; registry requires authentication - select a Secret with \"username\" and \"password\" keys for basic authentication", err) + } return nil, err } ch, err = loader.Load(cp) @@ -247,6 +262,11 @@ func UpgradeReleaseAsync( } if chartUrl != "" { ch.Metadata.Annotations["chart_url"] = chartUrl + if rel.Chart.Metadata != nil && rel.Chart.Metadata.Annotations != nil { + if inst, ok := rel.Chart.Metadata.Annotations["installation"]; ok { + ch.Metadata.Annotations["installation"] = inst + } + } addAuthSecretAnnotation(ch, auth_secret) } go func() { diff --git a/pkg/helm/actions/upgrade_release_test.go b/pkg/helm/actions/upgrade_release_test.go index 6f123437f6c..ed6dacff907 100644 --- a/pkg/helm/actions/upgrade_release_test.go +++ b/pkg/helm/actions/upgrade_release_test.go @@ -502,7 +502,7 @@ func TestUpgradeReleaseWithoutDependenciesAsync(t *testing.T) { var rel *v1.Secret var err error go func() { - rel, err = UpgradeReleaseAsync(tt.namespace, tt.releaseName, tt.chartPath, nil, actionConfig, client, coreClient, false, tt.indexEntry) + rel, err = UpgradeReleaseAsync(tt.namespace, tt.releaseName, tt.chartPath, nil, actionConfig, client, coreClient, false, tt.indexEntry, "") if tt.requireErr { fmt.Println("Error", err) require.Error(t, err) @@ -597,7 +597,7 @@ func TestUpgradeReleaseWithDependenciesAsync(t *testing.T) { store.Create(&r) go func() { - rel, err = UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, tt.chartPath, tt.values, actionConfig, client, coreClient, true, tt.indexEntry) + rel, err = UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, tt.chartPath, tt.values, actionConfig, client, coreClient, true, tt.indexEntry, "") require.NoError(t, err) require.Equal(t, fmt.Sprintf("sh.helm.release.v1.%v.v2", tt.releaseName), rel.ObjectMeta.Name) }() @@ -688,7 +688,7 @@ func TestUpgradeReleaseWithCustomValuesAsync(t *testing.T) { var rel *v1.Secret var err error go func() { - rel, err = UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, tt.chartPath, tt.values, actionConfig, client, coreClient, true, tt.indexEntry) + rel, err = UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, tt.chartPath, tt.values, actionConfig, client, coreClient, true, tt.indexEntry, "") require.NoError(t, err) require.Equal(t, fmt.Sprintf("sh.helm.release.v1.%v.v2", tt.releaseName), rel.ObjectMeta.Name) }() @@ -775,7 +775,7 @@ func TestUpgradeAfterURLInstallWithSecrets(t *testing.T) { // Upgrade — chartUrl is recovered from the annotation, auth credentials are applied secretsDriver := driver.NewSecrets(coreClient.Secrets(tt.releaseNamespace)) go func() { - upgradeResult, upgradeErr := UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, "", nil, actionConfig, dynamicClient, coreClient, true, "") + upgradeResult, upgradeErr := UpgradeReleaseAsync(tt.releaseNamespace, tt.releaseName, "", nil, actionConfig, dynamicClient, coreClient, true, "", "") require.NoError(t, upgradeErr) require.Equal(t, fmt.Sprintf("sh.helm.release.v1.%v.v2", tt.releaseName), upgradeResult.ObjectMeta.Name) }() @@ -791,3 +791,141 @@ func TestUpgradeAfterURLInstallWithSecrets(t *testing.T) { }) } } + +func TestUpgradeAsyncExplicitSecretOverridesAnnotation(t *testing.T) { + store := storage.Init(driver.NewMemory()) + actionConfig := &action.Configuration{ + RESTClientGetter: FakeConfig{}, + Releases: store, + KubeClient: &kubefake.PrintingKubeClient{Out: io.Discard}, + Capabilities: common.DefaultCapabilities, + } + + // Only create the override secret, not the annotation secret. + // If the code incorrectly uses the annotation value ("missing-secret"), + // GetUserCredentials will fail because that secret doesn't exist. + overrideSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "new-secret", Namespace: "test-ns"}, + Data: map[string][]byte{username: []byte("new-user"), password: []byte("new-pass")}, + } + clientInterface := k8sfake.NewSimpleClientset(overrideSecret) + coreClient := clientInterface.CoreV1() + registryClient, err := GetDefaultOCIRegistry() + require.NoError(t, err) + actionConfig.RegistryClient = registryClient + dynamicClient := K8sDynamicClientFromCRs() + + installRelease := releasev1.Release{ + Name: "override-test", + Namespace: "test-ns", + Info: &releasev1.Info{FirstDeployed: helmTime.Time{}, Status: "deployed"}, + Version: 1, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "mychart", + Version: "0.1.0", + Annotations: map[string]string{ + "chart_url": "http://localhost:8181/charts/mychart-0.1.0.tgz", + helmAuthSecretAnnotation: "missing-secret", + }, + }, + }, + } + store.Create(&installRelease) + + // Pass "new-secret" explicitly — this should override "missing-secret" from the annotation. + // The chart fetch will fail with 401 (fake credentials), but the key assertion is that + // it does NOT fail with "failed to get user credentials Secret missing-secret" which + // would indicate the annotation value was used instead of the explicit override. + _, err = UpgradeReleaseAsync("test-ns", "override-test", "", nil, actionConfig, dynamicClient, coreClient, true, "", "new-secret") + require.Error(t, err) + require.NotContains(t, err.Error(), "missing-secret", "should use the explicit secret, not the annotation value") +} + +func TestUpgradeAsyncNoneSentinelClearsAnnotationSecret(t *testing.T) { + store := storage.Init(driver.NewMemory()) + actionConfig := &action.Configuration{ + RESTClientGetter: FakeConfig{}, + Releases: store, + KubeClient: &kubefake.PrintingKubeClient{Out: io.Discard}, + Capabilities: common.DefaultCapabilities, + } + + // Create the annotation secret — if __none__ doesn't work, the code would + // fall back to this secret and NOT produce a 401 auth hint. + annotationSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "old-secret", Namespace: "test-ns"}, + Data: map[string][]byte{username: []byte("user"), password: []byte("pass")}, + } + clientInterface := k8sfake.NewSimpleClientset(annotationSecret) + coreClient := clientInterface.CoreV1() + registryClient, err := GetDefaultOCIRegistry() + require.NoError(t, err) + actionConfig.RegistryClient = registryClient + dynamicClient := K8sDynamicClientFromCRs() + + installRelease := releasev1.Release{ + Name: "none-test", + Namespace: "test-ns", + Info: &releasev1.Info{FirstDeployed: helmTime.Time{}, Status: "deployed"}, + Version: 1, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "mychart", + Version: "0.1.0", + Annotations: map[string]string{ + "chart_url": "http://localhost:8181/charts/mychart-0.1.0.tgz", + helmAuthSecretAnnotation: "old-secret", + }, + }, + }, + } + store.Create(&installRelease) + + // "__none__" sentinel — should clear the secret and NOT fall back to the annotation. + // The chart fetch will fail with 401 since no auth is applied, proving the sentinel worked. + _, err = UpgradeReleaseAsync("test-ns", "none-test", "", nil, actionConfig, dynamicClient, coreClient, true, "", "__none__") + require.Error(t, err) + require.Contains(t, err.Error(), "registry requires authentication", + "should get 401 hint because __none__ cleared the secret and skipped annotation fallback") +} + +func TestUpgradeAsyncGetUserCredentialsFailureReturnsError(t *testing.T) { + store := storage.Init(driver.NewMemory()) + actionConfig := &action.Configuration{ + RESTClientGetter: FakeConfig{}, + Releases: store, + KubeClient: &kubefake.PrintingKubeClient{Out: io.Discard}, + Capabilities: common.DefaultCapabilities, + } + + // No secrets in the fake client — GetUserCredentials will fail + clientInterface := k8sfake.NewSimpleClientset() + coreClient := clientInterface.CoreV1() + registryClient, err := GetDefaultOCIRegistry() + require.NoError(t, err) + actionConfig.RegistryClient = registryClient + dynamicClient := K8sDynamicClientFromCRs() + + installRelease := releasev1.Release{ + Name: "err-test", + Namespace: "test-ns", + Info: &releasev1.Info{FirstDeployed: helmTime.Time{}, Status: "deployed"}, + Version: 1, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "mychart", + Version: "0.1.0", + Annotations: map[string]string{ + "chart_url": "http://localhost:8181/charts/mychart-0.1.0.tgz", + }, + }, + }, + } + store.Create(&installRelease) + + // Pass a secret name that doesn't exist — should return an error, not silently log + _, err = UpgradeReleaseAsync("test-ns", "err-test", "", nil, actionConfig, dynamicClient, coreClient, true, "", "nonexistent-secret") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to get user credentials Secret nonexistent-secret") +} diff --git a/pkg/helm/handlers/handler_test.go b/pkg/helm/handlers/handler_test.go index 0e8b2409c5c..f4e17fdc6e0 100644 --- a/pkg/helm/handlers/handler_test.go +++ b/pkg/helm/handlers/handler_test.go @@ -137,8 +137,8 @@ func fakeUpgradeRelease(name, ns string, t *testing.T, fakeRelease *releasev1.Re } } -func fakeUpgradeReleaseAsync(name, ns string, t *testing.T, fakeSecret *kv1.Secret, err error) func(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, client dynamic.Interface, coreClient corev1client.CoreV1Interface, fileCleanUp bool, indexEntry string) (*kv1.Secret, error) { - return func(namespace, n, url string, vals map[string]interface{}, conf *action.Configuration, client dynamic.Interface, coreClient corev1client.CoreV1Interface, fileCleanUp bool, indexEntry string) (*kv1.Secret, error) { +func fakeUpgradeReleaseAsync(name, ns string, t *testing.T, fakeSecret *kv1.Secret, err error) func(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, client dynamic.Interface, coreClient corev1client.CoreV1Interface, fileCleanUp bool, indexEntry string, basicAuthSecretName string) (*kv1.Secret, error) { + return func(namespace, n, url string, vals map[string]interface{}, conf *action.Configuration, client dynamic.Interface, coreClient corev1client.CoreV1Interface, fileCleanUp bool, indexEntry string, basicAuthSecretName string) (*kv1.Secret, error) { if namespace != ns { t.Errorf("Namespace mismatch expected %s received %s", ns, namespace) } diff --git a/pkg/helm/handlers/handlers.go b/pkg/helm/handlers/handlers.go index 4cfefa057d1..2d288acd165 100644 --- a/pkg/helm/handlers/handlers.go +++ b/pkg/helm/handlers/handlers.go @@ -67,7 +67,7 @@ type helmHandlers struct { installChart func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*releasev1.Release, error) installChartFromURL func(string, string, string, map[string]interface{}, *action.Configuration, corev1client.CoreV1Interface, string, string) (*kv1.Secret, error) listReleases func(*action.Configuration, bool) ([]*releasev1.Release, error) - upgradeReleaseAsync func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*kv1.Secret, error) + upgradeReleaseAsync func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string, string) (*kv1.Secret, error) upgradeRelease func(string, string, string, map[string]interface{}, *action.Configuration, dynamic.Interface, corev1client.CoreV1Interface, bool, string) (*releasev1.Release, error) uninstallRelease func(string, *action.Configuration) (*releasecommon.UninstallReleaseResponse, error) uninstallReleaseAsync func(string, string, string, *action.Configuration, corev1client.CoreV1Interface) error @@ -309,7 +309,7 @@ func (h *helmHandlers) HandleUpgradeReleaseAsync(user *auth.User, w http.Respons serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: err.Error()}) return } - resp, err := h.upgradeReleaseAsync(req.Namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.DynamicClient, handlerClients.CoreClient, false, req.IndexEntry) + resp, err := h.upgradeReleaseAsync(req.Namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.DynamicClient, handlerClients.CoreClient, false, req.IndexEntry, req.BasicAuthSecretName) if err != nil { if err.Error() == actions.ErrReleaseRevisionNotFound.Error() { serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm releases: %v", err)})