Skip to content

Commit a755b0c

Browse files
danrivettdanrivett
authored
fix: added support for generating S3 permissions using Bucket references (#648)
Resolves #647
1 parent 7df8af4 commit a755b0c

File tree

2 files changed

+310
-10
lines changed

2 files changed

+310
-10
lines changed

‎lib/deploy/stepFunctions/compileIamRole.js

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,25 @@ function getEventBridgeSchedulerPermissions(state) {
634634
];
635635
}
636636

637+
// Because the S3 Bucket parameter can be either a literal name or a reference to an
638+
// existing S3 bucket we need to resolve
639+
function resolveS3BucketReference(bucket, resource) {
640+
if (isIntrinsic(bucket)) {
641+
return {
642+
'Fn::Sub': [
643+
resource,
644+
{ bucket },
645+
],
646+
};
647+
}
648+
649+
return resource.replaceAll('${bucket}', bucket);
650+
}
651+
652+
function resolveS3BucketReferences(bucket, resources) {
653+
return resources.map(resource => resolveS3BucketReference(bucket, resource));
654+
}
655+
637656
function getS3ObjectPermissions(action, state) {
638657
const bucket = state.Parameters.Bucket || '*';
639658
const key = state.Parameters.Key || '*';
@@ -644,27 +663,29 @@ function getS3ObjectPermissions(action, state) {
644663
return [
645664
{
646665
action: 's3:Get*',
647-
resource: [
648-
`arn:aws:s3:::${bucket}`,
649-
`arn:aws:s3:::${bucket}/*`,
650-
],
666+
resource: resolveS3BucketReferences(bucket, [
667+
// The bucket variable is not interpolated here, or below,
668+
// but inside resolveS3BucketReferences() to also handle intrinsics
669+
'arn:aws:s3:::${bucket}',
670+
'arn:aws:s3:::${bucket}/*',
671+
]),
651672
},
652673
{
653674
action: 's3:List*',
654-
resource: [
655-
`arn:aws:s3:::${bucket}`,
656-
`arn:aws:s3:::${bucket}/*`,
657-
],
675+
resource: resolveS3BucketReferences(bucket, [
676+
'arn:aws:s3:::${bucket}',
677+
'arn:aws:s3:::${bucket}/*',
678+
]),
658679
},
659680
];
660681
}
661682

662683
if (prefix) {
663-
arn = `arn:aws:s3:::${bucket}/${prefix}/${key}`;
684+
arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${prefix}/${key}`);
664685
} else if (bucket === '*' && key === '*') {
665686
arn = '*';
666687
} else {
667-
arn = `arn:aws:s3:::${bucket}/${key}`;
688+
arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${key}`);
668689
}
669690

