From f73cf3a1647112d70597da54ca1ee087e83fa12c Mon Sep 17 00:00:00 2001 From: Joao Pedro Poloni Ponce Date: Mon, 6 Jan 2025 16:54:46 -0300 Subject: [PATCH] fix: list existing secret in build time secret modal --- .../SecretSection/SecretSection.tsx | 21 +++++-- .../__tests__/SecretSection.spec.tsx | 39 +++++++++++++ .../ImportForm/__tests__/submit-utils.spec.ts | 53 +++++++++++++++++- src/components/ImportForm/submit-utils.ts | 7 ++- src/components/ImportForm/type.ts | 4 +- src/components/Secrets/SecretForm.tsx | 55 ++++++++++++++----- src/components/Secrets/SecretModal.tsx | 6 +- .../Secrets/SecretModalLauncher.tsx | 2 +- .../Secrets/__tests___/SecretModal.spec.tsx | 30 +++++++--- .../Secrets/__tests___/secret-utils.spec.ts | 2 +- src/components/Secrets/utils/secret-utils.ts | 30 ++++++---- src/types/secret.ts | 15 ++++- src/utils/validation-utils.ts | 4 +- 13 files changed, 215 insertions(+), 53 deletions(-) diff --git a/src/components/ImportForm/SecretSection/SecretSection.tsx b/src/components/ImportForm/SecretSection/SecretSection.tsx index 7d449967..30144d59 100644 --- a/src/components/ImportForm/SecretSection/SecretSection.tsx +++ b/src/components/ImportForm/SecretSection/SecretSection.tsx @@ -3,15 +3,16 @@ import { TextInputTypes, GridItem, Grid, FormSection } from '@patternfly/react-c import { PlusCircleIcon } from '@patternfly/react-icons/dist/js/icons/plus-circle-icon'; import { useFormikContext } from 'formik'; import { InputField } from 'formik-pf'; +import { Base64 } from 'js-base64'; import { useSecrets } from '../../../hooks/useSecrets'; import { SecretModel } from '../../../models'; import TextColumnField from '../../../shared/components/formik-fields/text-column-field/TextColumnField'; +import { ExistingSecret, SecretType } from '../../../types'; import { AccessReviewResources } from '../../../types/rbac'; import { useAccessReviewForModels } from '../../../utils/rbac'; import { ButtonWithAccessTooltip } from '../../ButtonWithAccessTooltip'; import { useModalLauncher } from '../../modal/ModalProvider'; import { SecretModalLauncher } from '../../Secrets/SecretModalLauncher'; -import { getSupportedPartnerTaskSecrets } from '../../Secrets/utils/secret-utils'; import { useWorkspaceInfo } from '../../Workspace/useWorkspaceInfo'; import { ImportFormValues } from '../type'; @@ -25,12 +26,20 @@ const SecretSection = () => { const [secrets, secretsLoaded] = useSecrets(namespace, workspace); - const partnerTaskNames = getSupportedPartnerTaskSecrets().map(({ label }) => label); - const partnerTaskSecrets: string[] = + const partnerTaskSecrets: ExistingSecret[] = secrets && secretsLoaded - ? secrets - ?.filter((rs) => partnerTaskNames.includes(rs.metadata.name)) - ?.map((s) => s.metadata.name) || [] + ? secrets?.map((secret) => ({ + type: secret.type as SecretType, + name: secret.metadata.name, + providerUrl: '', + tokenKeyName: secret.metadata.name, + keyValuePairs: Object.keys(secret.data).map((key) => ({ + key, + value: Base64.decode(secret.data[key]), + readOnlyKey: true, + readOnlyValue: true, + })), + })) : []; const onSubmit = React.useCallback( diff --git a/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx b/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx index 1817cef6..2ee34313 100644 --- a/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx +++ b/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx @@ -1,4 +1,5 @@ import { screen, fireEvent, act, waitFor } from '@testing-library/react'; +import { useSecrets } from '../../../../hooks/useSecrets'; import { useAccessReviewForModels } from '../../../../utils/rbac'; import { createK8sWatchResourceMock, formikRenderer } from '../../../../utils/test-utils'; import SecretSection from '../SecretSection'; @@ -7,13 +8,35 @@ jest.mock('../../../../utils/rbac', () => ({ useAccessReviewForModels: jest.fn(), })); +jest.mock('../../../../hooks/useSecrets', () => ({ + useSecrets: jest.fn(), +})); + const watchResourceMock = createK8sWatchResourceMock(); const accessReviewMock = useAccessReviewForModels as jest.Mock; +const useSecretsMock = useSecrets as jest.Mock; describe('SecretSection', () => { beforeEach(() => { watchResourceMock.mockReturnValue([[], true]); accessReviewMock.mockReturnValue([true, true]); + useSecretsMock.mockReturnValue([ + [ + { + metadata: { + name: 'snyk-secret', + namespace: 'test-ws', + }, + data: { + 'snyk-token': 'c255ay1zZWNyZXQ=', + }, + type: 'Opaque', + apiVersion: 'v1', + kind: 'Secret', + }, + ], + true, + ]); }); it('should render secret section', () => { @@ -23,6 +46,22 @@ describe('SecretSection', () => { screen.getByTestId('add-secret-button'); }); + it('should render secret section, secret do not load yet', () => { + useSecretsMock.mockReturnValue([[], false]); + formikRenderer(, {}); + + screen.getByText('Build time secret'); + screen.getByTestId('add-secret-button'); + }); + + it('should render secret section with empty list of secrets', () => { + useSecretsMock.mockReturnValue([[], true]); + formikRenderer(, {}); + + screen.getByText('Build time secret'); + screen.getByTestId('add-secret-button'); + }); + it('should render added secrets in removable lists', () => { formikRenderer(, { newSecrets: ['secret-one', 'secret-two'] }); diff --git a/src/components/ImportForm/__tests__/submit-utils.spec.ts b/src/components/ImportForm/__tests__/submit-utils.spec.ts index 6e2a21e7..582dc396 100644 --- a/src/components/ImportForm/__tests__/submit-utils.spec.ts +++ b/src/components/ImportForm/__tests__/submit-utils.spec.ts @@ -1,3 +1,4 @@ +import { SecretType } from '../../../types'; import { createApplication, createComponent, @@ -77,7 +78,7 @@ describe('Submit Utils: createResources', () => { expect(createImageRepositoryMock).toHaveBeenCalledTimes(0); }); - it('should not create application but create components', async () => { + it('should not create application but create components without secrets', async () => { createApplicationMock.mockResolvedValue({ metadata: { name: 'test-app' } }); createComponentMock.mockResolvedValue({ metadata: { name: 'test-component' } }); await createResources( @@ -93,6 +94,56 @@ describe('Submit Utils: createResources', () => { }, pipeline: 'dbcd', componentName: 'component', + importSecrets: [ + { + existingSecrets: [ + { + name: 'secret', + type: SecretType.opaque, + providerUrl: '', + tokenKeyName: 'secret', + keyValuePairs: [ + { + key: 'secret', + value: 'value', + readOnlyKey: true, + }, + ], + }, + ], + type: 'Opaque', + secretName: 'secret', + keyValues: [{ key: 'secret', value: 'test-value', readOnlyKey: true }], + }, + ], + }, + 'test-ws-tenant', + 'test-ws', + 'url.bombino', + ); + expect(createApplicationMock).toHaveBeenCalledTimes(0); + expect(createIntegrationTestMock).toHaveBeenCalledTimes(0); + expect(createComponentMock).toHaveBeenCalledTimes(2); + expect(createImageRepositoryMock).toHaveBeenCalledTimes(2); + }); + + it('should not create application, create components and secret', async () => { + createApplicationMock.mockResolvedValue({ metadata: { name: 'test-app' } }); + createComponentMock.mockResolvedValue({ metadata: { name: 'test-component' } }); + await createResources( + { + application: 'test-app', + inAppContext: true, + showComponent: true, + isPrivateRepo: false, + source: { + git: { + url: 'https://github.com/', + }, + }, + pipeline: 'dbcd', + componentName: 'component', + importSecrets: [], }, 'test-ws-tenant', 'test-ws', diff --git a/src/components/ImportForm/submit-utils.ts b/src/components/ImportForm/submit-utils.ts index 3648e4f0..1c9fb1c1 100644 --- a/src/components/ImportForm/submit-utils.ts +++ b/src/components/ImportForm/submit-utils.ts @@ -101,7 +101,10 @@ export const createResources = async ( let createdComponent; if (showComponent) { - await createSecrets(importSecrets, workspace, namespace, true); + const secretsToCreate = importSecrets.filter((secret) => + secret.existingSecrets.find((existing) => secret.secretName === existing.name) ? false : true, + ); + await createSecrets(secretsToCreate, workspace, namespace, true); createdComponent = await createComponent( { componentName, application, gitProviderAnnotation, source, gitURLAnnotation }, @@ -123,7 +126,7 @@ export const createResources = async ( isPrivate: isPrivateRepo, bombinoUrl, }); - await createSecrets(importSecrets, workspace, namespace, false); + await createSecrets(secretsToCreate, workspace, namespace, false); } return { diff --git a/src/components/ImportForm/type.ts b/src/components/ImportForm/type.ts index 9161c952..eb21518f 100644 --- a/src/components/ImportForm/type.ts +++ b/src/components/ImportForm/type.ts @@ -1,4 +1,4 @@ -import { ImportSecret } from '../../types'; +import { SecretFormValues } from '../../types'; export type ImportFormValues = { application: string; @@ -17,6 +17,6 @@ export type ImportFormValues = { }; }; pipeline: string; - importSecrets?: ImportSecret[]; + importSecrets?: SecretFormValues[]; newSecrets?: string[]; }; diff --git a/src/components/Secrets/SecretForm.tsx b/src/components/Secrets/SecretForm.tsx index 543ae307..8630b011 100644 --- a/src/components/Secrets/SecretForm.tsx +++ b/src/components/Secrets/SecretForm.tsx @@ -1,33 +1,58 @@ -import React from 'react'; +import React, { useMemo } from 'react'; import { Form } from '@patternfly/react-core'; import { SelectVariant } from '@patternfly/react-core/deprecated'; import { useFormikContext } from 'formik'; import { DropdownItemObject } from '../../shared/components/dropdown'; import KeyValueFileInputField from '../../shared/components/formik-fields/key-value-file-input-field/KeyValueFileInputField'; import SelectInputField from '../../shared/components/formik-fields/SelectInputField'; -import { SecretFormValues, SecretTypeDropdownLabel } from '../../types'; +import { + SecretFormValues, + SecretTypeDropdownLabel, + K8sSecretType, + ExistingSecret, +} from '../../types'; import { RawComponentProps } from '../modal/createModalLauncher'; import SecretTypeSelector from './SecretTypeSelector'; import { + supportedPartnerTasksSecrets, getSupportedPartnerTaskKeyValuePairs, isPartnerTask, - getSupportedPartnerTaskSecrets, } from './utils/secret-utils'; type SecretFormProps = RawComponentProps & { - existingSecrets: string[]; + existingSecrets: ExistingSecret[]; }; const SecretForm: React.FC> = ({ existingSecrets }) => { const { values, setFieldValue } = useFormikContext(); + const [currentType, setType] = React.useState(values.type); const defaultKeyValues = [{ key: '', value: '', readOnlyKey: false }]; const defaultImageKeyValues = [{ key: '.dockerconfigjson', value: '', readOnlyKey: true }]; - const initialOptions = getSupportedPartnerTaskSecrets().filter( - (secret) => !existingSecrets.includes(secret.value), - ); - const [options, setOptions] = React.useState(initialOptions); - const currentTypeRef = React.useRef(values.type); + let options = useMemo(() => { + return existingSecrets + .filter((secret) => secret.type === K8sSecretType[currentType]) + .concat( + currentType === SecretTypeDropdownLabel.opaque && + existingSecrets.find((s) => s.name === 'snyk-secret') === undefined + ? [supportedPartnerTasksSecrets.snyk] + : [], + ) + .filter((secret) => secret.type !== K8sSecretType[SecretTypeDropdownLabel.image]) + .map((secret) => ({ value: secret.name, lable: secret.name })); + }, [currentType, existingSecrets]); + const optionsValues = useMemo(() => { + return existingSecrets + .filter((secret) => secret.type === K8sSecretType[currentType]) + .filter((secret) => secret.type !== K8sSecretType[SecretTypeDropdownLabel.image]) + .reduce( + (dictOfSecrets, secret) => { + dictOfSecrets[secret.name] = secret; + return dictOfSecrets; + }, + { 'snyk-secret': supportedPartnerTasksSecrets.snyk }, + ); + }, [currentType, existingSecrets]); const clearKeyValues = () => { const newKeyValues = values.keyValues.filter((kv) => !kv.readOnlyKey); @@ -35,7 +60,7 @@ const SecretForm: React.FC> = ({ existi }; const resetKeyValues = () => { - setOptions([]); + options = []; const newKeyValues = values.keyValues.filter( (kv) => !kv.readOnlyKey && (!!kv.key || !!kv.value), ); @@ -55,11 +80,11 @@ const SecretForm: React.FC> = ({ existi { - currentTypeRef.current = type; + setType(type); if (type === SecretTypeDropdownLabel.image) { resetKeyValues(); values.secretName && - isPartnerTask(values.secretName) && + isPartnerTask(values.secretName, optionsValues) && void setFieldValue('secretName', ''); } else { setOptions(initialOptions); @@ -81,15 +106,15 @@ const SecretForm: React.FC> = ({ existi toggleId="secret-name-toggle" toggleAriaLabel="secret-name-dropdown" onClear={() => { - if (currentTypeRef.current !== values.type || isPartnerTask(values.secretName)) { + if (currentType !== values.type || isPartnerTask(values.secretName, optionsValues)) { clearKeyValues(); } }} onSelect={(_, value: string) => { - if (isPartnerTask(value)) { + if (isPartnerTask(value, optionsValues)) { void setFieldValue('keyValues', [ ...values.keyValues.filter((kv) => !kv.readOnlyKey && (!!kv.key || !!kv.value)), - ...getSupportedPartnerTaskKeyValuePairs(value), + ...getSupportedPartnerTaskKeyValuePairs(value, optionsValues), ]); } void setFieldValue('secretName', value); diff --git a/src/components/Secrets/SecretModal.tsx b/src/components/Secrets/SecretModal.tsx index 0ac79dfc..4a24d713 100644 --- a/src/components/Secrets/SecretModal.tsx +++ b/src/components/Secrets/SecretModal.tsx @@ -8,7 +8,7 @@ import { ModalVariant, } from '@patternfly/react-core'; import { Formik } from 'formik'; -import { ImportSecret, SecretTypeDropdownLabel } from '../../types'; +import { ImportSecret, SecretTypeDropdownLabel, ExistingSecret } from '../../types'; import { SecretFromSchema } from '../../utils/validation-utils'; import { RawComponentProps } from '../modal/createModalLauncher'; import SecretForm from './SecretForm'; @@ -25,11 +25,11 @@ const createPartnerTaskSecret = ( }; export type SecretModalValues = ImportSecret & { - existingSecrets: string[]; + existingSecrets: ExistingSecret[]; }; type SecretModalProps = RawComponentProps & { - existingSecrets: string[]; + existingSecrets: ExistingSecret[]; onSubmit: (value: SecretModalValues) => void; }; diff --git a/src/components/Secrets/SecretModalLauncher.tsx b/src/components/Secrets/SecretModalLauncher.tsx index 84ca8428..81173b65 100644 --- a/src/components/Secrets/SecretModalLauncher.tsx +++ b/src/components/Secrets/SecretModalLauncher.tsx @@ -4,7 +4,7 @@ import { createRawModalLauncher } from '../modal/createModalLauncher'; import SecretForm from './SecretModal'; export const SecretModalLauncher = ( - existingSecrets?: string[], + existingSecrets?: ExistingSecret[], onSubmit?: (values: SecretFormValues) => void, onClose?: () => void, ) => diff --git a/src/components/Secrets/__tests___/SecretModal.spec.tsx b/src/components/Secrets/__tests___/SecretModal.spec.tsx index 5bf9564a..ce770636 100644 --- a/src/components/Secrets/__tests___/SecretModal.spec.tsx +++ b/src/components/Secrets/__tests___/SecretModal.spec.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, screen, waitFor } from '@testing-library/react'; -import { SecretTypeDropdownLabel } from '../../../types'; +import { SecretTypeDropdownLabel, SecretType } from '../../../types'; import { formikRenderer } from '../../../utils/test-utils'; import SecretModal, { SecretModalValues } from '../SecretModal'; import { supportedPartnerTasksSecrets } from '../utils/secret-utils'; @@ -11,11 +11,27 @@ const initialValues: SecretModalValues = { existingSecrets: [], }; +const snykSecret = { + type: SecretType.opaque, + name: 'snyk-secret', + tokenKeyName: 'snyk-secret', + providerUrl: '', + keyValuePairs: [{ key: 'snyk_token', value: 'snyk_value', readOnlyKey: true }], +}; + +const testSecret = { + type: SecretType.opaque, + name: 'test-secret', + tokenKeyName: 'test-secret', + providerUrl: '', + keyValuePairs: [{ key: 'test_token', value: 'test_value', readOnlyKey: true }], +}; + describe('SecretForm', () => { it('should show secret form in a modal', async () => { formikRenderer( , @@ -30,7 +46,7 @@ describe('SecretForm', () => { it('should render validation message when user click on create button without filling the form', async () => { formikRenderer( , @@ -48,7 +64,7 @@ describe('SecretForm', () => { formikRenderer( , initialValues, @@ -67,7 +83,7 @@ describe('SecretForm', () => { formikRenderer( , initialValues, @@ -91,7 +107,7 @@ describe('SecretForm', () => { formikRenderer( , initialValues, @@ -102,7 +118,7 @@ describe('SecretForm', () => { const modal = screen.queryByTestId('build-secret-modal'); fireEvent.click(modal.querySelector('#secret-name-toggle-select-typeahead')); }); - expect(screen.queryByText('snyk-secret')).not.toBeInTheDocument(); + expect(screen.queryByText('snyk-secret')).toBeInTheDocument(); }); it('should remove the selected value with clearn button is clicked', async () => { diff --git a/src/components/Secrets/__tests___/secret-utils.spec.ts b/src/components/Secrets/__tests___/secret-utils.spec.ts index 920d39db..533382cd 100644 --- a/src/components/Secrets/__tests___/secret-utils.spec.ts +++ b/src/components/Secrets/__tests___/secret-utils.spec.ts @@ -39,7 +39,7 @@ describe('getSupportedPartnerTaskKeyValuePairs', () => { it('should return snyk secret values ', () => { expect(getSupportedPartnerTaskKeyValuePairs('snyk-secret')).toEqual([ - { key: 'snyk_token', readOnlyKey: true, value: '' }, + { key: 'snyk_token', readOnlyKey: true, value: '', readOnlyValue: false }, ]); }); }); diff --git a/src/components/Secrets/utils/secret-utils.ts b/src/components/Secrets/utils/secret-utils.ts index 2f7b8ac8..16de4f7a 100644 --- a/src/components/Secrets/utils/secret-utils.ts +++ b/src/components/Secrets/utils/secret-utils.ts @@ -16,6 +16,7 @@ import { SecretTypeDisplayLabel, SecretTypeDropdownLabel, SourceSecretType, + ExistingSecret, } from '../../../types'; export type PartnerTask = { @@ -27,16 +28,17 @@ export type PartnerTask = { key: string; value: string; readOnlyKey?: boolean; + readOnlyValue?: boolean; }[]; }; -export const supportedPartnerTasksSecrets: { [key: string]: PartnerTask } = { +export const supportedPartnerTasksSecrets: { [key: string]: ExistingSecret } = { snyk: { type: SecretType.opaque, name: 'snyk-secret', providerUrl: 'https://snyk.io/', tokenKeyName: 'snyk_token', - keyValuePairs: [{ key: 'snyk_token', value: '', readOnlyKey: true }], + keyValuePairs: [{ key: 'snyk_token', value: '', readOnlyKey: true, readOnlyValue: false }], }, }; @@ -46,19 +48,23 @@ export const getSupportedPartnerTaskSecrets = () => { value: secret.name, })); }; -export const isPartnerTaskAvailable = (type: string) => - !!Object.values(supportedPartnerTasksSecrets).find( - (secret) => secret.type === K8sSecretType[type], - ); +export const isPartnerTaskAvailable = ( + type: string, + arr: { [key: string]: ExistingSecret } = supportedPartnerTasksSecrets, +) => !!Object.values(arr).find((secret) => secret.type === K8sSecretType[type]); -export const isPartnerTask = (secretName: string) => { - return !!Object.values(supportedPartnerTasksSecrets).find((secret) => secret.name === secretName); +export const isPartnerTask = ( + secretName: string, + arr: { [key: string]: ExistingSecret } = supportedPartnerTasksSecrets, +) => { + return !!Object.values(arr).find((secret) => secret.name === secretName); }; -export const getSupportedPartnerTaskKeyValuePairs = (secretName?: string) => { - const partnerTask = Object.values(supportedPartnerTasksSecrets).find( - (secret) => secret.name === secretName, - ); +export const getSupportedPartnerTaskKeyValuePairs = ( + secretName?: string, + arr: { [key: string]: ExistingSecret } = supportedPartnerTasksSecrets, +) => { + const partnerTask = Object.values(arr).find((secret) => secret.name === secretName); return partnerTask ? partnerTask.keyValuePairs : []; }; diff --git a/src/types/secret.ts b/src/types/secret.ts index bd10adc5..8fbfa000 100644 --- a/src/types/secret.ts +++ b/src/types/secret.ts @@ -106,8 +106,21 @@ export interface Target { secretName: string; } +export type ExistingSecret = { + type: SecretType; + name: string; + providerUrl: string; + tokenKeyName: string; + keyValuePairs: { + key: string; + value: string; + readOnlyKey?: boolean; + readOnlyValue?: boolean; + }[]; +}; + export type SecretFormValues = ImportSecret & { - existingSecrets?: string[]; + existingSecrets?: ExistingSecret[]; }; export enum SecretTypeDropdownLabel { diff --git a/src/utils/validation-utils.ts b/src/utils/validation-utils.ts index c8544fd5..8d6ff2ee 100644 --- a/src/utils/validation-utils.ts +++ b/src/utils/validation-utils.ts @@ -24,8 +24,8 @@ export const SecretFromSchema = yup.object({ secretName: resourceNameYupValidation.test( 'existing-secret-test', 'Secret already exists', - (value, { parent: { existingSecrets } }) => { - return !existingSecrets.includes(value); + (value) => { + return value !== undefined; }, ), keyValues: yup.array().of(