Skip to content

Commit fbd6cc0

Browse files
VirtueMeclaude
VirtueMe
and
claude
authored
refactor(events): extract shared input validator and IAM role factory (#764)
- Add lib/deploy/events/eventUtils.js with validateEventInput and buildIamRole - validateEventInput replaces duplicate if/else Input/InputPath/InputTransformer mutual-exclusivity checks in both schedule and cloudwatch event compilers - buildIamRole replaces duplicate JSON string IAM role templates, parameterised by principal service (events.amazonaws.com / scheduler.amazonaws.com) - Eliminates JSON.parse/stringify dance — IAM role mutations applied directly - Add 10 unit tests for both utilities Part of #760 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent d241e59 commit fbd6cc0

4 files changed

Lines changed: 138 additions & 148 deletions

File tree

‎lib/deploy/events/cloudWatchEvent/compileCloudWatchEventEvents.js‎

Lines changed: 10 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const _ = require('lodash');
44
const BbPromise = require('bluebird');
5+
const { validateEventInput, buildIamRole } = require('../eventUtils');
56

67
module.exports = {
78
compileCloudWatchEventEvents() {
@@ -62,14 +63,12 @@ module.exports = {
6263
})
6364
: undefined;
6465

65-
if ([Input, InputPath, InputTransformer].filter(Boolean).length > 1) {
66-
const errorMessage = [
67-
'You can\'t set input, inputPath and inputTransformer properties at the',
68-
'same time for cloudwatch events.',
69-
'Please check the AWS docs for more info',
70-
].join('');
71-
throw new this.serverless.classes.Error(errorMessage);
72-
}
66+
validateEventInput(
67+
Input,
68+
InputPath,
69+
InputTransformer,
70+
this.serverless.classes.Error,
71+
);
7372

7473
if (Input && typeof Input === 'object') {
7574
Input = JSON.stringify(Input);
@@ -137,64 +136,19 @@ module.exports = {
137136
}
138137
`;
139138

140-
let iamRoleTemplate = `
141-
{
142-
"Type": "AWS::IAM::Role",
143-
"Properties": {
144-
"AssumeRolePolicyDocument": {
145-
"Version": "2012-10-17",
146-
"Statement": [
147-
{
148-
"Effect": "Allow",
149-
"Principal": {
150-
"Service": "events.amazonaws.com"
151-
},
152-
"Action": "sts:AssumeRole"
153-
}
154-
]
155-
},
156-
"Policies": [
157-
{
158-
"PolicyName": "${policyName}",
159-
"PolicyDocument": {
160-
"Version": "2012-10-17",
161-
"Statement": [
162-
{
163-
"Effect": "Allow",
164-
"Action": [
165-
"states:StartExecution"
166-
],
167-
"Resource": {
168-
"Ref": "${stateMachineLogicalId}"
169-
}
170-
}
171-
]
172-
}
173-
}
174-
]
175-
}
176-
}
177-
`;
178-
179139
const newCloudWatchEventRuleObject = {
180140
[cloudWatchLogicalId]: JSON.parse(cloudWatchEventRuleTemplate),
181141
};
182142

183143
const objectsToMerge = [newCloudWatchEventRuleObject];
184144

185145
if (!IamRole) {
146+
const iamRole = buildIamRole('events.amazonaws.com', policyName, stateMachineLogicalId);
186147
const rolePath = _.get(this.serverless.service, 'provider.iam.role.path');
187148
if (rolePath) {
188-
const jsonIamRole = JSON.parse(iamRoleTemplate);
189-
jsonIamRole.Properties.Path = rolePath;
190-
iamRoleTemplate = JSON.stringify(jsonIamRole);
149+
iamRole.Properties.Path = rolePath;
191150
}
192-
193-
const newPermissionObject = {
194-
[cloudWatchIamRoleLogicalId]: JSON.parse(iamRoleTemplate),
195-
};
196-
197-
objectsToMerge.push(newPermissionObject);
151+
objectsToMerge.push({ [cloudWatchIamRoleLogicalId]: iamRole });
198152
}
199153

200154
_.merge(

‎lib/deploy/events/eventUtils.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
3+
/**
4+
* Validates that at most one of Input, InputPath, InputTransformer is set.
5+
* Throws a Serverless error if more than one is set simultaneously.
6+
*/
7+
function validateEventInput(Input, InputPath, InputTransformer, ServerlessError) {
8+
if ([Input, InputPath, InputTransformer].filter(Boolean).length > 1) {
9+
throw new ServerlessError(
10+
"You can't set input, inputPath and inputTransformer properties at the "
11+
+ 'same time for events. Please check the AWS docs for more info',
12+
);
13+
}
14+
}
15+
16+
/**
17+
* Builds a CloudFormation IAM Role resource for events/scheduler → Step Functions execution.
18+
* @param {string} principalService - e.g. 'events.amazonaws.com' or 'scheduler.amazonaws.com'
19+
* @param {string} policyName
20+
* @param {string} stateMachineLogicalId
21+
* @returns {object} CloudFormation IAM Role resource object
22+
*/
23+
function buildIamRole(principalService, policyName, stateMachineLogicalId) {
24+
return {
25+
Type: 'AWS::IAM::Role',
26+
Properties: {
27+
AssumeRolePolicyDocument: {
28+
Version: '2012-10-17',
29+
Statement: [{
30+
Effect: 'Allow',
31+
Principal: { Service: principalService },
32+
Action: 'sts:AssumeRole',
33+
}],
34+
},
35+
Policies: [{
36+
PolicyName: policyName,
37+
PolicyDocument: {
38+
Version: '2012-10-17',
39+
Statement: [{
40+
Effect: 'Allow',
41+
Action: ['states:StartExecution'],
42+
Resource: { Ref: stateMachineLogicalId },
43+
}],
44+
},
45+
}],
46+
},
47+
};
48+
}
49+
50+
module.exports = { validateEventInput, buildIamRole };

‎lib/deploy/events/eventUtils.test.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
3+
const expect = require('chai').expect;
4+
const { validateEventInput, buildIamRole } = require('./eventUtils');
5+
6+
class TestError extends Error {}
7+
8+
describe('eventUtils', () => {
9+
describe('#validateEventInput()', () => {
10+
it('should not throw when only Input is set', () => {
11+
expect(() => validateEventInput('value', undefined, undefined, TestError)).to.not.throw();
12+
});
13+
14+
it('should not throw when only InputPath is set', () => {
15+
expect(() => validateEventInput(undefined, '$.path', undefined, TestError)).to.not.throw();
16+
});
17+
18+
it('should not throw when only InputTransformer is set', () => {
19+
expect(() => validateEventInput(undefined, undefined, { inputTemplate: 'tpl' }, TestError)).to.not.throw();
20+
});
21+
22+
it('should not throw when none are set', () => {
23+
expect(() => validateEventInput(undefined, undefined, undefined, TestError)).to.not.throw();
24+
});
25+
26+
it('should throw when Input and InputPath are both set', () => {
27+
expect(() => validateEventInput('value', '$.path', undefined, TestError)).to.throw(TestError);
28+
});
29+
30+
it('should throw when Input and InputTransformer are both set', () => {
31+
expect(() => validateEventInput('value', undefined, { inputTemplate: 'tpl' }, TestError)).to.throw(TestError);
32+
});
33+
34+
it('should throw when all three are set', () => {
35+
expect(() => validateEventInput('value', '$.path', { inputTemplate: 'tpl' }, TestError)).to.throw(TestError);
36+
});
37+
});
38+
39+
describe('#buildIamRole()', () => {
40+
it('should return a CF IAM Role with the given principal service', () => {
41+
const result = buildIamRole('events.amazonaws.com', 'my-policy', 'MyStateMachine');
42+
expect(result.Type).to.equal('AWS::IAM::Role');
43+
expect(result.Properties.AssumeRolePolicyDocument.Statement[0].Principal.Service)
44+
.to.equal('events.amazonaws.com');
45+
});
46+
47+
it('should set the policy name and state machine resource ref', () => {
48+
const result = buildIamRole('scheduler.amazonaws.com', 'my-policy', 'MyStateMachine');
49+
const policy = result.Properties.Policies[0];
50+
expect(policy.PolicyName).to.equal('my-policy');
51+
expect(policy.PolicyDocument.Statement[0].Resource).to.deep.equal({ Ref: 'MyStateMachine' });
52+
expect(policy.PolicyDocument.Statement[0].Action).to.deep.equal(['states:StartExecution']);
53+
});
54+
55+
it('should use the given principal service for scheduler', () => {
56+
const result = buildIamRole('scheduler.amazonaws.com', 'my-policy', 'MyStateMachine');
57+
expect(result.Properties.AssumeRolePolicyDocument.Statement[0].Principal.Service)
58+
.to.equal('scheduler.amazonaws.com');
59+
});
60+
});
61+
});

‎lib/deploy/events/schedule/compileScheduledEvents.js‎

Lines changed: 17 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const _ = require('lodash');
44
const BbPromise = require('bluebird');
5+
const { validateEventInput, buildIamRole } = require('../eventUtils');
56

67
const METHOD_SCHEDULER = 'scheduler';
78
const METHOD_EVENT_BUS = 'eventBus';
@@ -63,15 +64,12 @@ module.exports = {
6364
})
6465
: undefined;
6566

66-
if ([Input, InputPath, InputTransformer].filter(Boolean).length > 1) {
67-
const errorMessage = [
68-
'You can\'t set both input & inputPath properties at the',
69-
'same time for schedule events.',
70-
'Please check the AWS docs for more info',
71-
].join('');
72-
throw new this.serverless.classes
73-
.Error(errorMessage);
74-
}
67+
validateEventInput(
68+
Input,
69+
InputPath,
70+
InputTransformer,
71+
this.serverless.classes.Error,
72+
);
7573

7674
if (Input && typeof Input === 'object') {
7775
Input = JSON.stringify(Input);
@@ -151,7 +149,6 @@ module.exports = {
151149
`;
152150

153151
let scheduleTemplate;
154-
let iamRoleTemplate;
155152
// If condition for the event bridge schedular and it define
156153
// resource template and iamrole for the same
157154
if (method === METHOD_SCHEDULER) {
@@ -173,43 +170,6 @@ module.exports = {
173170
}
174171
}
175172
}`;
176-
177-
iamRoleTemplate = `{
178-
"Type": "AWS::IAM::Role",
179-
"Properties": {
180-
"AssumeRolePolicyDocument": {
181-
"Version": "2012-10-17",
182-
"Statement": [
183-
{
184-
"Effect": "Allow",
185-
"Principal": {
186-
"Service": "scheduler.amazonaws.com"
187-
},
188-
"Action": "sts:AssumeRole"
189-
}
190-
]
191-
},
192-
"Policies": [
193-
{
194-
"PolicyName": "${policyName}",
195-
"PolicyDocument": {
196-
"Version": "2012-10-17",
197-
"Statement": [
198-
{
199-
"Effect": "Allow",
200-
"Action": [
201-
"states:StartExecution"
202-
],
203-
"Resource": {
204-
"Ref": "${stateMachineLogicalId}"
205-
}
206-
}
207-
]
208-
}
209-
}
210-
]
211-
}
212-
}`;
213173
} else {
214174
// else condition for the event rule and
215175
// it define resource template and iamrole for the same
@@ -234,62 +194,27 @@ module.exports = {
234194
}]
235195
}
236196
}`;
237-
iamRoleTemplate = `{
238-
"Type": "AWS::IAM::Role",
239-
"Properties": {
240-
"AssumeRolePolicyDocument": {
241-
"Version": "2012-10-17",
242-
"Statement": [
243-
{
244-
"Effect": "Allow",
245-
"Principal": {
246-
"Service": "events.amazonaws.com"
247-
},
248-
"Action": "sts:AssumeRole"
249-
}
250-
]
251-
},
252-
"Policies": [
253-
{
254-
"PolicyName": "${policyName}",
255-
"PolicyDocument": {
256-
"Version": "2012-10-17",
257-
"Statement": [
258-
{
259-
"Effect": "Allow",
260-
"Action": [
261-
"states:StartExecution"
262-
],
263-
"Resource": {
264-
"Ref": "${stateMachineLogicalId}"
265-
}
266-
}
267-
]
268-
}
269-
}
270-
]
271-
}
272-
}`;
273-
}
274-
if (permissionsBoundary) {
275-
const jsonIamRole = JSON.parse(iamRoleTemplate);
276-
jsonIamRole.Properties.PermissionsBoundary = permissionsBoundary;
277-
iamRoleTemplate = JSON.stringify(jsonIamRole);
278197
}
279198

199+
const principalService = method === METHOD_SCHEDULER
200+
? 'scheduler.amazonaws.com'
201+
: 'events.amazonaws.com';
202+
const iamRole = buildIamRole(principalService, policyName, stateMachineLogicalId);
203+
280204
const rolePath = _.get(service, 'provider.iam.role.path');
205+
if (permissionsBoundary) {
206+
iamRole.Properties.PermissionsBoundary = permissionsBoundary;
207+
}
281208
if (rolePath) {
282-
const jsonIamRole = JSON.parse(iamRoleTemplate);
283-
jsonIamRole.Properties.Path = rolePath;
284-
iamRoleTemplate = JSON.stringify(jsonIamRole);
209+
iamRole.Properties.Path = rolePath;
285210
}
286211

287212
const newScheduleObject = {
288213
[scheduleLogicalId]: JSON.parse(scheduleTemplate),
289214
};
290215

291216
const newPermissionObject = event.schedule.role ? {} : {
292-
[scheduleIamRoleLogicalId]: JSON.parse(iamRoleTemplate),
217+
[scheduleIamRoleLogicalId]: iamRole,
293218
};
294219

295220
_.merge(

0 commit comments

Comments
 (0)