670691
return [{

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

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,6 +2148,285 @@ describe('#compileIamRole', () => {
21482148
.to.be.deep.equal('*');
21492149
});
21502150

2151+
it('should resolve literal bucket names in S3 permissions', () => {
2152+
const literalBucket = 'my-test-bucket';
2153+
const literalKey = 'test-key.txt';
2154+
2155+
serverless.service.stepFunctions = {
2156+
stateMachines: {
2157+
myStateMachine1: {
2158+
id: 'StateMachine1',
2159+
definition: {
2160+
StartAt: 'A',
2161+
States: {
2162+
A: {
2163+
Type: 'Task',
2164+
Resource: 'arn:aws:states:::aws-sdk:s3:getObject',
2165+
Parameters: {
2166+
Bucket: literalBucket,
2167+
Key: literalKey,
2168+
},
2169+
End: true,
2170+
},
2171+
},
2172+
},
2173+
},
2174+
},
2175+
};
2176+
2177+
serverlessStepFunctions.compileIamRole();
2178+
const statements = serverlessStepFunctions.serverless.service.provider
2179+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2180+
.PolicyDocument.Statement;
2181+
2182+
expect(statements[0].Resource[0]).to.equal(`arn:aws:s3:::${literalBucket}/${literalKey}`);
2183+
});
2184+
2185+
it('should resolve CloudFormation reference buckets in S3 permissions', () => {
2186+
const bucketRef = { Ref: 'MyS3Bucket' };
2187+
2188+
serverless.service.stepFunctions = {
2189+
stateMachines: {
2190+
myStateMachine1: {
2191+
id: 'StateMachine1',
2192+
definition: {
2193+
StartAt: 'A',
2194+
States: {
2195+
A: {
2196+
Type: 'Task',
2197+
Resource: 'arn:aws:states:::aws-sdk:s3:getObject',
2198+
Parameters: {
2199+
Bucket: bucketRef,
2200+
Key: 'test-key.txt',
2201+
},
2202+
End: true,
2203+
},
2204+
},
2205+
},
2206+
},
2207+
},
2208+
};
2209+
2210+
serverlessStepFunctions.compileIamRole();
2211+
const statements = serverlessStepFunctions.serverless.service.provider
2212+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2213+
.PolicyDocument.Statement;
2214+
2215+
expect(statements[0].Resource[0]).to.deep.equal({
2216+
'Fn::Sub': [
2217+
'arn:aws:s3:::${bucket}/test-key.txt',
2218+
{ bucket: bucketRef },
2219+
],
2220+
});
2221+
});
2222+
2223+
it('should resolve Fn::GetAtt bucket references in S3 permissions', () => {
2224+
const bucketRef = { 'Fn::GetAtt': ['MyS3Bucket', 'Arn'] };
2225+
2226+
serverless.service.stepFunctions = {
2227+
stateMachines: {
2228+
myStateMachine1: {
2229+
id: 'StateMachine1',
2230+
definition: {
2231+
StartAt: 'A',
2232+
States: {
2233+
A: {
2234+
Type: 'Task',
2235+
Resource: 'arn:aws:states:::aws-sdk:s3:putObject',
2236+
Parameters: {
2237+
Bucket: bucketRef,
2238+
Key: 'output.json',
2239+
Body: { result: 'success' },
2240+
},
2241+
End: true,
2242+
},
2243+
},
2244+
},
2245+
},
2246+
},
2247+
};
2248+
2249+
serverlessStepFunctions.compileIamRole();
2250+
const statements = serverlessStepFunctions.serverless.service.provider
2251+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2252+
.PolicyDocument.Statement;
2253+
2254+
expect(statements[0].Resource[0]).to.deep.equal({
2255+
'Fn::Sub': [
2256+
'arn:aws:s3:::${bucket}/output.json',
2257+
{ bucket: bucketRef },
2258+
],
2259+
});
2260+
});
2261+
2262+
it('should resolve bucket references in S3 listObjectsV2 permissions', () => {
2263+
const bucketRef = { Ref: 'MyS3Bucket' };
2264+
2265+
serverless.service.stepFunctions = {
2266+
stateMachines: {
2267+
myStateMachine1: {
2268+
id: 'StateMachine1',
2269+
definition: {
2270+
StartAt: 'A',
2271+
States: {
2272+
A: {
2273+
Type: 'Map',
2274+
ItemProcessor: {
2275+
ProcessorConfig: {
2276+
Mode: 'DISTRIBUTED',
2277+
},
2278+
},
2279+
StartAt: 'B',
2280+
States: {
2281+
B: {
2282+
Type: 'Task',
2283+
Resource: 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:hello',
2284+
End: true,
2285+
},
2286+
},
2287+
ItemReader: {
2288+
Resource: 'arn:aws:states:::s3:listObjectsV2',
2289+
Parameters: {
2290+
Bucket: bucketRef,
2291+
Prefix: 'data',
2292+
},
2293+
},
2294+
End: true,
2295+
},
2296+
},
2297+
},
2298+
},
2299+
},
2300+
};
2301+
2302+
serverlessStepFunctions.compileIamRole();
2303+
const statements = serverlessStepFunctions.serverless.service.provider
2304+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2305+
.PolicyDocument.Statement;
2306+
2307+
expect(statements[3].Resource[0]).to.deep.equal({
2308+
'Fn::Sub': [
2309+
'arn:aws:s3:::${bucket}',
2310+
{ bucket: bucketRef },
2311+
],
2312+
});
2313+
expect(statements[3].Resource[1]).to.deep.equal({
2314+
'Fn::Sub': [
2315+
'arn:aws:s3:::${bucket}/*',
2316+
{ bucket: bucketRef },
2317+
],
2318+
});
2319+
});
2320+
2321+
it('should handle mixed literal and reference buckets correctly', () => {
2322+
const literalBucket = 'literal-bucket';
2323+
const bucketRef = { Ref: 'ReferenceBucket' };
2324+
const literalFile = 'file1.txt';
2325+
2326+
serverless.service.stepFunctions = {
2327+
stateMachines: {
2328+
myStateMachine1: {
2329+
id: 'StateMachine1',
2330+
definition: {
2331+
StartAt: 'A',
2332+
States: {
2333+
A: {
2334+
Type: 'Task',
2335+
Resource: 'arn:aws:states:::aws-sdk:s3:getObject',
2336+
Parameters: {
2337+
Bucket: literalBucket,
2338+
Key: literalFile,
2339+
},
2340+
Next: 'B',
2341+
},
2342+
B: {
2343+
Type: 'Task',
2344+
Resource: 'arn:aws:states:::aws-sdk:s3:putObject',
2345+
Parameters: {
2346+
Bucket: bucketRef,
2347+
Key: 'file2.txt',
2348+
Body: {},
2349+
},
2350+
End: true,
2351+
},
2352+
},
2353+
},
2354+
},
2355+
},
2356+
};
2357+
2358+
serverlessStepFunctions.compileIamRole();
2359+
const statements = serverlessStepFunctions.serverless.service.provider
2360+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2361+
.PolicyDocument.Statement;
2362+
2363+
// Check that we have consolidated permissions
2364+
expect(statements).to.have.lengthOf(2);
2365+
2366+
// First statement for s3:GetObject with literal bucket
2367+
expect(statements[0].Action).to.deep.equal(['s3:GetObject']);
2368+
expect(statements[0].Resource[0]).to.equal(`arn:aws:s3:::${literalBucket}/${literalFile}`);
2369+
2370+
// Second statement for s3:PutObject with reference bucket
2371+
expect(statements[1].Action).to.deep.equal(['s3:PutObject']);
2372+
expect(statements[1].Resource[0]).to.deep.equal({
2373+
'Fn::Sub': [
2374+
'arn:aws:s3:::${bucket}/file2.txt',
2375+
{ bucket: bucketRef },
2376+
],
2377+
});
2378+
});
2379+
2380+
it('should handle bucket references with prefixes correctly', () => {
2381+
const bucketRef = { Ref: 'MyS3Bucket' };
2382+
2383+
serverless.service.stepFunctions = {
2384+
stateMachines: {
2385+
myStateMachine1: {
2386+
id: 'StateMachine1',
2387+
definition: {
2388+
StartAt: 'A',
2389+
States: {
2390+
A: {
2391+
Type: 'Map',
2392+
ItemProcessor: {
2393+
StartAt: 'B',
2394+
States: {
2395+
B: {
2396+
Type: 'Task',
2397+
Resource: 'arn:aws:lambda:us-west-2:1234567890:function:foo',
2398+
End: true,
2399+
},
2400+
},
2401+
},
2402+
ResultWriter: {
2403+
Resource: 'arn:aws:states:::s3:putObject',
2404+
Parameters: {
2405+
Bucket: bucketRef,
2406+
Prefix: 'results',
2407+
},
2408+
},
2409+
End: true,
2410+
},
2411+
},
2412+
},
2413+
},
2414+
},
2415+
};
2416+
2417+
serverlessStepFunctions.compileIamRole();
2418+
const statements = serverlessStepFunctions.serverless.service.provider
2419+
.compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0]
2420+
.PolicyDocument.Statement;
2421+
2422+
expect(statements[1].Resource[0]).to.deep.equal({
2423+
'Fn::Sub': [
2424+
'arn:aws:s3:::${bucket}/results/*',
2425+
{ bucket: bucketRef },
2426+
],
2427+
});
2428+
});
2429+
21512430
it('should not generate any permissions for Task states not yet supported', () => {
21522431
const genStateMachine = id => ({
21532432
id,

0 commit comments

Comments
 (0)