Skip to content

Commit 41ea17a

Browse files
authored
upcoming: [M3-9777] - Add label field when creating a firewall from a template in CreateFirewallDrawer (#12069)
* add label when creating firewall from a template * make label optional * Added changeset: Add Firewall label to CreateFirewallDrawer when using firewall templates * update changeset * Revert "make label optional" This reverts commit b6a079d.
1 parent 7c2a188 commit 41ea17a

10 files changed

+200
-112
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Upcoming Features
3+
---
4+
5+
Add label field to CreateFirewallDrawer form when using firewall templates ([#12069](https://github.com/linode/manager/pull/12069))

packages/manager/cypress/e2e/core/firewalls/create-firewall-custom.spec.ts

+32
Original file line numberDiff line numberDiff line change
@@ -425,4 +425,36 @@ describe('Can create Firewalls using custom rules', () => {
425425
cy.contains(mockNodeBalancer.label).should('be.visible');
426426
});
427427
});
428+
429+
/*
430+
* - Confirms a validation error displays when trying to create a custom Firewall without inputting a label
431+
*/
432+
it('displays an error when trying to create a customFirewall without a label', () => {
433+
mockGetFirewalls([]).as('getFirewalls');
434+
435+
cy.visitWithLogin('/firewalls');
436+
cy.wait('@getFirewalls');
437+
438+
ui.button
439+
.findByTitle('Create Firewall')
440+
.should('be.visible')
441+
.should('be.enabled')
442+
.click();
443+
444+
ui.drawer
445+
.findByTitle('Create Firewall')
446+
.should('be.visible')
447+
.within(() => {
448+
cy.findByLabelText('Custom Firewall').should('be.checked');
449+
450+
// Make no changes to the default selections, just click "Create Firewall".
451+
ui.buttonGroup
452+
.findButtonByTitle('Create Firewall')
453+
.should('be.visible')
454+
.should('be.enabled')
455+
.click();
456+
});
457+
458+
cy.findByText('Label is required.');
459+
});
428460
});

packages/manager/cypress/e2e/core/firewalls/create-firewall-with-template.spec.ts

+67-39
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22
* @file Integration tests for Firewall creation flows involving templates.
33
*/
44

