Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

secretsmanager: secret resource policy already exists in stack #24383

Comments

@mrgrain
Copy link
Contributor

mrgrain commented Feb 28, 2023

Describe the bug

When attempting to deploy the following app, the Stack deployment fails in CloudFormation with the error:

 arn:aws:secretsmanager:us-east-1:<****accountId****>:secret:<secret_ID> already exists in stack arn:aws:cloudformation:us-east-1:<****accountId****>:stack/<stack_ID>
const vpc = ec2.Vpc.fromLookup(this , 'vpc', {
  vpcId: 'vpc-011221242122'

})
const secret = new rds.DatabaseSecret(this, 'testsecret', {
  username: 'clusteradmin',
});

const cluster = new rds.ServerlessCluster(this, 'AuroraCluster', {
  engine: rds.DatabaseClusterEngine.AURORA_MYSQL,
  vpc,
  credentials: rds.Credentials.fromSecret(secret),
  clusterIdentifier: 'db-endpoint-test1',
  defaultDatabaseName: 'demos1',

});

secret.addRotationSchedule('testschedule',{
  hostedRotation: secretsmanager.HostedRotation.mysqlSingleUser(),
});

cluster.grantDataApiAccess(new iam.AccountRootPrincipal());
cluster.grantDataApiAccess(new iam.ServicePrincipal("ecs-tasks.amazonaws.com "));

Detailed description

The issue occurs when a SecretAttachmentTarget is created and policy statements are added both via the SecretAttachmentTarget and the original Secret. The generated CloudFormation template will contain two ResourcePolicies for the same secret, which is rejected by CloudFormation during deployment.

The behavior is prevalent with Database Constructs (RDS, DocDB, Redshift) which internally relay on SecretAttachmentTarget. A common scenario (the code snippet above) is that a new Secret is created and has a rotation schedule added to it. The Secret is then passed to a Database, which internally creates a SecretAttachmentTarget. Later on, a Grantee is given access to the Database. This grant includes read permissions to the Database Credentials - which are provided via a SecretAttachmentTarget causing the above scenario.

Workaround

When using Secrets with Database Constructs, it is recommended to always use the same object instance to grant permissions to the Secret. Since Database Constructs use their internal instance, user's should always refer to the Secret via the Database Construct.

For example in the code snippet above, instead of

secret.addRotationSchedule('testschedule',{ ... })

one should use the Secret via the Cluster:

cluster?.secret.addRotationSchedule('testschedule',{ ... })

Solution

With v2.67.0 we have introduced a new feature flag @aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments to solve this issue.

Please see our developer guide for instructions on how to use feature flags.

New apps

Make sure your app is using aws-cdk-lib of version v2.67.0 or above and has the @aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments feature flag set to true.

No further change is required.

Existing Apps without without any AWS::SecretsManager::ResourcePolicy resources

Feature flags can only be set at an App level. Please check all Stacks that are part of this App for any usage of AWS::SecretsManager::ResourcePolicy.

If none is found, upgrade aws-cdk-lib to version v2.67.0 or above and set the @aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments feature flag totrue.

No further change is required, but make sure the carefully check the output of cdk diff before the next deployment. If you encounter any replacements of AWS::SecretsManager::ResourcePolicy, please follow the steps in the next section.

Existing Apps with AWS::SecretsManager::ResourcePolicy resources

If any AWS::SecretsManager::ResourcePolicy resource are used, a manual intervention is likely required. To check, do the following steps:

  1. Start by upgrade aws-cdk-lib to version v2.67.0 or above and set the @aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments feature flag totrue.

  2. Run cdk diff
    Only if the output contains replacements of AWS::SecretsManager::ResourcePolicy similar to the example below, you need to take manual steps outlined in the next section. Otherwise continue with your deployment as usual.

Resources
[-] AWS::SecretsManager::ResourcePolicy InstanceSecretAttachmentPolicy60A8B8DE destroy
[+] AWS::SecretsManager::ResourcePolicy InstanceSecretPolicy87F03D0F 

Manual Replacement of AWS::SecretsManager::ResourcePolicy

