diff --git a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.spec.tsx b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.spec.tsx index b588580a006..5717d2ac2ff 100644 --- a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.spec.tsx +++ b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.spec.tsx @@ -249,6 +249,15 @@ describe('ApplicationContainerStepSummary', () => { }) }) + it('does not send unsupported JSON references to the external variable API', async () => { + const { userEvent } = renderComponent({ + externalSecrets: [{ ...externalSecretsData[0], isJson: true }], + }) + await userEvent.click(screen.getByTestId('button-create')) + await waitFor(() => expect(mockNavigate).toHaveBeenCalled()) + expect(mockCreateVariable).not.toHaveBeenCalled() + }) + it('continues service creation when external secret creation fails', async () => { mockCreateVariable.mockRejectedValueOnce(new Error('External secret creation failed')) const { userEvent } = renderComponent() diff --git a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.tsx b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.tsx index a4ddc3b04ab..503e3f7d437 100644 --- a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.tsx +++ b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-summary/step-summary/step-summary.tsx @@ -115,6 +115,9 @@ export function ApplicationContainerStepSummary({ // External secret creation errors are surfaced by the mutation notifications but should not block the service creation flow. await Promise.allSettled( externalSecretsData.map(async (externalSecret) => { + // TODO(secret-manager-json): Persist the form’s JSON drafts here when the real API handler is available. + // Until then they are local only; never send JSON references through the single-variable API. + if (externalSecret.isJson) return const scope = externalSecret.scope ?? createPayload.serviceType return createVariable({ variableRequest: { diff --git a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-variables/step-variables/step-variables.spec.tsx b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-variables/step-variables/step-variables.spec.tsx index 1df02555b3b..f8c4498d78f 100644 --- a/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-variables/step-variables/step-variables.spec.tsx +++ b/libs/domains/services/feature/src/lib/service-creation-flow/application-container/application-container-variables/step-variables/step-variables.spec.tsx @@ -1,4 +1,6 @@ -import { type ReactNode } from 'react' +import { type SecretManagerAccess } from 'qovery-typescript-axios' +import { type ReactNode, useEffect } from 'react' +import { type AddSecretModalSubmitData } from '@qovery/domains/variables/feature' import { renderWithProviders, screen, waitFor } from '@qovery/shared/util-tests' import { ApplicationContainerCreationFlow, @@ -8,7 +10,7 @@ import { ApplicationContainerStepVariables } from './step-variables' const mockNavigate = jest.fn() const mockUseVariablesSecretManagers = jest.fn(() => ({ - secretManagers: [], + secretManagers: [] as SecretManagerAccess[], hasClusterSecretManagerConfigured: false, clusterId: 'cluster-1', })) @@ -36,7 +38,35 @@ jest.mock('@tanstack/react-router', () => ({ })) jest.mock('@qovery/domains/variables/feature', () => ({ - AddSecretModal: () => null, + AddSecretDropdown: jest.requireActual('@qovery/domains/variables/feature').AddSecretDropdown, + AddSecretModal: ({ + isFile, + isJson, + onSubmit, + onClose, + }: { + isFile?: boolean + isJson?: boolean + onSubmit: (data: AddSecretModalSubmitData) => void | Promise + onClose: () => void + }) => ( + + ), VariableFormModal: ({ onSubmit, scope, @@ -77,6 +107,21 @@ function VariablesState() { return
{JSON.stringify(variablesForm.watch())}
} +function JsonSecretStep({ keyName }: { keyName?: string }) { + const { variablesForm } = useApplicationContainerCreateContext() + useEffect(() => { + variablesForm.setValue('externalSecrets', [ + { + name: 'Application credentials', + reference: 'lucis/production/application', + isJson: true, + keyName, + }, + ]) + }, [variablesForm, keyName]) + return +} + describe('ApplicationContainerStepVariables', () => { const onBack = jest.fn() const onSubmit = jest.fn() @@ -164,6 +209,84 @@ describe('ApplicationContainerStepVariables', () => { expect(onSubmit).toHaveBeenCalled() }) + it.each([ + ['Add secret as variable', 'Variable secret modal'], + ['Add secret as file', 'File secret modal'], + ['Add secret as JSON', 'JSON secret modal'], + ])('opens %s from the external secret dropdown', async (option, modal) => { + mockUseVariablesSecretManagers.mockReturnValue({ + secretManagers: [{ id: 'first-source' }, { id: 'second-source' }] as SecretManagerAccess[], + hasClusterSecretManagerConfigured: true, + clusterId: 'cluster-1', + }) + const { userEvent } = renderWithProviders( + + + + ) + expect(screen.queryByRole('button', { name: 'Add secret as file' })).not.toBeInTheDocument() + const addButtons = screen.getAllByRole('button', { name: 'Add secret' }) + await userEvent.click(addButtons[addButtons.length - 1]) + expect(screen.getAllByRole('menuitem').map((item) => item.textContent)).toEqual([ + 'Add secret as variable', + 'Add secret as file', + 'Add secret as JSON', + ]) + await userEvent.click(screen.getByRole('menuitem', { name: option })) + expect(screen.getByText(modal)).toBeInTheDocument() + expect(screen.getByRole('button', { name: modal })).toBeEnabled() + }) + + it('adds and removes a user-entered JSON reference in the creation form', async () => { + mockUseVariablesSecretManagers.mockReturnValue({ + secretManagers: [], + hasClusterSecretManagerConfigured: true, + clusterId: 'cluster-1', + }) + const { userEvent } = renderWithProviders( + + + + + ) + const buttons = screen.getAllByRole('button', { name: 'Add secret' }) + await userEvent.click(buttons[buttons.length - 1]) + await userEvent.click(screen.getByRole('menuitem', { name: 'Add secret as JSON' })) + await userEvent.click(screen.getByRole('button', { name: 'JSON secret modal' })) + expect(screen.getByText('My JSON credentials')).toBeInTheDocument() + expect(screen.getByText('Key: APP')).toBeInTheDocument() + expect(screen.getByTestId('variables-state')).toHaveTextContent('"keyName":"APP"') + await userEvent.click(screen.getByRole('button', { name: 'Delete secret' })) + expect(screen.queryByText('My JSON credentials')).not.toBeInTheDocument() + }) + + it.each(['APP', 'APP_'])('renders JSON references with key name %s without expandable key details', (keyName) => { + mockUseVariablesSecretManagers.mockReturnValue({ + secretManagers: [], + hasClusterSecretManagerConfigured: true, + clusterId: 'cluster-1', + }) + renderWithProviders( + + + + ) + expect(screen.getByText('Application credentials')).toBeInTheDocument() + expect(screen.getByText('lucis/production/application')).toBeInTheDocument() + expect(screen.getByText(`Key: ${keyName}`)).toBeInTheDocument() + expect(screen.queryByLabelText(/Expand keys|Collapse keys|Copy DATABASE_URL/)).not.toBeInTheDocument() + expect(screen.queryByText(/secrets imported/)).not.toBeInTheDocument() + }) + it('does not expose external secret creation when no secret manager is linked', () => { renderWithProviders( void | Promise } -function cardHeaderActions({ - onAddDefault, - onAddAsFile, - defaultLabel, - asFileLabel, -}: { - onAddDefault: () => void - onAddAsFile: () => void - defaultLabel: string - asFileLabel: string -}) { - return ( -
- - -
- ) -} - -function emptyState({ - title, - onAddDefault, - onAddAsFile, - defaultLabel, - asFileLabel, -}: { - title: string - onAddDefault: () => void - onAddAsFile: () => void - defaultLabel: string - asFileLabel: string -}) { - return ( - -
- - -
- - ) -} - function mapModalDataToVariable(data: CreateUpdateVariableModalSubmitData, current?: VariableData): VariableData { return { variable: data.key, @@ -133,6 +66,7 @@ function mapSecretForForm( description: secret.description, filePath: secret.filePath, isFile: secret.isFile, + ...(secret.isJson ? { isJson: true, keyName: secret.keyName } : {}), reference: secret.reference, secretManagerAccessId: secret.secretManagerAccessId, source: source?.tableLabel ?? null, @@ -216,7 +150,15 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat }) } - const openSecretModal = ({ isFile, index }: { isFile: boolean; index?: number }) => { + const openSecretModal = ({ + isFile = false, + isJson = false, + index, + }: { + isFile?: boolean + isJson?: boolean + index?: number + }) => { const currentSecret = typeof index === 'number' ? externalSecrets[index] : undefined const defaultSource = secretSources.find((source) => source.value === currentSecret?.secretManagerAccessId) ?? @@ -230,9 +172,11 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat secretSources={secretSources} defaultSource={defaultSource} isFile={isFile} + isJson={isJson} initialSecret={currentSecret} onClose={closeModal} onSubmit={(secret) => { + // TODO(secret-manager-json): Persist JSON references with the real API in the final creation step. const mapped = mapSecretForForm(secret, sourceById, serviceScope) if (typeof index === 'number') { updateExternalSecret(index, mapped) @@ -307,10 +251,9 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat color="neutral" variant="solid" size="md" - className="text-ssm" onClick={() => openVariableModal()} > - + Add variable @@ -419,14 +361,13 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat

External secrets

- {hasClusterSecretManagerConfigured && - externalSecrets.length > 0 && - cardHeaderActions({ - onAddDefault: () => openSecretModal({ isFile: false }), - onAddAsFile: () => openSecretModal({ isFile: true }), - defaultLabel: 'Add secret', - asFileLabel: 'Add secret as file', - })} + {hasClusterSecretManagerConfigured && externalSecrets.length > 0 && ( + openSecretModal({ isFile: format === 'file', isJson: format === 'json' })} + /> + )}
@@ -440,13 +381,16 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat className="h-auto rounded-none border-0 bg-transparent px-8 py-8" /> ) : externalSecrets.length === 0 ? ( - emptyState({ - title: 'No external secrets added yet', - onAddDefault: () => openSecretModal({ isFile: false }), - onAddAsFile: () => openSecretModal({ isFile: true }), - defaultLabel: 'Add secret', - asFileLabel: 'Add secret as file', - }) + + openSecretModal({ isFile: format === 'file', isJson: format === 'json' })} + /> + ) : ( <>
@@ -475,6 +419,12 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat )}
+ {secret.isJson && ( +
+ + {`Key: ${secret.keyName}`} +
+ )} {secret.filePath && (
@@ -495,7 +445,7 @@ export function ApplicationContainerStepVariables({ onBack, onSubmit }: Applicat variant="outline" size="xs" iconOnly - onClick={() => openSecretModal({ isFile: !!secret.isFile, index })} + onClick={() => openSecretModal({ isFile: !!secret.isFile, isJson: !!secret.isJson, index })} > diff --git a/libs/domains/variables/feature/src/index.ts b/libs/domains/variables/feature/src/index.ts index 1fa39cefaf4..183616421d4 100644 --- a/libs/domains/variables/feature/src/index.ts +++ b/libs/domains/variables/feature/src/index.ts @@ -20,3 +20,4 @@ export * from './lib/import-environment-variable-modal/import-environment-variab export * from './lib/external-secrets/external-secrets-tab' export * from './lib/external-secrets/secret-manager-feature-flag-entry-point' export * from './lib/external-secrets/add-secret-modal/add-secret-modal' +export * from './lib/external-secrets/add-secret-dropdown' diff --git a/libs/domains/variables/feature/src/lib/external-secrets/add-secret-dropdown.tsx b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-dropdown.tsx new file mode 100644 index 00000000000..af0ecabe19d --- /dev/null +++ b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-dropdown.tsx @@ -0,0 +1,52 @@ +import { Button, DropdownMenu, Icon } from '@qovery/shared/ui' + +export type SecretFormat = 'variable' | 'file' | 'json' + +const ADD_SECRET_OPTIONS = [ + { + value: 'variable', + label: 'Add secret as variable', + icon: 'key' as const, + }, + { + value: 'file', + label: 'Add secret as file', + icon: 'file-lines' as const, + }, + { + value: 'json', + label: 'Add secret as JSON', + icon: 'brackets-curly' as const, + }, +] satisfies { value: SecretFormat; label: string; icon: 'key' | 'file-lines' | 'brackets-curly' }[] + +type AddSecretDropdownProps = { + color?: 'brand' | 'neutral' + size?: 'sm' | 'md' + onSelect: (format: SecretFormat) => void +} + +export function AddSecretDropdown({ color = 'brand', size = 'md', onSelect }: AddSecretDropdownProps) { + return ( + + + + + + {ADD_SECRET_OPTIONS.map((option) => ( + } + onSelect={() => onSelect(option.value)} + > + {option.label} + + ))} + + + ) +} diff --git a/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.spec.tsx b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.spec.tsx index b8448053369..d7bd73e721f 100644 --- a/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.spec.tsx +++ b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.spec.tsx @@ -88,4 +88,122 @@ describe('AddSecretModal', () => { ) }) }) + it('imports a JSON key and follows its name until the secret name is overridden', async () => { + const onSubmit = jest.fn() + const { userEvent } = renderWithProviders( + + ) + expect(screen.queryByLabelText('Display name')).not.toBeInTheDocument() + expect(screen.queryByLabelText('Key prefix (optional)')).not.toBeInTheDocument() + expect(screen.queryByText('Variables to import')).not.toBeInTheDocument() + await userEvent.type(screen.getByLabelText('Reference'), 'production/database') + expect(screen.getByRole('button', { name: 'Add secret' })).toBeDisabled() + await userEvent.type(screen.getByLabelText('Key name'), 'DATABASE_URL') + expect(screen.getByLabelText('Secret name')).toHaveValue('DATABASE_URL') + await userEvent.clear(screen.getByLabelText('Key name')) + await userEvent.type(screen.getByLabelText('Key name'), 'DATABASE_PASSWORD') + expect(screen.getByLabelText('Secret name')).toHaveValue('DATABASE_PASSWORD') + await userEvent.clear(screen.getByLabelText('Secret name')) + expect(screen.getByRole('button', { name: 'Add secret' })).toBeDisabled() + await userEvent.type(screen.getByLabelText('Secret name'), 'APP_PASSWORD') + await userEvent.clear(screen.getByLabelText('Key name')) + await userEvent.type(screen.getByLabelText('Key name'), 'password') + expect(screen.getByLabelText('Secret name')).toHaveValue('APP_PASSWORD') + await userEvent.click(screen.getByRole('button', { name: 'Add secret' })) + await waitFor(() => + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'APP_PASSWORD', + keyName: 'password', + reference: 'production/database', + isJson: true, + }) + ) + ) + expect(onSubmit.mock.calls[0][0]).not.toHaveProperty('keyPrefix') + }) + + it.each(['Key name', 'Secret name'])('requires a nonblank %s for JSON secrets', async (label) => { + const onSubmit = jest.fn() + const { userEvent } = renderWithProviders( + + ) + await userEvent.type(screen.getByLabelText('Reference'), 'production/database') + await userEvent.type(screen.getByLabelText('Key name'), 'password') + await userEvent.clear(screen.getByLabelText(label)) + await userEvent.type(screen.getByLabelText(label), ' ') + expect(screen.getByRole('button', { name: 'Add secret' })).toBeDisabled() + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('keeps names unlinked after an override even when they match again', async () => { + const { userEvent } = renderWithProviders( + + ) + await userEvent.type(screen.getByLabelText('Key name'), 'password') + await userEvent.clear(screen.getByLabelText('Secret name')) + await userEvent.type(screen.getByLabelText('Secret name'), 'password') + await userEvent.type(screen.getByLabelText('Key name'), '_new') + expect(screen.getByLabelText('Secret name')).toHaveValue('password') + }) + + it('submits the auto-filled secret name without adding an underscore', async () => { + const onSubmit = jest.fn() + const { userEvent } = renderWithProviders( + + ) + await userEvent.type(screen.getByLabelText('Reference'), 'production/database') + await userEvent.type(screen.getByLabelText('Key name'), 'password') + await userEvent.click(screen.getByRole('button', { name: 'Add secret' })) + await waitFor(() => + expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ name: 'password', keyName: 'password' })) + ) + }) + + it('preserves the saved secret name when editing a JSON key and supports retry after failure', async () => { + const onClose = jest.fn() + const onSubmit = jest.fn().mockRejectedValue(new Error('Could not save')) + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined) + const { userEvent } = renderWithProviders( + + ) + expect(screen.getByLabelText('Secret name')).toHaveValue('APP_PASSWORD') + expect(screen.getByLabelText('Key name')).toHaveValue('password') + await userEvent.type(screen.getByLabelText('Key name'), '_new') + expect(screen.getByLabelText('Secret name')).toHaveValue('APP_PASSWORD') + await userEvent.click(screen.getByRole('button', { name: 'Edit secret' })) + await waitFor(() => + expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ name: 'APP_PASSWORD', keyName: 'password_new' })) + ) + expect(onClose).not.toHaveBeenCalled() + expect(screen.getByRole('alert')).toHaveTextContent('Could not save') + onSubmit.mockResolvedValueOnce(undefined) + await userEvent.click(screen.getByRole('button', { name: 'Edit secret' })) + await waitFor(() => expect(onClose).toHaveBeenCalled()) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + consoleError.mockRestore() + }) + + it('does not infer JSON keys from the reference name', async () => { + useSecretManagerProviderSecretsMock.mockReturnValue({ + data: { results: ['production/app'] }, + isError: false, + isFetching: false, + } as ReturnType) + const { userEvent } = renderWithProviders( + + ) + await userEvent.click(screen.getByLabelText('Reference')) + await userEvent.click(screen.getByText('production/app')) + expect(screen.getByLabelText('Key name')).toHaveValue('') + expect(screen.getByLabelText('Secret name')).toHaveValue('') + expect(screen.getByRole('button', { name: 'Add secret' })).toBeDisabled() + }) }) diff --git a/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.tsx b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.tsx index 218069b6302..f91308e37f4 100644 --- a/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.tsx +++ b/libs/domains/variables/feature/src/lib/external-secrets/add-secret-modal/add-secret-modal.tsx @@ -32,7 +32,9 @@ export type AddSecretModalSubmitData = { name: string description?: string filePath?: string + keyName?: string isFile: boolean + isJson?: boolean reference: string secretManagerAccessId: string } @@ -42,12 +44,14 @@ export type AddSecretModalInitialSecret = { reference: string description?: string filePath?: string + keyName?: string } interface AddSecretModalProps { secretSources: SecretSourceOption[] defaultSource?: SecretSourceOption isFile?: boolean + isJson?: boolean mode?: 'create' | 'edit' initialSecret?: AddSecretModalInitialSecret scope?: VariableScope @@ -60,6 +64,7 @@ type AddSecretFormValues = { reference: string path: string secretName: string + keyName: string description: string acknowledgeEnvironmentSecretCost: boolean } @@ -68,6 +73,7 @@ export function AddSecretModal({ secretSources, defaultSource, isFile = false, + isJson = false, mode = 'create', initialSecret, scope, @@ -82,6 +88,7 @@ export function AddSecretModal({ reference: initialSecret?.reference ?? '', path: initialSecret?.filePath ?? '', secretName: initialSecret?.name ?? '', + keyName: initialSecret?.keyName ?? '', description: initialSecret?.description ?? '', acknowledgeEnvironmentSecretCost: false, }, @@ -89,13 +96,14 @@ export function AddSecretModal({ }) const { enableAlertClickOutside } = useModal() const [referenceInput, setReferenceInput] = useState('') + const [submitError, setSubmitError] = useState() + const [isSecretNameOverridden, setIsSecretNameOverridden] = useState(mode === 'edit') const debouncedReferenceInput = useDebounce(referenceInput, 300) const isDirty = methods.formState.isDirty const shouldDisplayEnvironmentAcknowledgement = scope === 'ENVIRONMENT' && mode === 'create' const secretNameValue = methods.watch('secretName') const selectedSourceId = methods.watch('source') - const { data: providerSecrets, isError: isProviderSecretsError, @@ -127,11 +135,7 @@ export function AddSecretModal({ ) const referenceOptions = useMemo( - () => - (providerSecrets?.results ?? []).map((secret) => ({ - label: secret, - value: secret, - })), + () => (providerSecrets?.results ?? []).map((secret) => ({ label: secret, value: secret })), [providerSecrets?.results] ) @@ -147,29 +151,34 @@ export function AddSecretModal({ const fallbackFilePath = `/vault/secrets/${finalReference.split('/').pop() || 'secret'}` const finalPath = data.path.trim() || fallbackFilePath + setSubmitError(undefined) try { await onSubmit({ name: finalName, description: descriptionValue ? descriptionValue : undefined, filePath: isFile ? finalPath : undefined, isFile, + ...(isJson ? { isJson: true, keyName: data.keyName.trim() } : {}), reference: finalReference, secretManagerAccessId: selectedSource.value, }) - posthog.capture('external-secret-form-submitted', { - mode, - scope, - is_file: isFile, - success: true, - }) + if (!isJson) + posthog.capture('external-secret-form-submitted', { + mode, + scope, + is_file: isFile, + success: true, + }) onClose() } catch (error) { - posthog.capture('external-secret-form-submitted', { - mode, - scope, - is_file: isFile, - success: false, - }) + if (!isJson) + posthog.capture('external-secret-form-submitted', { + mode, + scope, + is_file: isFile, + success: false, + }) + setSubmitError(error instanceof Error ? error.message : 'Could not save the secret. Please try again.') console.error(error) } }) @@ -181,24 +190,23 @@ export function AddSecretModal({ return ( 'Edit secret as JSON') + .with({ isJson: true }, () => 'Add secret as JSON') + .with({ isFile: true, mode: 'edit' }, () => 'Edit secret as file') + .with({ isFile: true }, () => 'Add secret as file') + .with({ mode: 'edit' }, () => 'Edit secret') + .otherwise(() => 'Add secret')} + description={match({ isJson, isFile }) + .with( + { isJson: true }, + () => 'Qovery will import the selected JSON key as an environment variable at deployment.' + ) + .with( + { isFile: true }, + () => 'Qovery will resolve the external secret at deployment and mount it as a file inside your container.' + ) + .otherwise(() => 'Qovery will resolve the external secret at deployment.')} onSubmit={handleSubmit} onClose={onClose} submitLabel={mode === 'edit' ? 'Edit secret' : 'Add secret'} @@ -250,7 +258,7 @@ export function AddSecretModal({ onChange={(value) => { const selected = value as string field.onChange(selected) - if (!secretNameValue) { + if (!isJson && !secretNameValue) { const inferredName = selected.split('/').pop()?.toUpperCase() if (inferredName) { methods.setValue('secretName', inferredName, { shouldValidate: true }) @@ -284,18 +292,47 @@ export function AddSecretModal({ /> )} + {isJson && ( + !!value.trim() || 'Please enter a key name.' }} + render={({ field, fieldState: { error } }) => ( + { + field.onChange(event) + if (!isSecretNameOverridden) { + methods.setValue('secretName', event.target.value, { shouldValidate: true }) + } + }} + hint="Key to import from the selected JSON secret." + error={error?.message} + /> + )} + /> + )} + !!value.trim() || 'Please enter a secret name.' }} render={({ field, fieldState: { error } }) => ( { + field.onChange(event) + if (isJson) setIsSecretNameOverridden(true) + }} + hint={ + isJson ? 'Environment variable name to use in your code.' : 'Name that other Qovery services will use' + } error={error?.message} /> )} @@ -306,7 +343,7 @@ export function AddSecretModal({ control={methods.control} render={({ field }) => ( + {submitError && ( +

+ {submitError} +

+ )} + {shouldDisplayEnvironmentAcknowledgement && (
diff --git a/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.spec.tsx b/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.spec.tsx index 7a39ef598d8..f7845be1ce4 100644 --- a/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.spec.tsx +++ b/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.spec.tsx @@ -1,7 +1,7 @@ import { useParams } from '@tanstack/react-router' import { type ReactElement, type ReactNode } from 'react' import { useModal, useModalConfirmation } from '@qovery/shared/ui' -import { renderWithProviders, screen } from '@qovery/shared/util-tests' +import { act, renderWithProviders, screen } from '@qovery/shared/util-tests' import { useCreateVariable } from '../hooks/use-create-variable/use-create-variable' import { useDeleteVariable } from '../hooks/use-delete-variable/use-delete-variable' import { useEditVariable } from '../hooks/use-edit-variable/use-edit-variable' @@ -504,4 +504,85 @@ describe('ExternalSecretsTab', () => { '/organization/organization-id/cluster/cluster-id/settings/addons' ) }) + it('opens the JSON form without a forced submission block', async () => { + const openModal = jest.fn() + useModalMock.mockReturnValue({ openModal, closeModal: jest.fn() }) + useVariablesMock.mockReturnValue({ data: [] } as unknown as ReturnType) + useVariablesSecretManagersMock.mockReturnValue({ + secretManagers: [], + hasClusterSecretManagerConfigured: true, + clusterId: 'cluster-id', + }) + const { userEvent } = renderWithProviders() + await userEvent.click(screen.getByRole('button', { name: 'Add secret' })) + await userEvent.click(screen.getByRole('menuitem', { name: 'Add secret as JSON' })) + const modal = openModal.mock.calls[0][0].content as ReactElement<{ isJson: boolean; submitDisabled?: boolean }> + expect(modal.props.isJson).toBe(true) + expect(modal.props.submitDisabled).toBeUndefined() + expect(screen.queryByText('Lucis application secrets')).not.toBeInTheDocument() + }) + it('adds, edits and deletes user-entered JSON references without using the ordinary variable API', async () => { + const createVariable = jest.fn() + const editVariable = jest.fn() + const deleteVariable = jest.fn() + const openModal = jest.fn() + const openModalConfirmation = jest.fn() + useCreateVariableMock.mockReturnValue({ mutateAsync: createVariable } as unknown as ReturnType< + typeof useCreateVariable + >) + useEditVariableMock.mockReturnValue({ mutateAsync: editVariable } as unknown as ReturnType) + useDeleteVariableMock.mockReturnValue({ mutateAsync: deleteVariable } as unknown as ReturnType< + typeof useDeleteVariable + >) + useModalMock.mockReturnValue({ openModal, closeModal: jest.fn() }) + useModalConfirmationMock.mockReturnValue({ openModalConfirmation }) + useVariablesMock.mockReturnValue({ data: [] } as unknown as ReturnType) + useVariablesSecretManagersMock.mockReturnValue({ + secretManagers: [], + hasClusterSecretManagerConfigured: true, + clusterId: 'cluster-id', + }) + const { userEvent } = renderWithProviders() + await userEvent.click(screen.getByRole('button', { name: 'Add secret' })) + await userEvent.click(screen.getByRole('menuitem', { name: 'Add secret as JSON' })) + const modal = openModal.mock.calls[0][0].content as ReactElement<{ + isJson: boolean + initialSecret?: { keyName?: string } + onSubmit: (data: AddSecretModalSubmitData) => Promise + }> + expect(modal.props.isJson).toBe(true) + const data = { + name: 'Application credentials', + reference: 'lucis/app', + isJson: true, + isFile: false, + secretManagerAccessId: 'manager-id', + keyName: 'password', + } + await act(async () => modal.props.onSubmit(data)) + expect(screen.getByText('lucis/app')).toBeInTheDocument() + expect(screen.getByText('1 external secret')).toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: 'Edit' })) + const editModal = openModal.mock.calls[1][0].content as typeof modal + expect(editModal.props.isJson).toBe(true) + expect(screen.getByText('Key: password')).toBeInTheDocument() + expect(editModal.props.initialSecret?.keyName).toBe('password') + await act(async () => + editModal.props.onSubmit({ + ...data, + name: 'Updated credentials', + keyName: 'new_password', + reference: 'lucis/updated', + }) + ) + expect(screen.getByText('lucis/updated')).toBeInTheDocument() + expect(screen.getByText('Updated credentials')).toBeInTheDocument() + expect(screen.getByText('Key: new_password')).toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: 'Delete' })) + await act(async () => openModalConfirmation.mock.calls[0][0].action()) + expect(screen.queryByText('lucis/updated')).not.toBeInTheDocument() + expect(createVariable).not.toHaveBeenCalled() + expect(editVariable).not.toHaveBeenCalled() + expect(deleteVariable).not.toHaveBeenCalled() + }) }) diff --git a/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.tsx b/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.tsx index eb680add32e..a53d6da1be0 100644 --- a/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.tsx +++ b/libs/domains/variables/feature/src/lib/external-secrets/external-secrets-tab.tsx @@ -21,7 +21,6 @@ import { isExternalSecretVariable } from '@qovery/domains/variables/util' import { Button, Checkbox, - DropdownMenu, EmptyState, Icon, TableFilter, @@ -37,6 +36,7 @@ import { useDeleteVariable } from '../hooks/use-delete-variable/use-delete-varia import { useEditVariable } from '../hooks/use-edit-variable/use-edit-variable' import { useVariablesSecretManagers } from '../hooks/use-variables-secret-managers/use-variables-secret-managers' import { useVariables } from '../hooks/use-variables/use-variables' +import { AddSecretDropdown, type SecretFormat } from './add-secret-dropdown' import { AddSecretModal, type AddSecretModalSubmitData, @@ -46,19 +46,6 @@ import { const { Table } = TablePrimitives -const ADD_SECRET_OPTIONS = [ - { - value: 'variable', - label: 'Add secret as variable', - icon: 'key' as const, - }, - { - value: 'file', - label: 'Add secret as file', - icon: 'file-lines' as const, - }, -] - const gridLayoutClassName = 'grid w-full grid-cols-[32px_minmax(0,1fr)_240px_220px_140px_104px]' type ExternalSecretRow = { @@ -67,6 +54,9 @@ type ExternalSecretRow = { description?: string filePath?: string isFile?: boolean + isJson?: boolean + keyName?: string + secretManagerAccessId?: string reference: string source: string | null sourceIcon?: 'AWS' | 'GCP' @@ -75,36 +65,6 @@ type ExternalSecretRow = { const columnHelper = createColumnHelper() -type AddSecretDropdownProps = { - color?: 'brand' | 'neutral' - onSelect: (isFile: boolean) => void -} - -function AddSecretDropdown({ color = 'brand', onSelect }: AddSecretDropdownProps) { - return ( - - - - - - {ADD_SECRET_OPTIONS.map((option) => ( - } - onSelect={() => onSelect(option.value === 'file')} - > - {option.label} - - ))} - - - ) -} - export type ExternalSecretsTabProps = { scope: VariableScope parentId: string @@ -129,9 +89,12 @@ export function ExternalSecretsTab({ suspense: true, }) - const secrets = useMemo( - () => - variables.filter(isExternalSecretVariable).map((variable) => { + // TODO(secret-manager-json): Replace these user-entered drafts and their handlers with the real API. + // No seeded data; drafts reset on remount and are isolated to their service/environment. + const [jsonDrafts, setJsonDrafts] = useState<(ExternalSecretRow & { parentId: string })[]>([]) + const secrets = useMemo( + () => [ + ...variables.filter(isExternalSecretVariable).map((variable) => { const secretManager = secretManagers.find((manager) => manager.id === variable.secret_manager_access_id) return { @@ -146,7 +109,9 @@ export function ExternalSecretsTab({ scope: generateScopeLabel(variable.scope), } }), - [secretManagers, variables] + ...jsonDrafts.filter((secret) => secret.parentId === parentId), + ], + [secretManagers, variables, jsonDrafts, parentId] ) const [search, setSearch] = useState('') const [sorting, setSorting] = useState([]) @@ -165,7 +130,11 @@ export function ExternalSecretsTab({ if (!secret?.source) { return secretSources[0] } - return secretSources.find((option) => option.tableLabel === secret.source) ?? secretSources[0] + return ( + secretSources.find( + (option) => option.value === secret.secretManagerAccessId || option.tableLabel === secret.source + ) ?? secretSources[0] + ) }, [secretSources] ) @@ -176,16 +145,32 @@ export function ExternalSecretsTab({ description, filePath, isFile, + isJson, + keyName, reference, secretManagerAccessId, - }: { - name: string - description?: string - filePath?: string - isFile: boolean - reference: string - secretManagerAccessId: string - }) => { + }: AddSecretModalSubmitData) => { + // TODO(secret-manager-json): Replace local creation with the real JSON API handler. + if (isJson) { + const manager = secretManagers.find((item) => item.id === secretManagerAccessId) + setJsonDrafts((current) => [ + ...current, + { + id: `json-draft-${crypto.randomUUID()}`, + parentId, + name, + reference, + description, + isJson: true, + keyName, + secretManagerAccessId, + source: manager?.name ?? null, + sourceIcon: getSecretManagerProvider(manager), + scope: generateScopeLabel(scope), + }, + ]) + return + } await createVariable({ variableRequest: { key: name, @@ -200,14 +185,44 @@ export function ExternalSecretsTab({ }) onCreateSecret?.() }, - [createVariable, onCreateSecret, parentId, scope] + [createVariable, onCreateSecret, parentId, scope, secretManagers] ) const handleEditSecret = useCallback( async ( secretId: string, - { name, description, filePath, isFile, reference, secretManagerAccessId }: AddSecretModalSubmitData + { + name, + description, + filePath, + isFile, + isJson, + keyName, + reference, + secretManagerAccessId, + }: AddSecretModalSubmitData ) => { + // TODO(secret-manager-json): Replace local editing with the real JSON API handler. + if (isJson) { + const manager = secretManagers.find((item) => item.id === secretManagerAccessId) + setJsonDrafts((current) => + current.map((secret) => + secret.id === secretId + ? { + ...secret, + name, + reference, + description, + keyName, + secretManagerAccessId, + source: manager?.name ?? null, + sourceIcon: getSecretManagerProvider(manager), + } + : secret + ) + ) + return + } const variableEditRequest = { key: name, value: reference, @@ -222,11 +237,12 @@ export function ExternalSecretsTab({ }) onEditSecret?.() }, - [editVariable, onEditSecret] + [editVariable, onEditSecret, secretManagers] ) const handleOpenAddSecret = useCallback( - (isFile: boolean) => { + (format: SecretFormat) => { + const isFile = format === 'file' posthog.capture('external-secret-add-clicked', { scope, is_file: isFile, @@ -237,6 +253,7 @@ export function ExternalSecretsTab({ secretSources={secretSources} defaultSource={secretSources[0]} isFile={isFile} + isJson={format === 'json'} scope={scope} onClose={closeModal} onSubmit={handleCreateSecret} @@ -260,6 +277,7 @@ export function ExternalSecretsTab({ secretSources={secretSources} defaultSource={getSourceOption(secret)} isFile={secret.isFile ?? false} + isJson={secret.isJson} initialSecret={secret} onClose={closeModal} onSubmit={(updated) => handleEditSecret(secret.id, updated)} @@ -284,8 +302,12 @@ export function ExternalSecretsTab({ const handleDeleteSecrets = useCallback( async (secretIds: string[]) => { const idsToDelete = new Set(secretIds) - await Promise.all([...idsToDelete].map((variableId) => deleteVariable({ variableId }))) - onDeleteSecret?.() + // TODO(secret-manager-json): Replace local deletion with the real JSON API handler. + const draftIds = new Set(jsonDrafts.map((secret) => secret.id)) + const persistedIds = [...idsToDelete].filter((id) => !draftIds.has(id)) + await Promise.all(persistedIds.map((variableId) => deleteVariable({ variableId }))) + setJsonDrafts((current) => current.filter((secret) => !idsToDelete.has(secret.id))) + if (persistedIds.length > 0) onDeleteSecret?.() setRowSelection((prev) => { const next = { ...prev } for (const secretId of secretIds) { @@ -294,7 +316,7 @@ export function ExternalSecretsTab({ return next }) }, - [deleteVariable, onDeleteSecret] + [deleteVariable, onDeleteSecret, jsonDrafts] ) const handleConfirmDeleteSecrets = useCallback( @@ -403,6 +425,12 @@ export function ExternalSecretsTab({ )}
+ {secret.isJson && ( +
+ + {`Key: ${secret.keyName}`} +
+ )} {showFilePath && (
@@ -515,10 +543,11 @@ export function ExternalSecretsTab({ const shouldShowEmptyState = secrets.length === 0 && Boolean(emptyStateConfig) const totalRows = table.getPreFilteredRowModel().rows.length - const isSearching = table.getRowCount() !== totalRows - const countText = isSearching - ? `${table.getRowCount()}/${totalRows} external ${pluralize(table.getRowCount(), 'secret')}` - : `${totalRows} external ${pluralize(totalRows, 'secret')}` + const visibleRows = table.getRowCount() + const countText = + visibleRows !== totalRows + ? `${visibleRows}/${totalRows} external ${pluralize(totalRows, 'secret')}` + : `${totalRows} external ${pluralize(totalRows, 'secret')}` const selectedRows = table.getSelectedRowModel().rows.map(({ original }) => original) const selectedIds = selectedRows.map((secret) => secret.id) const hasSelection = selectedIds.length > 0 @@ -529,7 +558,7 @@ export function ExternalSecretsTab({
{countText} - + @@ -601,6 +630,13 @@ export function ExternalSecretsTab({ ))} + {table.getRowModel().rows.length === 0 && ( + + + No secrets match your search or filters. + + + )} {table.getRowModel().rows.map((row) => (