Circular dependency on s3 notification to a destination when both destination and s3 are encrypted by same CMK

[r00t-ankit]

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

• I'm submitting a ...

  • 🪲 bug report

• What is the current behavior?
  If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce

import cdk = require('@aws-cdk/cdk');
import s3 = require('@aws-cdk/aws-s3')
import sqs = require("@aws-cdk/aws-sqs");
import kms = require("@aws-cdk/aws-kms");
import {SqsDestination} from "@aws-cdk/aws-s3-notifications";
import {BucketEncryption} from "@aws-cdk/aws-s3";
import {QueueEncryption} from "@aws-cdk/aws-sqs";

export class TestConstruct extends cdk.Construct {
    constructor(scope: cdk.Construct, id: string) {
        super(scope, id);

        const cmk = new kms.Key(this, 'CDKTest')

        const queue =  new sqs.Queue(this, 'TestQueue', {
            queueName: 'TestQueue',
            encryption: QueueEncryption.Kms,
            encryptionMasterKey: cmk
        })

        const bucket =  new s3.Bucket(this, 'test-ankag-bucket', {
            bucketName: 'test-ankag-bucket',
            encryption: BucketEncryption.Kms,
            encryptionKey: cmk
        })

        bucket.addObjectCreatedNotification(new SqsDestination(queue));
    }
}

Exception:
Circular dependency between resources: [TestConstructCDKTest25F6C8B9, TestConstructTestQueuePolicy8D6FDA03, TestConstructtestankagbucket1D7F9833, TestConstructtestankagbucketNotifications6A969D21, TestConstructTestQueue9EDE46FC]

• What is the expected behavior (or behavior of feature suggested)?

s3 notification should be created to sqs without circular dependency exception

• What is the motivation / use case for changing the behavior or adding this feature?
  This is a bug

• Please tell us about your environment:

  • CDK CLI Version: 0.33
  • Module Version: 0.33
  • OS: [all | Windows 10 | OSX Mojave | Ubuntu | etc... ]
  • Language: [all | TypeScript | Java | Python ] TypeScript

• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

CDK 0.32 is working fine but when upgrading to CDK 0.33 we had to introduce a SqsDestination and thats when we start seeing this issue

[kadishmal]

Is it still failing with 0.35?

[r00t-ankit]

I am not sure if this is failing with 0.35, I tried with 0.34 and it is failing

[moofish32]

@skinny85 is this also fixed in #3694 ?

[skinny85]

@skinny85 is this also fixed in #3694 ?

I'm actually not sure - @r00t-ankit , can you try upgrading to 1.9.0 and see if that helps?

Thanks,
Adam

[stephankaag]

I just ran into this as well. So not solved yet.

    const bucket = new S3.Bucket(this, "Bucket", {
      encryption: S3.BucketEncryption.S3_MANAGED,
    });

    const queue = new SQS.Queue(this, 'Queue', {
      encryption: SQS.QueueEncryption.KMS_MANAGED
    });

    bucket.addEventNotification(
      S3.EventType.OBJECT_CREATED_PUT,
      new S3Notifications.SqsDestination(queue)
    );

[techcoderunner]

I am also facing the similar issue when using the same KMS keys along with s3 notification service. This is very old thread and still open. Anybody has any solutions how to solve this or any alternative mechanism?

[stevenjackson121]

We are facing the same issue: There is no circular problem without encryption, but with encryption we cannot deploy.

[nikevp]

I am also facing the similar issue when using the same KMS keys along with s3 notification service. This is very old thread and still open. Anybody has any solutions how to solve this or any alternative mechanism?

+1

Adding KMS encryption to the destination source introduces the error.

[moofish32]

Have tried using the trust account option for the key? https://docs.aws.amazon.com/cdk/api/latest/docs/aws-kms-readme.html#key-policies

…

On Thu, Apr 22, 2021 at 10:40 AM nikevp ***@***.***> wrote: I am also facing the similar issue when using the same KMS keys along with s3 notification service. This is very old thread and still open. Anybody has any solutions how to solve this or any alternative mechanism? +1 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3067 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFCFWAD3FP24XYZJVHZLH3TKBNQVANCNFSM4H3NIJ5A> .

[berenddeboer]

Have tried using the trust account option for the key?

You mean using trustAccountIdentities: true? That didn't help.

Facing exactly the same issue.

I could only fix this by using a different key for S3 ad for SQS.

[berenddeboer]

BTW, I'm on 1.102.0 (build a75d52f), and this is definitely not fixed.

[antoineaws]

Same issue here, using 1.117.0. Feel free to reach out if you need more info to reproduce. Thanks

[miralgpatel]

Have tried using the trust account option for the key?

You mean using trustAccountIdentities: true? That didn't help.

Facing exactly the same issue.

I could only fix this by using a different key for S3 ad for SQS.

Didn't work for me. Mind sharing your sample code?

[timfulmer]

Still happening for me w/ CDK v2.49.0.

[timfulmer]

Have tried using the trust account option for the key?

You mean using trustAccountIdentities: true? That didn't help.

Facing exactly the same issue.

I could only fix this by using a different key for S3 ad for SQS.

This did not end up working for me either, two different keys, one for s3, another for SQS, still getting the cyclical dependency error.

[timfulmer]

Have tried using the trust account option for the key? https://docs.aws.amazon.com/cdk/api/latest/docs/aws-kms-readme.html#key-policies
…

