Skip to content

Commit f6a7d9c

Browse files
horike37horike37
authored
Merge pull request #569 from aligent/support-s3-putobject-task
Support s3 putobject task
2 parents 6cd7097 + b163d81 commit f6a7d9c

File tree

2 files changed

+244
-20
lines changed

2 files changed

+244
-20
lines changed

‎lib/deploy/stepFunctions/compileIamRole.js‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ function getTaskStates(states, stateMachineName) {
3232
if (state.ItemReader) {
3333
taskStates.push(state.ItemReader);
3434
}
35+
if (state.ResultWriter) {
36+
taskStates.push(state.ResultWriter);
37+
}
3538
return taskStates;
3639
}
3740
default: {
@@ -429,22 +432,24 @@ function getEventBridgePermissions(state) {
429432
];
430433
}
431434

432-
function getS3GetObjectPermissions(state) {
433-
const bucket = state.Parameters['Bucket.$'] ? state.Parameters['Bucket.$'] : state.Parameters.Bucket;
434-
const key = state.Parameters['Key.$'] ? state.Parameters['Key.$'] : state.Parameters.Key;
435+
function getS3ObjectPermissions(action, state) {
436+
const bucket = state.Parameters.Bucket || '*';
437+
const key = state.Parameters.Key || '*';
438+
const prefix = state.Parameters.Prefix;
439+
let arn;
435440

436-
if (bucket.startsWith('$') && key.startsWith('$')) {
437-
return [{
438-
action: 's3:GetObject',
439-
resource: [
440-
'*',
441-
],
442-
}];
441+
if (prefix) {
442+
arn = `arn:aws:s3:::${bucket}/${prefix}/${key}`;
443+
} else if (bucket === '*' && key === '*') {
444+
arn = '*';
445+
} else {
446+
arn = `arn:aws:s3:::${bucket}/${key}`;
443447
}
448+
444449
return [{
445-
action: 's3:GetObject',
450+
action,
446451
resource: [
447-
`arn:aws:s3:::${bucket}/${key}`,
452+
arn,
448453
],
449454
}];
450455
}
@@ -564,7 +569,10 @@ function getIamPermissions(taskStates) {
564569

565570
case 'arn:aws:states:::s3:getObject':
566571
case 'arn:aws:states:::aws-sdk:s3:getObject':
567-
return getS3GetObjectPermissions(state);
572+
return getS3ObjectPermissions('s3:GetObject', state);
573+
case 'arn:aws:states:::s3:putObject':
574+
case 'arn:aws:states:::aws-sdk:s3:putObject':
575+
return getS3ObjectPermissions('s3:PutObject', state);
568576

569577
default:
570578
if (isIntrinsic(state.Resource) || !!state.Resource.match(/arn:aws(-[a-z]+)*:lambda/)) {

‎lib/deploy/stepFunctions/compileIamRole.test.js‎

Lines changed: 223 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,7 +1452,7 @@ describe('#compileIamRole', () => {
14521452
expectDenyAllPolicy(policy);
14531453
});
14541454

1455-
it('should give s3:GetObject permission for only objects referenced by state machine', () => {
1455+
it('should give s3 permissions for only objects referenced by state machine', () => {
14561456
const hello = 'hello.txt';
14571457
const world = 'world.txt';
14581458
const testBucket = 'test-bucket';
@@ -1469,6 +1469,16 @@ describe('#compileIamRole', () => {
14691469
Bucket: bucket,
14701470
Key: key,
14711471
},
1472+
Next: 'B',
1473+
},
1474+
B: {
1475+
Type: 'Task',
1476+
Resource: 'arn:aws:states:::aws-sdk:s3:putObject',
1477+
Parameters: {
1478+
Bucket: bucket,
1479+
Key: key,
1480+
Body: {},
1481+
},
14721482
End: true,
14731483
},
14741484
},
@@ -1491,11 +1501,20 @@ describe('#compileIamRole', () => {
14911501
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${hello}`]);
14921502
expect(policy2.PolicyDocument.Statement[0].Resource)
14931503
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${world}`]);
1504+
1505+
[policy1, policy2].forEach((policy) => {
1506+
expect(policy.PolicyDocument.Statement[0].Action)
1507+
.to.be.deep.equal([
1508+
's3:GetObject',
1509+
's3:PutObject',
1510+
]);
1511+
});
14941512
});
14951513

14961514
it('should give s3:GetObject permission for only objects referenced by state machine with ItemReader', () => {
1515+
const hello = 'hello.txt';
1516+
const world = 'world.txt';
14971517
const testBucket = 'test-bucket';
1498-
const testKey = 'test-key';
14991518

15001519
const genStateMachine = (id, lambdaArn, bucket, key) => ({
15011520
id,
@@ -1517,8 +1536,8 @@ describe('#compileIamRole', () => {
15171536
ItemReader: {
15181537
Resource: 'arn:aws:states:::s3:getObject',
15191538
Parameters: {
1520-
'Bucket.$': bucket,
1521-
'Key.$': key,
1539+
Bucket: bucket,
1540+
Key: key,
15221541
},
15231542
},
15241543
End: true,
@@ -1530,9 +1549,9 @@ describe('#compileIamRole', () => {
15301549
serverless.service.stepFunctions = {
15311550
stateMachines: {
15321551
myStateMachine1: genStateMachine('StateMachine1',
1533-
'arn:aws:lambda:us-west-2:1234567890:function:foo', '$.testBucket', '$.testKey'),
1552+
'arn:aws:lambda:us-west-2:1234567890:function:foo', testBucket, hello),
15341553
myStateMachine2: genStateMachine('StateMachine2',
1535-
'arn:aws:lambda:us-west-2:1234567890:function:foo', testBucket, testKey),
1554+
'arn:aws:lambda:us-west-2:1234567890:function:foo', testBucket, world),
15361555
},
15371556
};
15381557

@@ -1541,10 +1560,207 @@ describe('#compileIamRole', () => {
15411560
.provider.compiledCloudFormationTemplate.Resources;
15421561
const policy1 = resources.StateMachine1Role.Properties.Policies[0];
15431562
const policy2 = resources.StateMachine2Role.Properties.Policies[0];
1563+
expect(policy1.PolicyDocument.Statement[1].Resource)
1564+
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${hello}`]);
1565+
expect(policy2.PolicyDocument.Statement[1].Resource)
1566+
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${world}`]);
1567+
});
1568+
1569+
it('should give s3:GetObject permission to * when Bucket.$ and Key.$ are seen on ItemReader', () => {
1570+
const genStateMachine = (id, lambdaArn) => ({
1571+
id,
1572+
definition: {
1573+
StartAt: 'A',
1574+
States: {
1575+
A: {
1576+
Type: 'Map',
1577+
ItemProcessor: {
1578+
StartAt: 'B',
1579+
States: {
1580+
B: {
1581+
Type: 'Task',
1582+
Resource: lambdaArn,
1583+
End: true,
1584+
},
1585+
},
1586+
},
1587+
ItemReader: {
1588+
Resource: 'arn:aws:states:::s3:getObject',
1589+
Parameters: {
1590+
Bucket: 'test-bucket',
1591+
Key: 'test-key',
1592+
},
1593+
},
1594+
Next: 'C',
1595+
},
1596+
C: {
1597+
Type: 'Map',
1598+
ItemProcessor: {
1599+
StartAt: 'D',
1600+
States: {
1601+
D: {
1602+
Type: 'Task',
1603+
Resource: lambdaArn,
1604+
End: true,
1605+
},
1606+
},
1607+
},
1608+
ItemReader: {
1609+
Resource: 'arn:aws:states:::s3:getObject',
1610+
Parameters: {
1611+
'Bucket.$': '$.testBucket',
1612+
'Key.$': '$.key',
1613+
},
1614+
},
1615+
End: true,
1616+
},
1617+
},
1618+
},
1619+
});
1620+
1621+
serverless.service.stepFunctions = {
1622+
stateMachines: {
1623+
myStateMachine1: genStateMachine('StateMachine1',
1624+
'arn:aws:lambda:us-west-2:1234567890:function:foo'),
1625+
},
1626+
};
1627+
1628+
serverlessStepFunctions.compileIamRole();
1629+
const resources = serverlessStepFunctions.serverless.service
1630+
.provider.compiledCloudFormationTemplate.Resources;
1631+
const policy1 = resources.StateMachine1Role.Properties.Policies[0];
1632+
1633+
// even though some tasks target specific values, other states use Bucket.$
1634+
// and Key.$ so we need to give broad permissions to be able to get any
1635+
// bucket and key the input specifies
15441636
expect(policy1.PolicyDocument.Statement[1].Resource)
15451637
.to.be.deep.equal('*');
1638+
});
1639+
1640+
it('should give s3:PutObject permission for only objects referenced by state machine with ResultWriter', () => {
1641+
const hello = 'hello';
1642+
const world = 'world';
1643+
const testBucket = 'test-bucket';
1644+
1645+
const genStateMachine = (id, lambdaArn, bucket, prefix) => ({
1646+
id,
1647+
definition: {
1648+
StartAt: 'A',
1649+
States: {
1650+
A: {
1651+
Type: 'Map',
1652+
ItemProcessor: {
1653+
StartAt: 'B',
1654+
States: {
1655+
B: {
1656+
Type: 'Task',
1657+
Resource: lambdaArn,
1658+
End: true,
1659+
},
1660+
},
1661+
},
1662+
ResultWriter: {
1663+
Resource: 'arn:aws:states:::s3:putObject',
1664+
Parameters: {
1665+
Bucket: bucket,
1666+
Prefix: prefix,
1667+
},
1668+
},
1669+
End: true,
1670+
},
1671+
},
1672+
},
1673+
});
1674+
1675+
serverless.service.stepFunctions = {
1676+
stateMachines: {
1677+
myStateMachine1: genStateMachine('StateMachine1',
1678+
'arn:aws:lambda:us-west-2:1234567890:function:foo', testBucket, hello),
1679+
myStateMachine2: genStateMachine('StateMachine2',
1680+
'arn:aws:lambda:us-west-2:1234567890:function:foo', testBucket, world),
1681+
},
1682+
};
1683+
1684+
serverlessStepFunctions.compileIamRole();
1685+
const resources = serverlessStepFunctions.serverless.service
1686+
.provider.compiledCloudFormationTemplate.Resources;
1687+
const policy1 = resources.StateMachine1Role.Properties.Policies[0];
1688+
const policy2 = resources.StateMachine2Role.Properties.Policies[0];
1689+
expect(policy1.PolicyDocument.Statement[1].Resource)
1690+
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${hello}/*`]);
15461691
expect(policy2.PolicyDocument.Statement[1].Resource)
1547-
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${testKey}`]);
1692+
.to.be.deep.equal([`arn:aws:s3:::${testBucket}/${world}/*`]);
1693+
});
1694+
1695+
it('should give s3:PutObject permission to * when Bucket.$ and Prefix.$ are seen on ResultWriter', () => {
1696+
const genStateMachine = (id, lambdaArn) => ({
1697+
id,
1698+
definition: {
1699+
StartAt: 'A',
1700+
States: {
1701+
A: {
1702+
Type: 'Map',
1703+
ItemProcessor: {
1704+
StartAt: 'B',
1705+
States: {
1706+
B: {
1707+
Type: 'Task',
1708+
Resource: lambdaArn,
1709+
End: true,
1710+
},
1711+
},
1712+
},
1713+
ResultWriter: {
1714+
Resource: 'arn:aws:states:::s3:putObject',
1715+
Parameters: {
1716+
Bucket: 'test-bucket',
1717+
Prefix: 'test-prefix',
1718+
},
1719+
},
1720+
Next: 'C',
1721+
},
1722+
C: {
1723+
Type: 'Map',
1724+
ItemProcessor: {
1725+
StartAt: 'D',
1726+
States: {
1727+
D: {
1728+
Type: 'Task',
1729+
Resource: lambdaArn,
1730+
End: true,
1731+
},
1732+
},
1733+
},
1734+
ResultWriter: {
1735+
Resource: 'arn:aws:states:::s3:putObject',
1736+
Parameters: {
1737+
'Bucket.$': '$.testBucket',
1738+
'Prefix.$': '$.prefix',
1739+
},
1740+
},
1741+
End: true,
1742+
},
1743+
},
1744+
},
1745+
});
1746+
1747+
serverless.service.stepFunctions = {
1748+
stateMachines: {
1749+
myStateMachine1: genStateMachine('StateMachine1',
1750+
'arn:aws:lambda:us-west-2:1234567890:function:foo'),
1751+
},
1752+
};
1753+
1754+
serverlessStepFunctions.compileIamRole();
1755+
const resources = serverlessStepFunctions.serverless.service
1756+
.provider.compiledCloudFormationTemplate.Resources;
1757+
const policy1 = resources.StateMachine1Role.Properties.Policies[0];
1758+
1759+
// even though some tasks target specific values, other states use Bucket.$
1760+
// and Prefix.$ so we need to give broad permissions to be able to write to
1761+
// any bucket and prefix the input specifies
1762+
expect(policy1.PolicyDocument.Statement[1].Resource)
1763+
.to.be.deep.equal('*');
15481764
});
15491765

15501766
it('should not generate any permissions for Task states not yet supported', () => {

0 commit comments

Comments
 (0)