(aws-iam): Policy names from different stacks collide because Logical ID is used as policy name, not Physical ID

[SamStephens]

Describe the bug

When two stacks both import a role with the same Logical ID, and both try and add policies to that role, both stacks will use the same name for the policy, and the policy from one stack will overwrite the other.

Expected Behavior

I'd expect the IAM policy to use the Physical ID of the Cloudformation resource as its name. The Physical IDs differ across the two stacks.

Current Behavior

The IAM policy uses the Logical ID of the Cloudformation resource as its name. The Logical IDs are the same across both stacks. Hence one overwrites the other.

Reproduction Steps

Here's the app.py for three stacks that demonstrate this problem. After deploying all three stacks, you'll see that the role has a single policy, that only provides permissions to one of the two lambdas.

#!/usr/bin/env python3

from aws_cdk import aws_lambda, aws_iam, CfnOutput, Stack, Fn, App
from constructs import Construct

class RoleStack(Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        test_role = aws_iam.Role(
            scope=self,
            id="TestRole",
            role_name="TestRole",
            assumed_by=aws_iam.ServicePrincipal('lambda.amazonaws.com'),
        )

        CfnOutput(
            scope=self,
            id="TestRoleName",
            export_name="TestRoleName",
            value=test_role.role_name,
        )

class RoleImportConstruct(Construct):
    def __init__(self,
                 scope: Construct,
                 construct_id: str,
                 **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        self.test_role = aws_iam.Role.from_role_name(
            scope=self,
            id="TestRole",
            role_name=Fn.import_value('TestRoleName'),
        )

class Demo1Stack(Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        role_import_construct = RoleImportConstruct(
            scope=self,
            construct_id="RoleImportConstruct",
        )

        fake_lambda = aws_lambda.Function.from_function_name(
            scope=self,
            id="FakeLambda1",
            function_name="FakeLambda1",
        )

        fake_lambda.grant_invoke(role_import_construct.test_role)

class Demo2Stack(Stack):
    def __init__(self,
                 scope: Construct,
                 construct_id: str,
                 **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        role_import_construct = RoleImportConstruct(
            scope=self,
            construct_id="RoleImportConstruct",
        )

        fake_lambda = aws_lambda.Function.from_function_name(
            scope=self,
            id="FakeLambda2",
            function_name="FakeLambda2",
        )

        fake_lambda.grant_invoke(role_import_construct.test_role)

app = App()

role_stack = RoleStack(app, "RoleStack")
demo_1_stack = Demo1Stack(app, "Demo1Stack")
demo_2_stack = Demo1Stack(app, "Demo2Stack")
demo_1_stack.add_dependency(role_stack)
demo_2_stack.add_dependency(role_stack)

app.synth()

Possible Solution

I'm not sure what the right solution is here, because: (a) I think this is a Cloudformation bug, rather than a CDK bug; (b) changing the Policy resource to use the Logical ID rather than the Physical ID would be a breaking change for existing stacks.

Additional Information/Context

The similarity to https://github.com/aws/aws-cdk/issues/8742 is worth considering, where we see basically the same issue with Lambda Layers from different stacks overwriting each other.

CDK CLI Version

2.45.0 (build af1fb7c)

Framework Version

2.45.0

Node.js Version

v16.15.1

OS

Ubuntu (Windows Subsystem for Linux)

Language

Python

Language Version

Python 3.9.7

Other information

No response

[SamStephens]

Thinking about this, it may be worth auditing for any other places where the Logical ID is used as the resource name, or to construct the Physical ID of the resource as Lambda Layers do. Any resource for which the Logical ID is used as the resource name is going to be subject to this issue.

Is the underlying issue here that the Stack name is not used as part of the Logical ID; because Stack names are guaranteed to be unique?

[rix0rrr]

Yes.

We will have to switch to using Names.uniqueResourceName (and put that change behind a feature flag).