If you were to continue with this deployment, you will be running into the root cause of this issue: CloudFormation failing the deployment due to an attempt of two AWS::SecretsManager::ResourcePolicy resources being created. Unfortunately this is the case even though one of the resources is scheduled for deletion. You have two options to resolve the issue:

Temporarily remove AWS::SecretsManager::ResourcePolicy from Stacks (recommended)

A replacement of the resource in a single deployment is not possible. However, we can manually re-create the replacement by following these general steps:

  1. Remove all AWS::SecretsManager::ResourcePolicy from any Stacks
    One-by-one remove any code that adds statements to a AWS::SecretsManager::ResourcePolicy. These statements likely start with grant or deny, but also includes adding rotation schedules.
    Be careful not to remove any stateful resources (like the Secret itself or Databases).
  2. Deploy the App
    You should see all AWS::SecretsManager::ResourcePolicy being removed from the Stack
  3. Re-add AWS::SecretsManager::ResourcePolicy
    Revert your code to the previous state.
  4. Deploy the App again
    You should see equivalent AWS::SecretsManager::ResourcePolicy resources added to the Stack

Please note this will temporarily remove permissions granted to the Secret via a ResourcePolicy and might cause issues with downstream services relying on these permissions. If this is the case, you might want to use the alternative method referred to in the next section.

Rename Logical ID of affected resources

Partly this issue occurs because the change of the AWS::SecretsManager::ResourcePolicy resource is regarded as a replacement by CDK & CloudFormation (which doesn't work). The CDK includes a mechanism to enforce an update of the resource instead (which does work).

To achieve this, we need to instruct the Stack containing the resource to re-use the original logical ID, using the renameLogicalId() method:

Resources
[-] AWS::SecretsManager::ResourcePolicy InstanceSecretAttachmentPolicy60A8B8DE destroy
[+] AWS::SecretsManager::ResourcePolicy InstanceSecretPolicy87F03D0F 

For the above cdk diff output, you would need to add the following statement to your app:

// stack.renameLogicalId(logicalIdToBeReplaced, originalLogicalId);
stack.renameLogicalId('InstanceSecretPolicy87F03D0F', 'InstanceSecretAttachmentPolicy60A8B8DE');

Repeat this for every replacement of AWS::SecretsManager::ResourcePolicy.

This approach is less preferable because it leaves unnecessary artifacts behind in your code which might cause further manual changes in future (e.g. if the resource is removed). However it can be an alternative approach if temporarily removing the ResourcePolicy is not possible.

CDK Framework Version

All version <= v2.66.1

@mergify mergify bot closed this as completed in #24365 Feb 28, 2023
mergify bot pushed a commit that referenced this issue Feb 28, 2023
…under feature flag) (#24365)

The issue is prevalent with Database clusters, e.g. when a secret with a rotation is used and a grantee is granted access to the db cluster.

The root cause of this issue is the Cluster's use of a SecretAttachmentTarget to replace the original secret. This is allowed, since SecretAttachmentTarget do implement the ISecret interface. When policy statements are added, a new resource policy is created using the SecretAttachmentTarget ARN. This only works because in CloudFormation SecretAttachmentTarget returns the ARN of the Secret. However a secret can only have a single resource policy. The above scenario leads to a policy being created for the Secret and one for the SecretAttachmentTarget. Which is then (correctly) rejected by CloudFormation.

The fix is to forward any policy changes on the SecretAttachmentTarget to the attached secret.

Fixes #24383

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
…under feature flag) (aws#24365)

The issue is prevalent with Database clusters, e.g. when a secret with a rotation is used and a grantee is granted access to the db cluster.

The root cause of this issue is the Cluster's use of a SecretAttachmentTarget to replace the original secret. This is allowed, since SecretAttachmentTarget do implement the ISecret interface. When policy statements are added, a new resource policy is created using the SecretAttachmentTarget ARN. This only works because in CloudFormation SecretAttachmentTarget returns the ARN of the Secret. However a secret can only have a single resource policy. The above scenario leads to a policy being created for the Secret and one for the SecretAttachmentTarget. Which is then (correctly) rejected by CloudFormation.

The fix is to forward any policy changes on the SecretAttachmentTarget to the attached secret.

Fixes aws#24383

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment