From 6fe4278d094aea2ca850491a0e0146c6d8e0e8ae Mon Sep 17 00:00:00 2001 From: Banks Nussman <115251059+bnussman-akamai@users.noreply.github.com> Date: Thu, 18 Jan 2024 10:06:23 -0500 Subject: [PATCH] upcoming: [M3-7602] - Improve AGLB Configuration - Add Certificate Drawer (revision 2) (#10066) * implement new UX * small fixes * update e2e test * add certifiate drawer loading state and cancel button * Added changeset: Improve AGLB Configuration - Add Certificate Drawer * move components to their own files and add error handling * sort props --------- Co-authored-by: Banks Nussman --- ...r-10066-upcoming-features-1705422066984.md | 5 + .../load-balancer-configurations.spec.ts | 24 ++-- .../Certificates/CreateCertificateDrawer.tsx | 5 + .../Configurations/AddCertificateDrawer.tsx | 57 ++++++++ .../AddExistingCertificateForm.tsx | 71 ++++++++++ .../Configurations/AddNewCertificateForm.tsx | 133 ++++++++++++++++++ .../ApplyCertificatesDrawer.tsx | 111 --------------- .../Configurations/ConfigurationForm.tsx | 32 +++-- .../Routes/AddRouteDrawer.tsx | 2 +- .../manager/src/queries/aglb/certificates.ts | 12 +- .../validation/src/loadbalancers.schema.ts | 6 +- 11 files changed, 311 insertions(+), 147 deletions(-) create mode 100644 packages/manager/.changeset/pr-10066-upcoming-features-1705422066984.md create mode 100644 packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddCertificateDrawer.tsx create mode 100644 packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddExistingCertificateForm.tsx create mode 100644 packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddNewCertificateForm.tsx delete mode 100644 packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ApplyCertificatesDrawer.tsx diff --git a/packages/manager/.changeset/pr-10066-upcoming-features-1705422066984.md b/packages/manager/.changeset/pr-10066-upcoming-features-1705422066984.md new file mode 100644 index 00000000000..e0c5d7fa469 --- /dev/null +++ b/packages/manager/.changeset/pr-10066-upcoming-features-1705422066984.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Improve AGLB Configuration - Add Certificate Drawer ([#10066](https://github.com/linode/manager/pull/10066)) diff --git a/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-configurations.spec.ts b/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-configurations.spec.ts index f8162221933..878ce09886f 100644 --- a/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-configurations.spec.ts +++ b/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-configurations.spec.ts @@ -85,9 +85,11 @@ describe('Akamai Global Load Balancer configurations page', () => { .should('be.visible') .type(configuration.label); - ui.button.findByTitle('Apply Certificates').should('be.visible').click(); + ui.button.findByTitle('Add Certificate').should('be.visible').click(); + + ui.drawer.findByTitle('Add Certificate').within(() => { + cy.findByLabelText('Add Existing Certificate').click(); - ui.drawer.findByTitle('Apply Certificates').within(() => { cy.findByLabelText('Host Header').should('be.visible').type('*'); cy.findByLabelText('Certificate').should('be.visible').click(); @@ -98,7 +100,7 @@ describe('Akamai Global Load Balancer configurations page', () => { .click(); ui.button - .findByTitle('Save') + .findByTitle('Add') .should('be.visible') .should('be.enabled') .click(); @@ -312,9 +314,11 @@ describe('Akamai Global Load Balancer configurations page', () => { .should('be.visible') .type('test'); - ui.button.findByTitle('Apply Certificates').should('be.visible').click(); + ui.button.findByTitle('Add Certificate').should('be.visible').click(); + + ui.drawer.findByTitle('Add Certificate').within(() => { + cy.findByLabelText('Add Existing Certificate').click(); - ui.drawer.findByTitle('Apply Certificates').within(() => { cy.findByLabelText('Host Header').should('be.visible').type('*'); cy.findByLabelText('Certificate').should('be.visible').click(); @@ -325,7 +329,7 @@ describe('Akamai Global Load Balancer configurations page', () => { .click(); ui.button - .findByTitle('Save') + .findByTitle('Add') .should('be.visible') .should('be.enabled') .click(); @@ -412,12 +416,14 @@ describe('Akamai Global Load Balancer configurations page', () => { cy.findByLabelText('Port').should('be.visible').clear().type('444'); ui.button - .findByTitle('Apply More Certificates') + .findByTitle('Add Certificate') .should('be.visible') .should('be.enabled') .click(); - ui.drawer.findByTitle('Apply Certificates').within(() => { + ui.drawer.findByTitle('Add Certificate').within(() => { + cy.findByLabelText('Add Existing Certificate').click(); + cy.findByLabelText('Host Header').type('example-1.com'); cy.findByLabelText('Certificate').click(); @@ -425,7 +431,7 @@ describe('Akamai Global Load Balancer configurations page', () => { ui.autocompletePopper.findByTitle(certificates[1].label).click(); ui.button - .findByTitle('Save') + .findByTitle('Add') .should('be.visible') .should('be.enabled') .click(); diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.tsx index a1e04d70220..4d620de9320 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.tsx @@ -106,8 +106,13 @@ export const CreateCertificateDrawer = (props: Props) => { primaryButtonProps={{ 'data-testid': 'submit', label: 'Upload Certificate', + loading: formik.isSubmitting, type: 'submit', }} + secondaryButtonProps={{ + label: 'Cancel', + onClick: onClose, + }} /> diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddCertificateDrawer.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddCertificateDrawer.tsx new file mode 100644 index 00000000000..4eab8f428cc --- /dev/null +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddCertificateDrawer.tsx @@ -0,0 +1,57 @@ +import React, { useState } from 'react'; + +import { Code } from 'src/components/Code/Code'; +import { Drawer } from 'src/components/Drawer'; +import { FormControlLabel } from 'src/components/FormControlLabel'; +import { Link } from 'src/components/Link'; +import { Radio } from 'src/components/Radio/Radio'; +import { RadioGroup } from 'src/components/RadioGroup'; +import { Typography } from 'src/components/Typography'; + +import { AddExistingCertificateForm } from './AddExistingCertificateForm'; +import { AddNewCertificateForm } from './AddNewCertificateForm'; + +import type { CertificateConfig } from '@linode/api-v4'; + +export interface AddCertificateDrawerProps { + loadbalancerId: number; + onAdd: (certificates: CertificateConfig) => void; + onClose: () => void; + open: boolean; +} + +type Mode = 'existing' | 'new'; + +export const AddCertificateDrawer = (props: AddCertificateDrawerProps) => { + const { onClose, open } = props; + + const [mode, setMode] = useState('new'); + + return ( + + {/* @TODO Add AGLB docs link - M3-7041 */} + + Input the host header that the Load Balancer will repsond to and the + respective certificate to deliver. Use * as a wildcard + apply to any host. Learn more. + + setMode(value as Mode)} value={mode}> + } + label="Create New Certificate" + value="new" + /> + } + label="Add Existing Certificate" + value="existing" + /> + + {mode === 'existing' ? ( + + ) : ( + + )} + + ); +}; diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddExistingCertificateForm.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddExistingCertificateForm.tsx new file mode 100644 index 00000000000..4c7878c1d1b --- /dev/null +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddExistingCertificateForm.tsx @@ -0,0 +1,71 @@ +import { CertificateEntrySchema } from '@linode/validation'; +import { useFormik } from 'formik'; +import React, { useEffect } from 'react'; + +import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; +import { TextField } from 'src/components/TextField'; + +import { CertificateSelect } from '../Certificates/CertificateSelect'; + +import type { AddCertificateDrawerProps } from './AddCertificateDrawer'; +import type { CertificateConfig } from '@linode/api-v4'; + +export const AddExistingCertificateForm = ( + props: AddCertificateDrawerProps +) => { + const { loadbalancerId, onAdd, onClose, open } = props; + + const formik = useFormik({ + initialValues: { + hostname: '', + id: -1, + }, + onSubmit(values) { + onAdd(values); + onClose(); + }, + validationSchema: CertificateEntrySchema, + }); + + useEffect(() => { + if (open) { + formik.resetForm(); + } + }, [open]); + + return ( +
+ + formik.setFieldValue('id', certificate?.id ?? null) + } + textFieldProps={{ + noMarginTop: true, + onBlur: () => formik.setFieldTouched('id'), + }} + errorText={formik.touched.id ? formik.errors.id : undefined} + filter={{ type: 'downstream' }} + loadbalancerId={loadbalancerId} + value={formik.values.id} + /> + + + + ); +}; diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddNewCertificateForm.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddNewCertificateForm.tsx new file mode 100644 index 00000000000..4747e1e4739 --- /dev/null +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/AddNewCertificateForm.tsx @@ -0,0 +1,133 @@ +import { + CertificateEntrySchema, + CreateCertificateSchema, +} from '@linode/validation'; +import { useFormik } from 'formik'; +import React, { useEffect } from 'react'; + +import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; +import { Notice } from 'src/components/Notice/Notice'; +import { TextField } from 'src/components/TextField'; +import { useLoadBalancerCertificateCreateMutation } from 'src/queries/aglb/certificates'; +import { getFormikErrorsFromAPIErrors } from 'src/utilities/formikErrorUtils'; + +import { + CERTIFICATES_COPY, + exampleCert, + exampleKey, +} from '../Certificates/constants'; + +import type { AddCertificateDrawerProps } from './AddCertificateDrawer'; +import type { + CertificateConfig, + CreateCertificatePayload, +} from '@linode/api-v4'; + +export const AddNewCertificateForm = (props: AddCertificateDrawerProps) => { + const { loadbalancerId, onAdd, onClose, open } = props; + + const { + mutateAsync: createCertificate, + error, + } = useLoadBalancerCertificateCreateMutation(loadbalancerId); + + const formik = useFormik< + CreateCertificatePayload & Omit + >({ + initialValues: { + certificate: '', + hostname: '', + key: '', + label: '', + type: 'downstream', + }, + async onSubmit({ certificate, hostname, key, label, type }, helpers) { + try { + const cert = await createCertificate({ + certificate, + key, + label, + type, + }); + + onAdd({ hostname, id: cert.id }); + onClose(); + } catch (error) { + helpers.setErrors(getFormikErrorsFromAPIErrors(error)); + } + }, + validationSchema: CertificateEntrySchema.omit(['id']).concat( + CreateCertificateSchema + ), + }); + + useEffect(() => { + if (open) { + formik.resetForm(); + } + }, [open]); + + const generalError = error?.[0].reason; + + return ( +
+ {generalError && } + + + + + + + ); +}; diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ApplyCertificatesDrawer.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ApplyCertificatesDrawer.tsx deleted file mode 100644 index c823a573672..00000000000 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ApplyCertificatesDrawer.tsx +++ /dev/null @@ -1,111 +0,0 @@ -import { Configuration } from '@linode/api-v4'; -import { CertificateConfigSchema } from '@linode/validation'; -import { useFormik } from 'formik'; -import React, { useEffect } from 'react'; - -import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; -import { Box } from 'src/components/Box'; -import { Button } from 'src/components/Button/Button'; -import { Code } from 'src/components/Code/Code'; -import { Divider } from 'src/components/Divider'; -import { Drawer } from 'src/components/Drawer'; -import { Link } from 'src/components/Link'; -import { TextField } from 'src/components/TextField'; -import { Typography } from 'src/components/Typography'; - -import { CertificateSelect } from '../Certificates/CertificateSelect'; - -interface Props { - loadbalancerId: number; - onAdd: (certificates: Configuration['certificates']) => void; - onClose: () => void; - open: boolean; -} - -const defaultCertItem = { - hostname: '', - id: -1, -}; - -export const ApplyCertificatesDrawer = (props: Props) => { - const { loadbalancerId, onAdd, onClose, open } = props; - - const formik = useFormik<{ certificates: Configuration['certificates'] }>({ - initialValues: { - certificates: [defaultCertItem], - }, - onSubmit(values) { - onAdd(values.certificates); - onClose(); - }, - validateOnChange: false, - validationSchema: CertificateConfigSchema, - }); - - useEffect(() => { - if (open) { - formik.resetForm(); - } - }, [open]); - - const onAddAnother = () => { - formik.setFieldValue('certificates', [ - ...formik.values.certificates, - defaultCertItem, - ]); - }; - - return ( - - {/* @TODO Add AGLB docs link - M3-7041 */} - - Input the host header that the Load Balancer will repsond to and the - respective certificate to deliver. Use * as a wildcard - apply to any host. Learn more. - -
- {formik.values.certificates.map(({ hostname, id }, index) => ( - - - formik.setFieldValue( - `certificates.${index}.hostname`, - e.target.value - ) - } - errorText={formik.errors.certificates?.[index]?.['hostname']} - label="Host Header" - value={hostname} - /> - - formik.setFieldValue( - `certificates.${index}.id`, - certificate?.id ?? null - ) - } - errorText={formik.errors.certificates?.[index]?.['id']} - filter={{ type: 'downstream' }} - loadbalancerId={loadbalancerId} - value={id} - /> - - - ))} - - - -
- ); -}; diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationForm.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationForm.tsx index 77cc9a6fbea..843b6e4e3f4 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationForm.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationForm.tsx @@ -24,7 +24,7 @@ import { getFormikErrorsFromAPIErrors } from 'src/utilities/formikErrorUtils'; import { AddRouteDrawer } from '../Routes/AddRouteDrawer'; import { RoutesTable } from '../Routes/RoutesTable'; -import { ApplyCertificatesDrawer } from './ApplyCertificatesDrawer'; +import { AddCertificateDrawer } from './AddCertificateDrawer'; import { CertificateTable } from './CertificateTable'; import { DeleteConfigurationDialog } from './DeleteConfigurationDialog'; import { @@ -34,7 +34,11 @@ import { protocolOptions, } from './constants'; -import type { Configuration, ConfigurationPayload } from '@linode/api-v4'; +import type { + CertificateConfig, + Configuration, + ConfigurationPayload, +} from '@linode/api-v4'; interface EditProps { configuration: Configuration; @@ -57,7 +61,7 @@ export const ConfigurationForm = (props: CreateProps | EditProps) => { loadbalancerId: string; }>(); - const [isApplyCertDialogOpen, setIsApplyCertDialogOpen] = useState(false); + const [isAddCertDrawerOpen, setIsAddCertDrawerOpen] = useState(false); const [isAddRouteDrawerOpen, setIsAddRouteDrawerOpen] = useState(false); const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false); @@ -120,8 +124,9 @@ export const ConfigurationForm = (props: CreateProps | EditProps) => { const handleRemoveCert = (index: number) => { formik.setFieldTouched('certificates'); - formik.values.certificates.splice(index, 1); - formik.setFieldValue('certificates', formik.values.certificates); + const newCerts = [...formik.values.certificates]; + newCerts.splice(index, 1); + formik.setFieldValue('certificates', newCerts); }; const handleRemoveRoute = (index: number) => { @@ -133,11 +138,11 @@ export const ConfigurationForm = (props: CreateProps | EditProps) => { formik.setFieldValue('route_ids', newRouteIds); }; - const handleAddCerts = (certificates: Configuration['certificates']) => { + const handleAddCerts = (certificate: CertificateConfig) => { formik.setFieldTouched('certificates'); formik.setFieldValue('certificates', [ ...formik.values.certificates, - ...certificates, + certificate, ]); }; @@ -229,13 +234,10 @@ export const ConfigurationForm = (props: CreateProps | EditProps) => { /> @@ -308,11 +310,11 @@ export const ConfigurationForm = (props: CreateProps | EditProps) => { onClose={() => setIsAddRouteDrawerOpen(false)} open={isAddRouteDrawerOpen} /> - setIsApplyCertDialogOpen(false)} - open={isApplyCertDialogOpen} + onClose={() => setIsAddCertDrawerOpen(false)} + open={isAddCertDrawerOpen} /> {mode === 'edit' && ( { /> createLoadbalancerCertificate(loadbalancerId, data), { onSuccess(certificate) { + queryClient.invalidateQueries([ + QUERY_KEY, + 'loadbalancer', + loadbalancerId, + 'certificates', + ]); queryClient.setQueryData( [ QUERY_KEY, @@ -82,12 +88,6 @@ export const useLoadBalancerCertificateCreateMutation = ( ], certificate ); - queryClient.invalidateQueries([ - QUERY_KEY, - 'loadbalancer', - loadbalancerId, - 'certificates', - ]); }, } ); diff --git a/packages/validation/src/loadbalancers.schema.ts b/packages/validation/src/loadbalancers.schema.ts index 783ed02234e..54cf464111c 100644 --- a/packages/validation/src/loadbalancers.schema.ts +++ b/packages/validation/src/loadbalancers.schema.ts @@ -34,7 +34,7 @@ export const UpdateCertificateSchema = object().shape( [['certificate', 'key']] ); -const CertificateEntrySchema = object({ +export const CertificateEntrySchema = object({ id: number() .typeError('Certificate ID must be a number.') .required('Certificate ID is required.') @@ -42,10 +42,6 @@ const CertificateEntrySchema = object({ hostname: string().required('A Host Header is required.'), }); -export const CertificateConfigSchema = object({ - certificates: array(CertificateEntrySchema), -}); - export const EndpointSchema = object({ ip: string().required('IP is required.'), host: string().nullable(),