Yes, this is enabled by default in CDK v2: #18446

[peterwoodworth]

I can synthesize on the latest version using the code posted in the original issue - can someone please provide reproduction steps along with code? Thanks

[timfulmer]

Thank you for taking a look into this @peterwoodworth. The synth step works for me as well. Here is the error on deploy:

:) .../circular_dependency % npx cdk deploy

✨  Synthesis time: 8.74s

CircularDependencyStack: building assets...

[0%] start: Building 6e75a12cb611efb1e7a2c35da8cd74a71057fa091954e1b38c76b535293691c5:current_account-current_region
[100%] success: Built 6e75a12cb611efb1e7a2c35da8cd74a71057fa091954e1b38c76b535293691c5:current_account-current_region

CircularDependencyStack: assets built

This deployment will make potentially sensitive changes according to your current security approval level (--require-approval broadening).
Please confirm you intend to make the following modifications:

IAM Statement Changes
<snip>
(NOTE: There may be security-related changes not in this list. See https://github.com/aws/aws-cdk/issues/1299)

Do you wish to deploy these changes (y/n)? y
CircularDependencyStack: deploying...
[0%] start: Publishing 6e75a12cb611efb1e7a2c35da8cd74a71057fa091954e1b38c76b535293691c5:current_account-current_region
[100%] success: Published 6e75a12cb611efb1e7a2c35da8cd74a71057fa091954e1b38c76b535293691c5:current_account-current_region
CircularDependencyStack: creating CloudFormation changeset...

 ❌  CircularDependencyStack failed: Error [ValidationError]: Circular dependency between resources: [TestQueuePolicyA65327BC, TestQueue6F0069AA, testankagbucketC0AC8B68, testankagbucketNotificationsFB5FDC1E, CDKTestD2D3EE8D]
    at Request.extractError (.../circular_dependency/node_modules/aws-sdk/lib/protocol/query.js:50:29)
    at Request.callListeners (.../circular_dependency/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (.../circular_dependency/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (.../circular_dependency/node_modules/aws-sdk/lib/request.js:686:14)
    at Request.transition (.../circular_dependency/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (.../circular_dependency/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at .../circular_dependency/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (.../circular_dependency/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (.../circular_dependency/node_modules/aws-sdk/lib/request.js:688:12)
    at Request.callListeners (.../circular_dependency/node_modules/aws-sdk/lib/sequential_executor.js:116:18) {
  code: 'ValidationError',
  time: 2022-11-11T17:06:45.236Z,
  requestId: 'a016dfc3-fcfb-42b9-9c7b-de6708fc6dad',
  statusCode: 400,
  retryable: false,
  retryDelay: 643.3626619767707
}

 ❌ Deployment failed: Error: Stack Deployments Failed: ValidationError: Circular dependency between resources: [TestQueuePolicyA65327BC, TestQueue6F0069AA, testankagbucketC0AC8B68, testankagbucketNotificationsFB5FDC1E, CDKTestD2D3EE8D]
    at deployStacks (.../circular_dependency/node_modules/aws-cdk/lib/deploy.ts:61:11)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at CdkToolkit.deploy (.../circular_dependency/node_modules/aws-cdk/lib/cdk-toolkit.ts:315:7)
    at initCommandLine (.../circular_dependency/node_modules/aws-cdk/lib/cli.ts:358:12)

Stack Deployments Failed: ValidationError: Circular dependency between resources: [TestQueuePolicyA65327BC, TestQueue6F0069AA, testankagbucketC0AC8B68, testankagbucketNotificationsFB5FDC1E, CDKTestD2D3EE8D]
:( .../circular_dependency % 

[peterwoodworth]

Whoops 🤦🏻 My mistake. Thought we check for that at synth

This is happening because when you add an object created notification, you create a policy on the key which references the bucket in a condition. The bucket already depends on the key, so, the circular dependency is created.

Using the reproduction code from the OP, this is the exact statement on the KeyPolicy which gets rendered:

      {
       "Action": [
        "kms:Decrypt",
        "kms:Encrypt",
        "kms:GenerateDataKey*",
        "kms:ReEncrypt*"
       ],
       "Condition": {
        "ArnLike": {
         "aws:SourceArn": {
          "Fn::GetAtt": [
           "testankagbucketC0AC8B68",
           "Arn"
          ]
         }
        }
       },
       "Effect": "Allow",
       "Principal": {
        "Service": "s3.amazonaws.com"
       },
       "Resource": "*"
      },

You can add this line of code to alter the Condition if you know in advance what the bucket arn will be:
(cmk.node.defaultChild as CfnKey).addPropertyOverride('KeyPolicy.Statement.1.Condition', ...);. Note that the 1 is the index in the Statement array, this number may be different on your key resource

Alternatively, a second way to achieve this is to use an AwsCustomResource at the end of stack creation to make the PutKeyPolicy API call. You'll need to configure permissions for this call to be authorized (probably with an escape hatch to override the KeyPolicy), but I would expect this to eliminate the circular dependency.

[timfulmer]

Amazing, thank you @peterwoodworth, adding this works:

    (cmk.node.defaultChild as CfnKey).addPropertyOverride(
      `KeyPolicy.Statement.1.Condition`,
      {
        ArnLike: {
          'aws:SourceArn': 'arn:aws:s3:::test-ankag-bucket',
        },
      },
    );