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

!! NOTICE: "ValidationError: Circular dependency between resources" for SecretsManager RotationSchedule with rotationLambda #14868

Closed
skinny85 opened this issue May 25, 2021 · 4 comments
Assignees
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. management/tracking Issues that track a subject or multiple issues p0

Comments

@skinny85
Copy link
Contributor

skinny85 commented May 25, 2021

Please add your +1 👍 to let us know you have encountered this

Status: IN-PROGRESS

Overview:

When using a RotationSchedule with a rotationLambda, there is a circular dependency that happens between the default IAM Policy of the rotationLambda Role, and the Function itself:

import { Construct, Stack, StackProps } from '@aws-cdk/core';
import * as lambda from '@aws-cdk/aws-lambda';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';

export class TestStack extends Stack {
    constructor(scope: Construct, id: string, props?: StackProps) {
        super(scope, id, props);

        const secret = new secretsmanager.Secret(this, 'Secret');
        const rotationLambda = new lambda.Function(this, 'RotateLambda', {
            runtime: lambda.Runtime.NODEJS_10_X,
            code: lambda.Code.fromInline('export.handler = event => event;'),
            handler: 'index.handler',
        });
        new secretsmanager.RotationSchedule(this, 'RotationSchedule', {
            secret,
            rotationLambda: rotationLambda,
        });
    }
}

Complete Error Message:

$ yarn cdk deploy
yarn run v1.22.10
TestStack: deploying...
TestStack: creating CloudFormation changeset...

 ❌  TestStack failed: ValidationError: Circular dependency between resources: [RotateLambdaCCE05B95, RotationSchedule2A725270, RotateLambdaServiceRoleDefaultPolicy12636DDF, RotateLambdaInvokeServicePrincipalsecretsmanageramazonawscom32ADD300]
Circular dependency between resources: [RotateLambdaCCE05B95, RotationSchedule2A725270, RotateLambdaServiceRoleDefaultPolicy12636DDF, RotateLambdaInvokeServicePrincipalsecretsmanageramazonawscom32ADD300]

Workaround:

This regression was introduced in CDK version 1.104.0, so if you use a version of CDK 1.103.0 or earlier, you will not be affected.

Solution:

Related Issues:

This was added in #14471.

@skinny85 skinny85 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 25, 2021
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label May 25, 2021
@skinny85 skinny85 pinned this issue May 25, 2021
@skinny85 skinny85 assigned skinny85 and unassigned njlynch May 25, 2021
@skinny85 skinny85 added management/tracking Issues that track a subject or multiple issues p0 and removed needs-triage This issue or PR still needs to be triaged. labels May 25, 2021
@skinny85
Copy link
Contributor Author

Revert of the faulty PR: #14869

@skinny85
Copy link
Contributor Author

PR with the patch release: #14881

workeitel added a commit to workeitel/aws-cdk that referenced this issue May 26, 2021
…nArn

The policy needs to restrict which secrets the lambda function is
allowed to access/rotate. The official documentation recommends usage of
the `secretsmanager:resource/AllowRotationLambdaArn` context key and any
secret (`"Resource": "*"`):

> Filters the request based on the ARN of the Lambda rotation function
attached to the target resource of the request. This enables you to
restrict access to only those secrets with a rotation Lambda ARN
matching this value. Secrets without rotation enabled or with a
different rotation Lambda ARN do not match.

https://docs.aws.amazon.com/secretsmanager/latest/userguide/rotating-secrets-required-permissions.html#rotating-secrets-required-permissions-user-vs-function
https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_iam-permissions.html#iam-contextkeys

But CDK automatically adds a `DependsOn` on the role and default policy
to prevent race condition when creating policy and lambda in parallel
which caused a circular dependency aws#14868.

By removing the condition the circular dependency is gone but since the
policy declares the target secret as well it was anyway redundant. Other
than the documentation above, which only used the condition but allowed
rotation for all secrets (`resources: "*"`).
@skinny85
Copy link
Contributor Author

