-
Notifications
You must be signed in to change notification settings - Fork 4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(core):
stack.urlSuffix
is no longer scoped (#4011)
`stack.urlSuffix` used to be a scoped Token. By the use of roles defined in another stack (using a `ServicePrincipal` which uses a `urlSuffix` token), this could lead to unintentional stack references. Two changes here: * URL Suffix (seems to) only change when Partition changes. Since Partition is unscoped (cross-partition references won't work anyway), we might as well make URL Suffix unscoped too. * `ServicePrincipalToken` should not have used the stack's `urlSuffix`, but constructed an unscoped `URL_SUFFIX` itself, since it was never intended to potentially create a cross-stack reference. It couldn't have, since it doesn't know where it is being defined, it just knows where it's being used. Technically, the second change isn't necessary anymore after we apply the first, but I made both anyway since the bug is still resolved even if find out we need roll back the first change because of a future region build. Fixes #3970.
- Loading branch information
1 parent
f63bf6f
commit 82e08bc
Showing
4 changed files
with
74 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import '@aws-cdk/assert/jest'; | ||
import { App, CfnOutput, Stack } from '@aws-cdk/core'; | ||
import iam = require('../lib'); | ||
|
||
test('use of cross-stack role reference does not lead to URLSuffix being exported', () => { | ||
// GIVEN | ||
const app = new App(); | ||
const first = new Stack(app, 'First'); | ||
const second = new Stack(app, 'Second'); | ||
|
||
// WHEN | ||
const role = new iam.Role(first, 'Role', { | ||
assumedBy: new iam.ServicePrincipal('s3.amazonaws.com') | ||
}); | ||
|
||
new CfnOutput(second, 'Output', { | ||
value: role.roleArn | ||
}); | ||
|
||
// THEN | ||
app.synth(); | ||
|
||
expect(first).toMatchTemplate({ | ||
Resources: { | ||
Role1ABCC5F0: { | ||
Type: "AWS::IAM::Role", | ||
Properties: { | ||
AssumeRolePolicyDocument: { | ||
Statement: [ | ||
{ | ||
Action: "sts:AssumeRole", | ||
Effect: "Allow", | ||
Principal: { Service: "s3.amazonaws.com" } | ||
} | ||
], | ||
Version: "2012-10-17" | ||
} | ||
} | ||
} | ||
}, | ||
Outputs: { | ||
ExportsOutputFnGetAttRole1ABCC5F0ArnB4C0B73E: { | ||
Value: { "Fn::GetAtt": [ "Role1ABCC5F0", "Arn" ] }, | ||
Export: { | ||
Name: "First:ExportsOutputFnGetAttRole1ABCC5F0ArnB4C0B73E" | ||
} | ||
} | ||
} | ||
} | ||
); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters