aws / aws/aws-cdk

aws_iam: Role.splitLargePolicy aspect breaks some other aspects

Open
#21,046 1 comment 1 reaction 1 assignee Claimed by @rix0rrr View on GitHub
@aws-cdk/aws-iam bug effort/medium p1
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the bug

Because the `splitLargePolicy` aspect has the side effect of freezing all PolicyStatements it touches, it can break later-running aspects that wish to mutate PolicyStatements.

Example: Let's imagine that I maintain a library that includes a construct to provide a standardized REST api for some common SQS Queue maintenance items. This construct uses an Aspect to find all the queues in the stack at `prepare` phase and grant itself the required permissions to perform its function (since fishing out a reference to every single queue in a deep stack tree may be difficult/impossible, depending on the user). For a construct like this, PolicyStatements going immutable before my aspect gets to them becomes a problem.

### Expected Behavior

PolicyStatements remain mutable through the end of the `prepare` phase, so they can freely be modified by Aspects.

### Current Behavior

Aspects running after the `splitLargePolicy` now raise an exception like:
```jsii.errors.JSIIError: addResources: freeze() has been called on this PolicyStatement previously, so it can no longer be modified```

### Reproduction Steps

```python
import jsii
import constructs
from aws_cdk import IAspect, Stack, Aspects
from aws_cdk.aws_iam import PolicyStatement, Effect
from aws_cdk.aws_lambda import Function, Runtime, Code
from aws_cdk.aws_sqs import Queue

class DefaultStack(Stack):
def __init__(self, scope, construct_id: str, **kwargs):
super().__init__(scope, construct_id, **kwargs)
QueueApi(self, 'MyQueueApi')
# Some Queue buried deep in the construct tree
Queue(self, 'MyQueue')

@jsii.implements(IAspect)
class QueueApi(constructs.Construct):
"""
Creates a .../queue API Resource that allows moving messages from one queue
to another within this stack.
"""
def __init__(self, scope: constructs.Construct, id: str):
super().__init__(scope, id)
# Pretend this is a GrantPrincipal from a Function, integrated into an API Gateway Method
function = Function(
self, 'QueueFunction',
handler='index.handler',
runtime=Runtime.NODEJS_14_X,
code=Code.from_asset('lambda')
)
self.policy_statement = PolicyStatement(
effect=Effect.ALLOW,
actions=[
'sqs:SendMessage',
'sqs:ChangeMessageVisibility',
'sqs:DeleteMessage',
'sqs:ReceiveMessage',
'sqs:GetQueueAttributes',
'sqs:GetQueueUrl'
],
resources=[]
)
# Add one statement with needed actions
function.grant_principal.add_to_principal_policy(self.policy_statement)
# Then add each Queue we find in the stack via the Aspect
Aspects.of(Stack.of(self)).add(self)

def visit(self, node: Queue):
"""
Find all sqs.Queue constructs and grant send/consume to our Function
"""
if not isinstance(node, Queue):
return

self.policy_statement.add_resources(node.queue_arn)
print(f'visit {node.node.path}')

```

### Possible Solution

This one is tough. To me, it seems like cdk itself should not be making normally mutable constructs suddenly immutable during the `prepare` phase, since the whole idea of Aspects is (and the documentation suggests) that users should be able to depend on constructs remaining mutable during the `prepare` phase, (unless their own code explicitly does otherwise, I suppose). I would guess that moving this policy splitting into a later phase would be a huge undertaking, though. Alternatively, all I can think of is is that maybe we need some additional control over the order that Aspects execute so that we can deliberately reduce the chances of an Aspect clash of this kind.

### Additional Information/Context

_No response_

### CDK CLI Version

2.30+

### Framework Version

_No response_

### Node.js Version

16.6.2

### OS

AL2

### Language

Python

### Language Version

Python 3.10.4

### Other information

```jsii.errors.JavaScriptError:
Error: addResources: freeze() has been called on this PolicyStatement previously, so it can no longer be modified
at KernelHost.completeCallback (/tmp/tmpwr1ura_s/lib/program.js:9489:35)
at KernelHost.processRequest (/tmp/tmpwr1ura_s/lib/program.js:9535:24)
at KernelHost.completeCallback (/tmp/tmpwr1ura_s/lib/program.js:9493:33)
at KernelHost.processRequest (/tmp/tmpwr1ura_s/lib/program.js:9535:24)
at KernelHost.completeCallback (/tmp/tmpwr1ura_s/lib/program.js:9493:33)
at KernelHost.callbackHandler (/tmp/tmpwr1ura_s/lib/program.js:9480:41)
at Construct.value (/tmp/tmpwr1ura_s/lib/program.js:8337:49)
at recurse (/tmp/jsii-kernel-og6OV1/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:1799)
at recurse (/tmp/jsii-kernel-og6OV1/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:2127)
at recurse (/tmp/jsii-kernel-og6OV1/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:2127)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/home/user/dev/cdk-split-policy-bug/cdk/app.py", line 12, in
app.synth()
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/aws_cdk/__init__.py", line 16926, in synth
return typing.cast(_CloudAssembly_c693643e, jsii.invoke(self, "synth", [options]))
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 143, in wrapped
return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 363, in invoke
return _callback_till_result(self, response, InvokeResponse)
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 231, in _callback_till_result
response = kernel.sync_complete(
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 401, in sync_complete
return self.provider.sync_complete(
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/providers/process.py", line 382, in sync_complete
resp = self._process.send(_CompleteRequest(complete=request), response_type)
File "/home/user/src/pyenv/versions/bug/lib/python3.10/site-packages/jsii/_kernel/providers/process.py", line 326, in send
raise JSIIError(resp.error) from JavaScriptError(resp.stack)
jsii.errors.JSIIError: addResources: freeze() has been called on this PolicyStatement previously, so it can no longer be modified
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.