The fix has been released in CDK version 1.106.1. Resolving.

@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.

mergify bot pushed a commit that referenced this issue Jun 17, 2021
…bda (#14882)

This reverts the revert #14868, adds a fix and an integration test to ensure it doesn't happen again.

The policy needs to restrict which secrets the lambda function is allowed to access/rotate. The official documentation recommends usage of the `secretsmanager:resource/AllowRotationLambdaArn` context key and any secret (`"Resource": "*"`):

> Filters the request based on the ARN of the Lambda rotation function attached to the target resource of the request. This enables you to restrict access to only those secrets with a rotation Lambda ARN matching this value. Secrets without rotation enabled or with a different rotation Lambda ARN do not match.

https://docs.aws.amazon.com/secretsmanager/latest/userguide/rotating-secrets-required-permissions.html#rotating-secrets-required-permissions-user-vs-function
https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_iam-permissions.html#iam-contextkeys

But CDK automatically adds a `DependsOn` on the role and default policy to prevent race condition when creating policy and lambda in parallel which caused a circular dependency #14868.

By removing the condition the circular dependency is gone but since the policy declares the target secret as well it was anyway redundant. Other than the documentation above, which only used the condition but allowed rotation for all secrets (`resources: "*"`).



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
matthewsvu pushed a commit to matthewsvu/aws-cdk that referenced this issue Jun 22, 2021
…bda (aws#14882)

This reverts the revert aws#14868, adds a fix and an integration test to ensure it doesn't happen again.

The policy needs to restrict which secrets the lambda function is allowed to access/rotate. The official documentation recommends usage of the `secretsmanager:resource/AllowRotationLambdaArn` context key and any secret (`"Resource": "*"`):

> Filters the request based on the ARN of the Lambda rotation function attached to the target resource of the request. This enables you to restrict access to only those secrets with a rotation Lambda ARN matching this value. Secrets without rotation enabled or with a different rotation Lambda ARN do not match.

https://docs.aws.amazon.com/secretsmanager/latest/userguide/rotating-secrets-required-permissions.html#rotating-secrets-required-permissions-user-vs-function
https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_iam-permissions.html#iam-contextkeys

But CDK automatically adds a `DependsOn` on the role and default policy to prevent race condition when creating policy and lambda in parallel which caused a circular dependency aws#14868.

By removing the condition the circular dependency is gone but since the policy declares the target secret as well it was anyway redundant. Other than the documentation above, which only used the condition but allowed rotation for all secrets (`resources: "*"`).



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@otaviomacedo otaviomacedo unpinned this issue Jun 28, 2021
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…bda (aws#14882)

This reverts the revert aws#14868, adds a fix and an integration test to ensure it doesn't happen again.

The policy needs to restrict which secrets the lambda function is allowed to access/rotate. The official documentation recommends usage of the `secretsmanager:resource/AllowRotationLambdaArn` context key and any secret (`"Resource": "*"`):

> Filters the request based on the ARN of the Lambda rotation function attached to the target resource of the request. This enables you to restrict access to only those secrets with a rotation Lambda ARN matching this value. Secrets without rotation enabled or with a different rotation Lambda ARN do not match.

https://docs.aws.amazon.com/secretsmanager/latest/userguide/rotating-secrets-required-permissions.html#rotating-secrets-required-permissions-user-vs-function
https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_iam-permissions.html#iam-contextkeys

But CDK automatically adds a `DependsOn` on the role and default policy to prevent race condition when creating policy and lambda in parallel which caused a circular dependency aws#14868.

By removing the condition the circular dependency is gone but since the policy declares the target secret as well it was anyway redundant. Other than the documentation above, which only used the condition but allowed rotation for all secrets (`resources: "*"`).



----

*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
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. management/tracking Issues that track a subject or multiple issues p0
Projects
None yet
Development

No branches or pull requests

2 participants