Skip to content

Commit a0f6bd2

Browse files
danrivettdanrivett
committed
fix: added support for generating S3 permissions using Bucket references
Resolves #647
1 parent 7df8af4 commit a0f6bd2

File tree

2 files changed

+312
-10
lines changed

2 files changed

+312
-10
lines changed

‎lib/deploy/stepFunctions/compileIamRole.js

Lines changed: 29 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,27 @@ 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+
'arn:aws:s3:::${bucket}',
668+
'arn:aws:s3:::${bucket}/*',
669+
]),
651670
},
652671
{
653672
action: 's3:List*',
654-
resource: [
655-
`arn:aws:s3:::${bucket}`,
656-
`arn:aws:s3:::${bucket}/*`,
657-
],
673+
resource: resolveS3BucketReferences(bucket, [
674+
'arn:aws:s3:::${bucket}',
675+
'arn:aws:s3:::${bucket}/*',
676+
]),
658677
},
659678
];
660679
}
661680

662681
if (prefix) {
663-
arn = `arn:aws:s3:::${bucket}/${prefix}/${key}`;
682+
arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${prefix}/${key}`);
664683
} else if (bucket === '*' && key === '*') {
665684
arn = '*';
666685
} else {
667-
arn = `arn:aws:s3:::${bucket}/${key}`;
686+
arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${key}`);
668687
}
669688

670689
return [{

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

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

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

0 commit comments

Comments
 (0)