does not work on CDK >=2.26 with the @aws-cdk/aws-iam:minimizePolicies feature flag enabled

[gshpychka]

I'm using the package

• iam-floyd
• cdk-iam-floyd

I'm using the package in language

• TypeScript/JavaScript (npm)
• Python (pip)

Describe the problem
After enabling the @aws-cdk/aws-iam:minimizePolicies feature flag my app doesn't synth anymore, throwing an error on synth.

When adding enough statements that policy minimization kicks in, I get the following error:

[...path.../Role/DefaultPolicy] A PolicyStatement must specify at least one 'action' or 'notAction'.
[...path.../Role/DefaultPolicy] A PolicyStatement used in an identity-based policy must specify at least one resource.

cdk-iam-floyd version: 0.382

I am not sure if this is a bug in the feature itself or in cdk-iam-floyd.

[gshpychka]

@udondan I'm struggling to pinpoint a simple enough repro, but the single constant that leads to the error is CDK 2.26+ with the feature flag enabled.

When I comment out some of the added statements to some roles, those roles don't trigger the error, so I'm guessing it only appears when the minimization kicks in.

[udondan]

I'll look into it but it's not going to be soon unfortunately.

Floyd also has a compression feature: .compact() Maybe you can just use this instead for now.

https://iam-floyd.readthedocs.io/en/v0.382.0/vocabulary.html#compact

[gshpychka]

@udondan I can't unfortunately - I have to use the feature flag because of aws/aws-cdk#20565

[udondan]

Since 2.29.0 this is no longer limited to a feature flag.

Unfortunately there seems to be no easy solution. Before 2.26.0 the cdk just called .toJSON() to generate the JSON statement for the CFN template. IAM Floyd overwrote this method to fill the statement props it previously stored in its own propperties.

Now with this policy merging, cdk accesses its props directly without calling any method. So there is nothing I can hook into. Therefore the statement props cannot be filled.

IAM Floyd stores the statement elements internally to be able to do stuff like... well the exact same thing that CDK is doing now: Policy optimization. 😉 So apparently this requires a redesign of how IAM Floyd works. The fact that there are two variants, IAM Floyd and CDK IAM Floyd, doesn't make it easier. Whatever solution is implemented, it also has to work without CDK functionality.

In essence: Until further notice you have to choose between IAM Floyd and CDK >= 2.29.0 (or CDK >= 2.26.0 with the feature flag)

[udondan]

After thinking more about it, I actually see no way how this can work without changing the API of Floyd or removing functionality.

Let me explain the situation in more detail first.

Floyd has a statement minimization feature that can be called via .compact(). It will cook down all actions to the smallest possible patterns. This functionality is very important when working with whole access levels. An example can be found here in the docs at the bottom of the page: https://iam-floyd.readthedocs.io/en/v0.382.0/vocabulary.html#compact - Without the .compact() at the end, the statement generated by the first 4 lines would exceed the policy limit.

To be able to calculate the patterns, all actions need to be stored internally without adding them to the actual statement - since there is no way to later remove or modify them, since the property holding them is private.

Once an action is added, it's added.

As mentioned before, in previous versions, the CDK just called .toJSON() which IAM Floyd hooked into and called the addActions() method for the internally stored actions and/or patterns.

As far as I recall, there is no reason other than consistency, but the same exact thing happens for conditions, resources and principals. So this could be changed, but would not solve the problem at all.

Now with the policy splitting/merging feature introduced via this MR does not call .toJSON() anymore but goes straight to the internal properties to compare them. Since there is nothing to compare, the resulting policy will only contain empty statements.

So the absolutely required hook to commit the statement details is gone. With this MR this now even is always on without any feature flag.

I checked all the changes and I cannot see any methods on the PolicyStatement object called at all, so there is no alternative I could hook into. And I cannot just directly call addActions() since then there is no way to implement Floyds minimization functionality.

I looked into JavaScripts property getter/setter functionality, which I hadn't have a use for until today. 😊 I hoped I might be able to fake a hook when the _action property is accessed. Unfortunately a getter cannot have the same name as an actual property of the class. So this doesn't work.

Where we can go from here:

1. Add a commit() method to Floyd. This would do what the .toJSON() override does (or just call the .toJSON() method) This would mean, it will always have to be called and a statement cannot be modified after that call. Also, all users will need to update their code.

   To stay with the previously linked example, this would look like this:

   new statement.Ec2() 
     .allow()
     .allReadActions()
     .allListActions()
     .compact()
     .commit()

   Now, what about the two package variants? Technically this is only required in the CDK variant. Do we introduce a breaking change in the standalone package and force all users to update their code, just to not introduce drift? If not, how can this be documented? All the examples in the docs apply to both package variants. If there is different functionality this is getting super messy to document.

2. Propose a change to CDK core, to implement a hook. Some empty dummy method that can be hooked into.

   For example, if the original PolicyStatement class would have a commit() method, which can be empty, Floyd could override it and do its magic. This commit method then needs to be called by CDK before it processes statements.

   Not sure how open the CDK team would be about this. I would actually expect them to not consider it at all. The only reason why they might consider it, might be that Floyd is - depending on the day you look at the stats - the most or second most used 3rd party CDK package. And now it's pretty much dead in the water. I assume this will have an impact on many CDK users.

3. [Share your thoughts. I'm out of ideas]

[udondan]

There's a fix now thanks to @rix0rrr

aws/aws-cdk#20911

We can now hook into freeze()