diff --git a/fixtures/basic-state-machine/serverless.yml b/fixtures/basic-state-machine/serverless.yml index 46dacc24..6c36193c 100644 --- a/fixtures/basic-state-machine/serverless.yml +++ b/fixtures/basic-state-machine/serverless.yml @@ -24,3 +24,13 @@ stepFunctions: Resource: Fn::GetAtt: [HelloLambdaFunction, Arn] End: true + fnSubMachine: + name: integration-basic-fn-sub-${opt:stage, 'test'} + definition: + StartAt: InvokeLambda + States: + InvokeLambda: + Type: Task + Resource: + Fn::Sub: "arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-fn" + End: true diff --git a/fixtures/basic-state-machine/verify.test.js b/fixtures/basic-state-machine/verify.test.js new file mode 100644 index 00000000..51e569c1 --- /dev/null +++ b/fixtures/basic-state-machine/verify.test.js @@ -0,0 +1,73 @@ +'use strict'; + +const fs = require('node:fs'); +const path = require('node:path'); +const expect = require('chai').expect; + +const templatePath = path.join(__dirname, '.serverless', 'cloudformation-template-update-stack.json'); + +describe('basic-state-machine fixture — CloudFormation template', () => { + let resources; + + before(() => { + const template = JSON.parse(fs.readFileSync(templatePath, 'utf8')); + resources = template.Resources; + }); + + it('should have an IAM role for each state machine', () => { + const stateMachineRoles = Object.values(resources).filter( + (r) => r.Type === 'AWS::IAM::Role' + && JSON.stringify(r).includes('states.'), + ); + expect(stateMachineRoles).to.have.lengthOf(2); + }); + + it('should grant lambda:InvokeFunction for a Fn::GetAtt resource reference', () => { + // basicMachine uses Fn::GetAtt: [HelloLambdaFunction, Arn] — the generated + // role should allow invoking that function and its aliases/versions (:*) + const roles = Object.values(resources).filter((r) => r.Type === 'AWS::IAM::Role'); + const statements = roles.flatMap((r) => r.Properties.Policies[0].PolicyDocument.Statement); + const lambdaStatement = statements.find((s) => { + const actions = [].concat(s.Action); + return actions.includes('lambda:InvokeFunction'); + }); + expect(lambdaStatement, 'should have a lambda:InvokeFunction statement').to.not.equal(undefined); + const arnList = [].concat(lambdaStatement.Resource); + const hasGetAtt = arnList.some((a) => a && a['Fn::GetAtt']); + expect(hasGetAtt, 'should reference the Lambda function via Fn::GetAtt').to.equal(true); + }); + + it('should not produce nested Fn::Sub when Resource is a Fn::Sub expression (issue #302)', () => { + // fnSubMachine uses a Fn::Sub ARN (simulating serverless-pseudo-parameters output). + // The versioned form (:*) must not nest a Fn::Sub inside a Fn::Sub variable map — + // that is invalid CloudFormation and causes MalformedPolicyDocument errors. + const roles = Object.values(resources).filter((r) => r.Type === 'AWS::IAM::Role'); + const statements = roles.flatMap((r) => r.Properties.Policies[0].PolicyDocument.Statement); + const allArns = statements + .filter((s) => [].concat(s.Action).includes('lambda:InvokeFunction')) + .flatMap((s) => [].concat(s.Resource)); + + for (const arn of allArns) { + if (arn && typeof arn === 'object' && Array.isArray(arn['Fn::Sub'])) { + const [, varMap] = arn['Fn::Sub']; + if (varMap) { + for (const val of Object.values(varMap)) { + expect(val, 'Fn::Sub variable map must not contain a nested Fn::Sub').to.not.have.property('Fn::Sub'); + } + } + } + } + }); + + it('should include a valid versioned ARN (:*) for the Fn::Sub resource', () => { + const fnSubTemplate = 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-fn'; + const roles = Object.values(resources).filter((r) => r.Type === 'AWS::IAM::Role'); + const statements = roles.flatMap((r) => r.Properties.Policies[0].PolicyDocument.Statement); + const allArns = statements + .filter((s) => [].concat(s.Action).includes('lambda:InvokeFunction')) + .flatMap((s) => [].concat(s.Resource)); + + const versionedArn = allArns.find((a) => a && a['Fn::Sub'] === `${fnSubTemplate}:*`); + expect(versionedArn, 'versioned ARN should be a Fn::Sub string with :* appended').to.not.equal(undefined); + }); +}); diff --git a/fixtures/package-lock.json b/fixtures/package-lock.json index 8d78043c..fff7739c 100644 --- a/fixtures/package-lock.json +++ b/fixtures/package-lock.json @@ -13,7 +13,7 @@ }, "..": { "name": "serverless-step-functions", - "version": "3.28.2", + "version": "3.29.0", "dev": true, "license": "MIT", "dependencies": { diff --git a/lib/deploy/stepFunctions/iamStrategies/lambda.js b/lib/deploy/stepFunctions/iamStrategies/lambda.js index fa7157da..4a99b177 100644 --- a/lib/deploy/stepFunctions/iamStrategies/lambda.js +++ b/lib/deploy/stepFunctions/iamStrategies/lambda.js @@ -85,6 +85,19 @@ function getPermissions(state, { plugin }) { }]; } +function getVersionedArn(functionArn) { + if (_.has(functionArn, 'Fn::Sub')) { + const sub = functionArn['Fn::Sub']; + if (typeof sub === 'string') { + return { 'Fn::Sub': `${sub}:*` }; + } + if (Array.isArray(sub)) { + return { 'Fn::Sub': [`${sub[0]}:*`, sub[1]] }; + } + } + return { 'Fn::Sub': ['${functionArn}:*', { functionArn }] }; +} + function getFallbackPermissions(state, { plugin }) { if (isIntrinsic(state.Resource) || !!state.Resource.match(/arn:aws(-[a-z]+)*:lambda/)) { const trimmedArn = trimAliasFromLambdaArn(state.Resource); @@ -93,7 +106,7 @@ function getFallbackPermissions(state, { plugin }) { action: 'lambda:InvokeFunction', resource: [ functionArn, - { 'Fn::Sub': ['${functionArn}:*', { functionArn }] }, + getVersionedArn(functionArn), ], }]; } diff --git a/lib/deploy/stepFunctions/iamStrategies/lambda.test.js b/lib/deploy/stepFunctions/iamStrategies/lambda.test.js index 922a3e7c..295313bd 100644 --- a/lib/deploy/stepFunctions/iamStrategies/lambda.test.js +++ b/lib/deploy/stepFunctions/iamStrategies/lambda.test.js @@ -307,4 +307,43 @@ describe('lambda strategy — getFallbackPermissions (direct lambda ARN task res const result = getFallbackPermissions(state, { plugin }); expect(result).to.deep.equal([]); }); + + it('should not produce a nested Fn::Sub when Resource is a Fn::Sub expression (issue #302)', () => { + // serverless-pseudo-parameters converts #{AWS::Region} → ${AWS::Region} and wraps + // the whole ARN in Fn::Sub. Putting that object as a variable value inside another + // Fn::Sub array is invalid CloudFormation and causes MalformedPolicyDocument errors. + const state = { + Type: 'Task', + Resource: { + 'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-fn', + }, + }; + const plugin = makePlugin(); + const result = getFallbackPermissions(state, { plugin }); + expect(result[0].action).to.equal('lambda:InvokeFunction'); + const arnList = result[0].resource; + for (const arn of arnList) { + if (arn && typeof arn === 'object' && Array.isArray(arn['Fn::Sub'])) { + const [, varMap] = arn['Fn::Sub']; + if (varMap) { + for (const val of Object.values(varMap)) { + expect(val, 'variable map value must not be a nested Fn::Sub').to.not.have.property('Fn::Sub'); + } + } + } + } + }); + + it('should generate a valid versioned ARN when Resource is a Fn::Sub string', () => { + const fnSub = 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-fn'; + const state = { + Type: 'Task', + Resource: { 'Fn::Sub': fnSub }, + }; + const plugin = makePlugin(); + const result = getFallbackPermissions(state, { plugin }); + const arnList = result[0].resource; + const versionedArn = arnList.find((a) => a && a['Fn::Sub'] === `${fnSub}:*`); + expect(versionedArn, 'versioned ARN should be a simple Fn::Sub string with :* appended').to.not.equal(undefined); + }); });