From af95dffa767d3c1e32219578745e2197991f58be Mon Sep 17 00:00:00 2001 From: Joao Pedro Poloni Ponce Date: Wed, 23 Oct 2024 16:51:20 -0300 Subject: [PATCH] fix: list existing secret in build time secret modal feat: create only non existing build time secrets feat: alternative solution for list existing secret test: add missging tests fix: fix type errors test: add coverage to missing lines chore: remove empty array test: add tests for submit-utils.ts feat: improve method to select secrets to create --- .../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 | 56 +++++++++++++------ src/components/Secrets/SecretModal.tsx | 6 +- .../Secrets/SecretModalLauncher.tsx | 2 +- .../Secrets/__tests___/SecretModal.spec.tsx | 32 ++++++++--- .../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, 216 insertions(+), 55 deletions(-) diff --git a/src/components/ImportForm/SecretSection/SecretSection.tsx b/src/components/ImportForm/SecretSection/SecretSection.tsx index a8988bd50..041d445fc 100644 --- a/src/components/ImportForm/SecretSection/SecretSection.tsx +++ b/src/components/ImportForm/SecretSection/SecretSection.tsx @@ -2,16 +2,17 @@ import React from 'react'; import { TextInputTypes, GridItem, Grid, FormSection } from '@patternfly/react-core'; import { PlusCircleIcon } from '@patternfly/react-icons/dist/js/icons/plus-circle-icon'; import { useFormikContext } from 'formik'; +import { Base64 } from 'js-base64'; import { useSecrets } from '../../../hooks/useSecrets'; import { SecretModel } from '../../../models'; import { InputField, TextColumnField } from '../../../shared'; +import { ExistingSecret, SecretType } from '../../../types'; import { AccessReviewResources } from '../../../types/rbac'; import { useAccessReviewForModels } from '../../../utils/rbac'; import { useWorkspaceInfo } from '../../../utils/workspace-context-utils'; import { ButtonWithAccessTooltip } from '../../ButtonWithAccessTooltip'; import { useModalLauncher } from '../../modal/ModalProvider'; import { SecretModalLauncher } from '../../Secrets/SecretModalLauncher'; -import { getSupportedPartnerTaskSecrets } from '../../Secrets/utils/secret-utils'; import { ImportFormValues } from '../type'; const accessReviewResources: AccessReviewResources = [{ model: SecretModel, verb: 'create' }]; @@ -24,12 +25,20 @@ const SecretSection = () => { const [secrets, secretsLoaded] = useSecrets(namespace); - 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 970234254..d0092e195 100644 --- a/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx +++ b/src/components/ImportForm/SecretSection/__tests__/SecretSection.spec.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import '@testing-library/jest-dom'; import { useK8sWatchResource } from '@openshift/dynamic-plugin-sdk-utils'; import { screen, fireEvent, act, waitFor } from '@testing-library/react'; +import { useSecrets } from '../../../../hooks/useSecrets'; import { useAccessReviewForModels } from '../../../../utils/rbac'; import { formikRenderer } from '../../../../utils/test-utils'; import SecretSection from '../SecretSection'; @@ -15,13 +16,51 @@ jest.mock('../../../../utils/rbac', () => ({ useAccessReviewForModels: jest.fn(), })); +jest.mock('../../../../hooks/useSecrets', () => ({ + useSecrets: jest.fn(), +})); + const watchResourceMock = useK8sWatchResource as jest.Mock; 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, 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 secret section', () => { diff --git a/src/components/ImportForm/__tests__/submit-utils.spec.ts b/src/components/ImportForm/__tests__/submit-utils.spec.ts index 1835281ed..395de71de 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, @@ -79,7 +80,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( @@ -95,6 +96,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 b5729c1ec..b7c88c39a 100644 --- a/src/components/ImportForm/submit-utils.ts +++ b/src/components/ImportForm/submit-utils.ts @@ -93,7 +93,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 }, @@ -113,7 +116,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 9161c9525..eb21518f3 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 1024f6e4f..0bac50786 100644 --- a/src/components/Secrets/SecretForm.tsx +++ b/src/components/Secrets/SecretForm.tsx @@ -1,32 +1,57 @@ -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, SelectInputField } from '../../shared'; import KeyValueFileInputField from '../../shared/components/formik-fields/key-value-file-input-field/KeyValueFileInputField'; -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); @@ -34,7 +59,7 @@ const SecretForm: React.FC> = ({ existi }; const resetKeyValues = () => { - setOptions([]); + options = []; const newKeyValues = values.keyValues.filter( (kv) => !kv.readOnlyKey && (!!kv.key || !!kv.value), ); @@ -54,14 +79,13 @@ const SecretForm: React.FC> = ({ existi { - currentTypeRef.current = type; + setType(type); if (type === SecretTypeDropdownLabel.image) { resetKeyValues(); values.secretName && - isPartnerTask(values.secretName) && + isPartnerTask(values.secretName, optionsValues) && setFieldValue('secretName', ''); } else { - setOptions(initialOptions); clearKeyValues(); } }} @@ -80,15 +104,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={(e, value) => { - if (isPartnerTask(value)) { + if (isPartnerTask(value, optionsValues)) { setFieldValue('keyValues', [ ...values.keyValues.filter((kv) => !kv.readOnlyKey && (!!kv.key || !!kv.value)), - ...getSupportedPartnerTaskKeyValuePairs(value), + ...getSupportedPartnerTaskKeyValuePairs(value, optionsValues), ]); } setFieldValue('secretName', value); diff --git a/src/components/Secrets/SecretModal.tsx b/src/components/Secrets/SecretModal.tsx index 4640e32b9..93387ff39 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 3277aadc9..eacfaad12 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?: any, 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 7ddcf7587..fc4108c80 100644 --- a/src/components/Secrets/__tests___/SecretModal.spec.tsx +++ b/src/components/Secrets/__tests___/SecretModal.spec.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import '@testing-library/jest-dom'; 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'; @@ -13,11 +13,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( , @@ -32,7 +48,7 @@ describe('SecretForm', () => { it('should render validation message when user click on create button without filling the form', async () => { formikRenderer( , @@ -50,7 +66,7 @@ describe('SecretForm', () => { formikRenderer( , initialValues, @@ -69,7 +85,7 @@ describe('SecretForm', () => { formikRenderer( , initialValues, @@ -88,12 +104,12 @@ describe('SecretForm', () => { }); }); - it('should not show the secrets in the select dropdown if it is already existing', async () => { + it('should show the secrets in the select dropdown if it is already existing', async () => { const onClose = jest.fn(); formikRenderer( , initialValues, @@ -104,7 +120,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 b00146ed1..516036865 100644 --- a/src/components/Secrets/__tests___/secret-utils.spec.ts +++ b/src/components/Secrets/__tests___/secret-utils.spec.ts @@ -42,7 +42,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 2370ef0b2..a934c9197 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 486f93707..908c3de10 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 c8544fd5b..8d6ff2ee6 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(