5-
import {
6-
accountFactory,
7-
firewallFactory,
8-
firewallTemplateFactory,
9-
} from 'src/factories';
105
import { mockGetAccount } from 'support/intercepts/account';
116
import { mockAppendFeatureFlags } from 'support/intercepts/feature-flags';
127
import {
@@ -17,8 +12,13 @@ import {
1712
} from 'support/intercepts/firewalls';
1813
import { mockApiInternalUser } from 'support/intercepts/general';
1914
import { ui } from 'support/ui';
20-
import { buildArray } from 'support/util/arrays';
21-
import { randomNumber } from 'support/util/random';
15+
import { randomLabel } from 'support/util/random';
16+
17+
import {
18+
accountFactory,
19+
firewallFactory,
20+
firewallTemplateFactory,
21+
} from 'src/factories';
2222

2323
const mockFirewallTemplateVpc = firewallTemplateFactory.build({
2424
slug: 'vpc',
@@ -51,13 +51,13 @@ describe('Can create Firewalls using templates', () => {
5151
/*
5252
* - Confirms that users can create a Firewall using the VPC template.
5353
* - Confirms that VPC template-specific details are shown prior to creating Firewall.
54-
* - Confirms that outgoing Firewall create request includes autogenerated label.
54+
* - Confirms that outgoing Firewall create request includes chosen Firewall label.
5555
* - Confirms that outgoing Firewall create request includes chosen rules.
5656
* - Confirms that landing page automatically updates to display the new Firewall.
5757
*/
5858
it('can create a Firewall using VPC template', () => {
5959
const mockFirewall = firewallFactory.build({
60-
label: 'vpc-1',
60+
label: randomLabel(),
6161
rules: mockFirewallTemplateVpc.rules,
6262
entities: [],
6363
});
@@ -83,6 +83,7 @@ describe('Can create Firewalls using templates', () => {
8383
.should('be.visible')
8484
.within(() => {
8585
cy.findByText('From a Template').should('be.visible').click();
86+
cy.findByText('Label').should('be.visible').type(mockFirewall.label);
8687

8788
cy.findByLabelText('Firewall Template').click();
8889
ui.autocompletePopper.find().within(() => {
@@ -104,16 +105,16 @@ describe('Can create Firewalls using templates', () => {
104105
});
105106

106107
cy.wait('@createFirewall').then((xhr) => {
107-
// Confirm that label is automatically assigned, and that rules reflect the selected template.
108-
expect(xhr.request.body['label']).to.equal('vpc-1');
108+
// Confirm that rules reflect the selected template.
109+
expect(xhr.request.body['label']).to.equal(mockFirewall.label);
109110
expect(xhr.request.body['rules']).to.deep.equal(
110111
mockFirewallTemplateVpc.rules
111112
);
112113
});
113114

114115
// Confirm that page automatically updates to show the new Firewall,
115116
// and that the expected information is displayed alongside the Firewall.
116-
cy.findByText('vpc-1')
117+
cy.findByText(mockFirewall.label)
117118
.should('be.visible')
118119
.closest('tr')
119120
.within(() => {
@@ -126,36 +127,24 @@ describe('Can create Firewalls using templates', () => {
126127
/*
127128
* - Confirms that users can create a Firewall using the Public template.
128129
* - Confirms that Public template-specific details are shown prior to creating Firewall.
129-
* - Confirms that outgoing Firewall create request includes autogenerated label.
130-
* - Confirms that autogenerated Firewall label takes into account existing Firewalls.
130+
* - Confirms that outgoing Firewall create request includes chosen Firewall label
131131
* - Confirms that outgoing Firewall create request includes chosen rules.
132132
* - Confirms that landing page automatically updates to display the new Firewall.
133133
*/
134134
it('can create a Firewall using Public template', () => {
135-
const existingFirewallCount = randomNumber(1, 10);
136-
137-
const mockExistingFirewalls = buildArray(existingFirewallCount, (i) => {
138-
return firewallFactory.build({
139-
label: `public-${i + 1}`,
140-
rules: mockFirewallTemplatePublic.rules,
141-
entities: [],
142-
});
143-
});
144-
145-
const mockNewFirewallLabel = `public-${existingFirewallCount + 1}`;
146-
const mockNewFirewall = firewallFactory.build({
147-
label: mockNewFirewallLabel,
135+
const mockFirewall = firewallFactory.build({
136+
label: randomLabel(),
148137
rules: mockFirewallTemplatePublic.rules,
149138
entities: [],
150139
});
151140

152-
mockGetFirewalls(mockExistingFirewalls).as('getFirewalls');
141+
mockGetFirewalls([]).as('getFirewalls');
153142
mockGetFirewallTemplates([
154143
mockFirewallTemplateVpc,
155144
mockFirewallTemplatePublic,
156145
]);
157146
mockGetFirewallTemplate(mockFirewallTemplatePublic);
158-
mockCreateFirewall(mockNewFirewall).as('createFirewall');
147+
mockCreateFirewall(mockFirewall).as('createFirewall');
159148

160149
cy.visitWithLogin('/firewalls');
161150
cy.wait('@getFirewalls');
@@ -171,6 +160,7 @@ describe('Can create Firewalls using templates', () => {
171160
.should('be.visible')
172161
.within(() => {
173162
cy.findByText('From a Template').should('be.visible').click();
163+
cy.findByText('Label').should('be.visible').type(mockFirewall.label);
174164

175165
cy.findByLabelText('Firewall Template').click();
176166
ui.autocompletePopper.find().within(() => {
@@ -186,9 +176,7 @@ describe('Can create Firewalls using templates', () => {
186176
).should('be.visible');
187177

188178
// Create the Firewall
189-
mockGetFirewalls([...mockExistingFirewalls, mockNewFirewall]).as(
190-
'getFirewalls'
191-
);
179+
mockGetFirewalls([mockFirewall]).as('getFirewalls');
192180
ui.buttonGroup
193181
.findButtonByTitle('Create Firewall')
194182
.should('be.visible')
@@ -197,17 +185,17 @@ describe('Can create Firewalls using templates', () => {
197185
});
198186

199187
cy.wait('@createFirewall').then((xhr) => {
200-
// Confirm that label is automatically assigned, taking into account existing firewall labels,
188+
// Confirm that label reflects chosen label
201189
// and that the Firewall rules reflect the selected template.
202-
expect(xhr.request.body['label']).to.equal(mockNewFirewallLabel);
190+
expect(xhr.request.body['label']).to.equal(mockFirewall.label);
203191
expect(xhr.request.body['rules']).to.deep.equal(
204192
mockFirewallTemplatePublic.rules
205193
);
206194
});
207195

208196
// Confirm that page automatically updates to show the new Firewall,
209197
// and that the expected information is displayed alongside the Firewall.
210-
cy.findByText(mockNewFirewallLabel)
198+
cy.findByText(mockFirewall.label)
211199
.should('be.visible')
212200
.closest('tr')
213201
.within(() => {
@@ -220,13 +208,13 @@ describe('Can create Firewalls using templates', () => {
220208
/*
221209
* - Confirms that users can create a Firewall using the internal template.
222210
* - Confirms that internal choice is present when internal API header exists and API includes the template.
223-
* - Confirms that outgoing Firewall create request includes autogenerated label.
211+
* - Confirms that outgoing Firewall create request includes selected Firewall label.
224212
* - Confirms that outgoing Firewall create request includes chosen rules.
225213
* - Confirms that landing page automatically updates to display the new Firewall.
226214
*/
227215
it('can create a Firewall using internal-only template as internal user', () => {
228216
const mockFirewall = firewallFactory.build({
229-
label: 'akamai-non-prod-1',
217+
label: randomLabel(),
230218
rules: mockFirewallTemplateInternalUser.rules,
231219
entities: [],
232220
});
@@ -254,6 +242,7 @@ describe('Can create Firewalls using templates', () => {
254242
.should('be.visible')
255243
.within(() => {
256244
cy.findByText('From a Template').should('be.visible').click();
245+
cy.findByText('Label').should('be.visible').type(mockFirewall.label);
257246

258247
cy.findByLabelText('Firewall Template').click();
259248
ui.autocompletePopper.find().within(() => {
@@ -275,15 +264,15 @@ describe('Can create Firewalls using templates', () => {
275264
cy.wait('@createFirewall').then((xhr) => {
276265
// Confirm that page automatically updates to show the new Firewall,
277266
// and that the expected information is displayed alongside the Firewall.
278-
expect(xhr.request.body['label']).to.equal('akamai-non-prod-1');
267+
expect(xhr.request.body['label']).to.equal(mockFirewall.label);
279268
expect(xhr.request.body['rules']).to.deep.equal(
280269
mockFirewallTemplateInternalUser.rules
281270
);
282271
});
283272

284273
// Confirm that page automatically updates to show the new Firewall,
285274
// and that the expected information is displayed alongside the Firewall.
286-
cy.findByText('akamai-non-prod-1')
275+
cy.findByText(mockFirewall.label)
287276
.should('be.visible')
288277
.closest('tr')
289278
.within(() => {
@@ -317,6 +306,7 @@ describe('Can create Firewalls using templates', () => {
317306
.should('be.visible')
318307
.within(() => {
319308
cy.findByText('From a Template').should('be.visible').click();
309+
cy.findByText('Label').should('be.visible').type(randomLabel());
320310

321311
cy.findByLabelText('Firewall Template').click();
322312
ui.autocompletePopper.find().within(() => {
@@ -329,6 +319,44 @@ describe('Can create Firewalls using templates', () => {
329319
});
330320
});
331321

322+
/*
323+
* - Confirms a validation error displays for firewall label if no label inputted
324+
* - Confirms a validation error displays for template select if no template chosen
325+
*/
326+
it('displays an error when trying to create a Firewall from a template without a label', () => {
327+
mockGetFirewalls([]).as('getFirewalls');
328+
mockGetFirewallTemplates([
329+
mockFirewallTemplateVpc,
330+
mockFirewallTemplatePublic,
331+
]);
332+
mockGetFirewallTemplate(mockFirewallTemplatePublic);
333+
334+
cy.visitWithLogin('/firewalls');
335+
cy.wait('@getFirewalls');
336+
337+
ui.button
338+
.findByTitle('Create Firewall')
339+
.should('be.visible')
340+
.should('be.enabled')
341+
.click();
342+
343+
ui.drawer
344+
.findByTitle('Create Firewall')
345+
.should('be.visible')
346+
.within(() => {
347+
cy.findByText('From a Template').should('be.visible').click();
348+
349+
ui.buttonGroup
350+
.findButtonByTitle('Create Firewall')
351+
.should('be.visible')
352+
.should('be.enabled')
353+
.click();
354+
});
355+
356+
cy.findByText('Label is required.');
357+
cy.findByText('Please select a template to create a firewall from.');
358+
});
359+
332360
// TODO M3-9775 - Delete this test once `linodeInterfaces` feature flag is removed.
333361
/*
334362
* - Confirms that "Custom Firewall" and "From a Template" selections are absent when feature flag is disabled.

packages/manager/src/components/GenerateFirewallDialog/useCreateFirewallFromTemplate.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { useQueryClient } from '@tanstack/react-query';
2-
31
import { firewallQueries, useCreateFirewall } from '@linode/queries';
2+
import { useQueryClient } from '@tanstack/react-query';
43

54
import type { DialogState } from './GenerateFirewallDialog';
65
import type {
@@ -46,11 +45,18 @@ export const useCreateFirewallFromTemplate = (options: {
4645

4746
export const createFirewallFromTemplate = async (options: {
4847
createFirewall: (firewall: CreateFirewallPayload) => Promise<Firewall>;
48+
firewallLabel?: string;
4949
queryClient: QueryClient;
5050
templateSlug: FirewallTemplateSlug;
5151
updateProgress?: (progress: number | undefined) => void;
5252
}): Promise<Firewall> => {
53-
const { createFirewall, queryClient, templateSlug, updateProgress } = options;
53+
const {
54+
createFirewall,
55+
queryClient,
56+
templateSlug,
57+
updateProgress,
58+
firewallLabel,
59+
} = options;
5460
if (updateProgress) {
5561
updateProgress(0);
5662
}
@@ -66,7 +72,7 @@ export const createFirewallFromTemplate = async (options: {
6672
updateProgress(80); // this gives the appearance of linear progress
6773
}
6874
// Determine new firewall name
69-
const label = getUniqueFirewallLabel(slug, firewalls);
75+
const label = firewallLabel ?? getUniqueFirewallLabel(slug, firewalls);
7076

7177
// Create new firewall
7278
return await createFirewall({ label, rules });

packages/manager/src/features/Firewalls/FirewallLanding/CreateFirewallDrawer.test.tsx

+6
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ describe('Create Firewall Drawer', () => {
3030
expect(title).toBeVisible();
3131
});
3232

33+
it('should always show the label field', () => {
34+
renderWithTheme(<CreateFirewallDrawer {...props} />);
35+
const label = screen.getByText('Label');
36+
expect(label).toBeVisible();
37+
});
38+
3339
it('should render radio buttons for default inbound/outbound policies', () => {
3440
renderWithTheme(<CreateFirewallDrawer {...props} />);
3541
const withinInboundPolicy = within(

0 commit comments

Comments
 (0)