fix: ensure the admin role stabilizes for self-managed stacksets

[martinjlowm]

Looks like #439 tackles the same problem, however, I think DependsOn is sufficient and is typically what is used in aws-cdk-lib, e.g. https://github.com/aws/aws-cdk/blob/0546ec24c5fa60cae41c6561d7178de7a403a713/packages/%40aws-cdk/aws-kinesisanalytics-flink-alpha/lib/application.ts#L1030

For anyone looking for a workaround with the current release, you can add the dependency like so:

if (selfManagedStackSet.role) {
  selfManagedStackSet.node.addDependency(selfManagedStackSet.role);
}

fixes #438, supersedes #439

[kaizencc]

Hi @martinjlowm I agree that this is the correct way to approach the issue. Can you add a unit test to be safe? I can't accept this without some sort of testing change. If you really cannot, I will add this to my list of things to do but that may incur a delay :)

The unit test can be super simple kind of like this:

https://github.com/aws/aws-cdk/blob/d48d77aa1adbddf27c94e279ee756bdc2c2d18cf/packages/%40aws-cdk/app-staging-synthesizer-alpha/test/app-staging-synthesizer.test.ts#L111-L123

[tabrezm]

AFAIK, it's not typical for a CDK resource to modify an existing role by adding a policy, which is what the current code does. That's why this proposed fix made it conditional on whether the role is new or existing.

[martinjlowm]

@kaizencc - a month later and here we go. I hope checking DependsOn will suffice ✌️

[alexpkarlsson]

Any updates on this?

AFAIK, it's not typical for a CDK resource to modify an existing role by adding a policy, which is what the current code does. That's why this proposed fix made it conditional on whether the role is new or existing.

Also in relation to this it would be great if it was also possible to conditionally skip adding a policy to the role if it's a pre-created one.

This is due to us preferably wanting to use a predefined role that is used to multiple StackSets which will potentially run into hitting the limit of 10 attached policies.

Not sure what the best way to handle this would be but could imagine just having an optional flag sent into DeploymentType.selfManaged()

Such as

DeploymentType.selfManaged({
  adminRole: role,
  executionRoleName: 'FooBar',
  attachRolePolicy: false,
})