Skip to content

Commit 017b7b7

Browse files
committed
feature(crr): align Apply Actions step contract with the crr-configurator spec and CRR docs
1 parent 9f6e6ca commit 017b7b7

4 files changed

Lines changed: 60 additions & 35 deletions

File tree

src/react/locations/CRRSetupWizard/steps/ApplyActionsStep/ApplyActionsStep.test.tsx

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,19 @@ const ALL_STEPS_WHEN_NEW_ACCOUNT_AND_RULE = [
5656
'import-destination-certificate',
5757
'create-source-account',
5858
'create-source-bucket',
59-
'create-destination-account',
59+
'create-account',
6060
'create-user',
6161
'create-access-key',
6262
'create-policy',
63-
'attach-policy',
63+
'create-role',
64+
'attach-role-policy',
65+
'create-bucket',
6466
'create-location',
6567
'create-replication-rule',
6668
] as const;
6769

6870
describe('ApplyActionsStep', () => {
69-
it('renders every ISV-worded action label and marks each step Pending... before any event lands', () => {
71+
it('lists every provisioning action to run, all pending, before the setup starts', () => {
7072
server.use(
7173
rest.post(STREAM_URL, (_req, res, ctx) =>
7274
res(ctx.set('Content-Type', 'application/x-ndjson'), ctx.body(ndjson())),
@@ -81,10 +83,12 @@ describe('ApplyActionsStep', () => {
8183
expect(screen.getByText('Create IAM User')).toBeInTheDocument();
8284
expect(screen.getByText('Generate Access Key')).toBeInTheDocument();
8385
expect(screen.getByText('Create Policy')).toBeInTheDocument();
84-
expect(screen.getByText('Attach Policy to User')).toBeInTheDocument();
86+
expect(screen.getByText('Create IAM Role')).toBeInTheDocument();
87+
expect(screen.getByText('Attach Policy to Role')).toBeInTheDocument();
88+
expect(screen.getByText('Create Target Bucket: target-bucket')).toBeInTheDocument();
8589
expect(screen.getByText('Create Location')).toBeInTheDocument();
8690
expect(screen.getByText('Create Replication Rule')).toBeInTheDocument();
87-
expect(screen.getAllByText('Pending...').length).toBe(10);
91+
expect(screen.getAllByText('Pending...').length).toBe(12);
8892
});
8993

9094
it('uses the destination instance name in the title when one was returned by Verify', () => {
@@ -117,7 +121,7 @@ describe('ApplyActionsStep', () => {
117121
expect(screen.queryByText(/Authenticate/i)).not.toBeInTheDocument();
118122
});
119123

120-
it('shows Success cells as events land and enables Continue once every step has succeeded', async () => {
124+
it('marks each action Success as it completes and only lets the user continue once all have succeeded', async () => {
121125
server.use(
122126
rest.post(STREAM_URL, (_req, res, ctx) =>
123127
res(
@@ -136,7 +140,7 @@ describe('ApplyActionsStep', () => {
136140
);
137141
render(<ApplyActionsStep {...VALUES} />, { wrapper: Wrapper });
138142

139-
await waitFor(() => expect(screen.getAllByText('Success').length).toBe(10));
143+
await waitFor(() => expect(screen.getAllByText('Success').length).toBe(12));
140144
const continueButton = screen.getByRole('button', { name: /Continue/i });
141145
await waitFor(() => expect(continueButton).toBeEnabled());
142146
await userEvent.click(continueButton);
@@ -214,18 +218,18 @@ describe('ApplyActionsStep', () => {
214218
await waitFor(() => expect(requestCount).toBe(2));
215219
});
216220

217-
it('drops create-source-account when the source account already exists', () => {
221+
it('skips creating a source account when the user reuses an existing one', () => {
218222
server.use(
219223
rest.post(STREAM_URL, (_req, res, ctx) =>
220224
res(ctx.set('Content-Type', 'application/x-ndjson'), ctx.body(ndjson())),
221225
),
222226
);
223227
render(<ApplyActionsStep {...VALUES} accountNameType="existing" />, { wrapper: Wrapper });
224228
expect(screen.queryByText(/Create Account on Source/i)).not.toBeInTheDocument();
225-
expect(screen.getAllByText('Pending...').length).toBe(9);
229+
expect(screen.getAllByText('Pending...').length).toBe(11);
226230
});
227231

228-
it('drops create-source-bucket and create-replication-rule when the wizard did not opt into replication rule creation', () => {
232+
it('skips the source bucket, target bucket and replication rule when the user opts out of creating a rule', () => {
229233
server.use(
230234
rest.post(STREAM_URL, (_req, res, ctx) =>
231235
res(ctx.set('Content-Type', 'application/x-ndjson'), ctx.body(ndjson())),
@@ -235,7 +239,8 @@ describe('ApplyActionsStep', () => {
235239
wrapper: Wrapper,
236240
});
237241
expect(screen.queryByText(/Create Bucket on Source/i)).not.toBeInTheDocument();
242+
expect(screen.queryByText(/Create Target Bucket/i)).not.toBeInTheDocument();
238243
expect(screen.queryByText(/Create Replication Rule/i)).not.toBeInTheDocument();
239-
expect(screen.getAllByText('Pending...').length).toBe(8);
244+
expect(screen.getAllByText('Pending...').length).toBe(9);
240245
});
241246
});

src/react/locations/CRRSetupWizard/steps/ApplyActionsStep/ApplyActionsStep.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,13 @@ export const ApplyActionsStep = (props: Props) => {
8383
createReplicationRule: createReplicationRule === true,
8484
sourceAccountName: accountName ?? '',
8585
sourceBucketName: sourceBucketName ?? '',
86+
targetBucketName: props.targetBucketName ?? '',
8687
destinationAccountName: destinationAccountName ?? '',
8788
},
8889
setup.events,
8990
{ globalErrorMessage },
9091
),
91-
[accountNameType, createReplicationRule, accountName, sourceBucketName, destinationAccountName, setup.events, globalErrorMessage],
92+
[accountNameType, createReplicationRule, accountName, sourceBucketName, props.targetBucketName, destinationAccountName, setup.events, globalErrorMessage],
9293
);
9394

9495
const hasStartedRef = useRef(false);

src/react/locations/CRRSetupWizard/steps/ApplyActionsStep/steps.test.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,45 @@ const baseInput: StepListInput = {
66
createReplicationRule: true,
77
sourceAccountName: 'src-account',
88
sourceBucketName: 'src-bucket',
9+
targetBucketName: 'target-bucket',
910
destinationAccountName: 'dest-account',
1011
};
1112

1213
describe('buildStepViews', () => {
13-
it('lists the 10 canonical steps in order when both create-source-account and create-replication-rule apply', () => {
14+
it('lists the full provisioning sequence in order when the user creates a new source account and a replication rule', () => {
1415
const views = buildStepViews(baseInput, []);
1516
expect(views.map((v) => v.id)).toEqual([
1617
'import-destination-certificate',
1718
'create-source-account',
1819
'create-source-bucket',
19-
'create-destination-account',
20+
'create-account',
2021
'create-user',
2122
'create-access-key',
2223
'create-policy',
23-
'attach-policy',
24+
'create-role',
25+
'attach-role-policy',
26+
'create-bucket',
2427
'create-location',
2528
'create-replication-rule',
2629
]);
2730
});
2831

29-
it('numbers steps starting at 1 and interpolates source/destination names', () => {
32+
it('numbers the steps and shows the chosen source and destination account names', () => {
3033
const [first, sourceAcc, sourceBkt, destAcc] = buildStepViews(baseInput, []);
3134
expect(first).toMatchObject({ step: 1, label: 'Import Destination Certificate' });
3235
expect(sourceAcc).toMatchObject({ step: 2, label: 'Create Account on Source: src-account' });
3336
expect(sourceBkt).toMatchObject({ step: 3, label: 'Create Bucket on Source: src-bucket' });
3437
expect(destAcc).toMatchObject({ step: 4, label: 'Create Account on Destination: dest-account' });
3538
});
3639

37-
it('uses ISV wording verbatim for the reused actions', () => {
40+
it('labels the destination IAM chain and the target bucket per the ARTESCA CRR procedure', () => {
3841
const labels = buildStepViews(baseInput, []).map((v) => v.label);
3942
expect(labels).toContain('Create IAM User');
4043
expect(labels).toContain('Generate Access Key');
4144
expect(labels).toContain('Create Policy');
42-
expect(labels).toContain('Attach Policy to User');
45+
expect(labels).toContain('Create IAM Role');
46+
expect(labels).toContain('Attach Policy to Role');
47+
expect(labels).toContain('Create Target Bucket: target-bucket');
4348
expect(labels).toContain('Create Location');
4449
expect(labels).toContain('Create Replication Rule');
4550
});
@@ -49,28 +54,33 @@ describe('buildStepViews', () => {
4954
expect(views.find((v) => v.id === 'create-source-account')).toBeUndefined();
5055
});
5156

52-
it('drops the two replication-only steps when the wizard did not opt into replication rule creation', () => {
57+
it('drops the replication-only steps when the wizard did not opt into replication rule creation', () => {
5358
const views = buildStepViews({ ...baseInput, createReplicationRule: false }, []);
5459
expect(views.find((v) => v.id === 'create-source-bucket')).toBeUndefined();
60+
expect(views.find((v) => v.id === 'create-bucket')).toBeUndefined();
5561
expect(views.find((v) => v.id === 'create-replication-rule')).toBeUndefined();
5662
});
5763

58-
it('never surfaces authenticate, create-role, attach-role-policy or create-bucket-on-destination', () => {
64+
it('never surfaces the backend authenticate step (it is covered by the Verify wizard step)', () => {
5965
const ids = buildStepViews(baseInput, []).map((v) => v.id) as string[];
60-
for (const id of ['authenticate', 'create-role', 'attach-role-policy', 'create-bucket']) {
61-
expect(ids).not.toContain(id);
66+
expect(ids).not.toContain('authenticate');
67+
});
68+
69+
it('surfaces the destination role, policy attachment and target bucket steps the CRR procedure requires', () => {
70+
const ids = buildStepViews(baseInput, []).map((v) => v.id) as string[];
71+
for (const id of ['create-role', 'attach-role-policy', 'create-bucket']) {
72+
expect(ids).toContain(id);
6273
}
6374
});
6475

65-
it('leaves every step pending until a matching event lands', () => {
76+
it('shows every step as pending before the setup runs', () => {
6677
const views = buildStepViews(baseInput, []);
6778
expect(views.every((v) => v.state === 'pending')).toBe(true);
6879
});
6980

70-
it('marks a step succeeded on step.completed and failed on step.failed with its error message', () => {
81+
it('marks a step done once it completes and shows the reason when one fails', () => {
7182
const events: SetupEvent[] = [
72-
{ event: 'step.completed', step: 'import-destination-certificate', at: 't' },
73-
{ event: 'step.completed', step: 'create-source-account', at: 't' },
83+
{ event: 'step.completed', step: 'create-account', at: 't' },
7484
{
7585
event: 'step.failed',
7686
step: 'create-user',
@@ -79,23 +89,23 @@ describe('buildStepViews', () => {
7989
},
8090
];
8191
const views = buildStepViews(baseInput, events);
82-
expect(views.find((v) => v.id === 'import-destination-certificate')?.state).toBe('succeeded');
92+
expect(views.find((v) => v.id === 'create-account')?.state).toBe('succeeded');
8393
const failed = views.find((v) => v.id === 'create-user');
8494
expect(failed?.state).toBe('failed');
8595
expect(failed?.errorMessage).toBe('IAM refused CreateUser: entity already exists');
8696
});
8797
});
8898

89-
describe('buildStepViews with a globalErrorMessage', () => {
90-
it('marks the first pending step as failed with the global error message when no step-level failure landed', () => {
99+
describe('when the whole setup fails without pinpointing a step', () => {
100+
it('blames the first step still waiting to run and shows why', () => {
91101
const views = buildStepViews(baseInput, [], { globalErrorMessage: 'network exploded' });
92102
const firstPending = views[0];
93103
expect(firstPending.state).toBe('failed');
94104
expect(firstPending.errorMessage).toBe('network exploded');
95105
for (const v of views.slice(1)) expect(v.state).toBe('pending');
96106
});
97107

98-
it('marks the first still-pending step as failed after some steps already succeeded', () => {
108+
it('blames the first unfinished step once earlier ones have already succeeded', () => {
99109
const events: SetupEvent[] = [
100110
{ event: 'step.completed', step: 'import-destination-certificate', at: 't' },
101111
{ event: 'step.completed', step: 'create-source-account', at: 't' },
@@ -107,7 +117,7 @@ describe('buildStepViews with a globalErrorMessage', () => {
107117
expect(views[2].errorMessage).toBe('stream ended without a terminal event');
108118
});
109119

110-
it('leaves the step-level failure in place when both a step.failed and a global error are present', () => {
120+
it('keeps a specific step failure visible rather than replacing it with the generic error', () => {
111121
const events: SetupEvent[] = [
112122
{
113123
event: 'step.failed',

src/react/locations/CRRSetupWizard/steps/ApplyActionsStep/steps.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ export type StepId =
44
| 'import-destination-certificate'
55
| 'create-source-account'
66
| 'create-source-bucket'
7-
| 'create-destination-account'
7+
| 'create-account'
88
| 'create-user'
99
| 'create-access-key'
1010
| 'create-policy'
11-
| 'attach-policy'
11+
| 'create-role'
12+
| 'attach-role-policy'
13+
| 'create-bucket'
1214
| 'create-location'
1315
| 'create-replication-rule';
1416

@@ -27,6 +29,7 @@ export type StepListInput = {
2729
createReplicationRule: boolean;
2830
sourceAccountName: string;
2931
sourceBucketName: string;
32+
targetBucketName: string;
3033
destinationAccountName: string;
3134
};
3235

@@ -57,14 +60,20 @@ const STEPS: StepDef[] = [
5760
label: (i) => `Create Bucket on Source: ${i.sourceBucketName}`,
5861
},
5962
{
60-
id: 'create-destination-account',
63+
id: 'create-account',
6164
when: () => true,
6265
label: (i) => `Create Account on Destination: ${i.destinationAccountName}`,
6366
},
6467
{ id: 'create-user', when: () => true, label: () => 'Create IAM User' },
6568
{ id: 'create-access-key', when: () => true, label: () => 'Generate Access Key' },
6669
{ id: 'create-policy', when: () => true, label: () => 'Create Policy' },
67-
{ id: 'attach-policy', when: () => true, label: () => 'Attach Policy to User' },
70+
{ id: 'create-role', when: () => true, label: () => 'Create IAM Role' },
71+
{ id: 'attach-role-policy', when: () => true, label: () => 'Attach Policy to Role' },
72+
{
73+
id: 'create-bucket',
74+
when: (i) => i.createReplicationRule,
75+
label: (i) => `Create Target Bucket: ${i.targetBucketName}`,
76+
},
6877
{ id: 'create-location', when: () => true, label: () => 'Create Location' },
6978
{
7079
id: 'create-replication-rule',

0 commit comments

Comments
 (0)