aws_s3: cyclic dependency introduced for cross-stack bucket use

[ashemedai]

Describe the bug

I think PR #23386 (2.59.0+) inadvertently introduced a cyclic dependency for cross-stack use of S3 buckets. Using the suggested way for cross-stack bucket use, as per this link, results in cdk synth failing with a cyclic reference error.

Using the same set up with 2.58.1 does not cause any cyclic reference problems.

Expected Behavior

Shared stack with central access logs S3 bucket created for use in other stacks. Other stack(s) then can be created and depend on said stack and refer to the appropriate bucket in their own serverAccessLogsBucket configuration for writing their access logs to. A not uncommon scenario, I would think.

Current Behavior

Dependent stacks are being added as a dependency to the original stack, causing a cyclic dependency.

Error: 'S3SharedStack' depends on 'S3CycleStack' (S3SharedStack -> S3CycleStack/my-main-bucket/Resource.Arn). Adding this dependency (S3CycleStack -> S3SharedStack/access-logs/Resource.Ref) would create a cyclic reference.
    at S3CycleStack._addAssemblyDependency (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/stack.js:1:9650)
    at operateOnDependency (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/deps.js:1:1637)
    at Object.addDependency (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/deps.js:1:321)
    at S3CycleStack.addDependency (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/stack.js:1:6809)
    at resolveValue (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/private/refs.js:1:3223)
    at Object.resolveReferences (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/private/refs.js:1:870)
    at Object.prepareApp (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/private/prepare-app.js:1:565)
    at Object.synthesize (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:606)
    at App.synth (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/stage.js:1:1922)
    at process.<anonymous> (/home/asmodai/projects/s3-cycle/node_modules/aws-cdk-lib/core/lib/app.js:1:1157)

Reproduction Steps

mkdir s3-cycle
cdk init app -l typescript

bin/s3-cycle.ts

#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import { S3SharedStack } from '../lib/s3-shared-stack';
import { S3CycleStack } from '../lib/s3-cycle-stack';

const app = new cdk.App();

const s3SharedStack = new S3SharedStack(app, "S3SharedStack", {
})

new S3CycleStack(app, 'S3CycleStack', {
  accessLogsBucket: s3SharedStack.accessLogsBucket
});

lib/s3-shared-stack.ts

import * as cdk from 'aws-cdk-lib';
import { aws_s3 as s3 } from 'aws-cdk-lib';
import { Construct } from 'constructs';

export class S3SharedStack extends cdk.Stack {
  public readonly accessLogsBucket: s3.Bucket;

  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const sharedAccessLogsBucket = new s3.Bucket(this, `access-logs`, {
      objectOwnership: s3.ObjectOwnership.BUCKET_OWNER_ENFORCED,
    });

    this.accessLogsBucket = sharedAccessLogsBucket;
  }
}

lib/s3-cycle-stack.ts

import * as cdk from 'aws-cdk-lib';
import { aws_s3 as s3 } from 'aws-cdk-lib';
import { Construct } from 'constructs';

export interface SharedProps extends cdk.StackProps {
  accessLogsBucket: s3.IBucket,
}

export class S3CycleStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props: SharedProps) {
    super(scope, id, props);

    new s3.Bucket(this, `my-main-bucket`, {
      serverAccessLogsBucket: props.accessLogsBucket,
      serverAccessLogsPrefix: 'some-unique-prefix',
    })
  }
}

Possible Solution

Unsure right now, still trying to figure out what is actually being made dependent.

Additional Information/Context

When test cases are added, it makes sense to add cross-stack tests as well. Not just the happy path all-in-one-stack tests.

CDK CLI Version

2.59.0 (build b24095d)

Framework Version

2.59.0

Node.js Version

Node.js v18.12.1

OS

Ubuntu 22.04.1 LTS

Language

Typescript

Language Version

TypeScript 4.9.4

Other information

PR #23386 as mentioned earlier.

[kylelaker]

I believe this was fixed in #23552 which should be included in the next release of the CDK. And as suggested, that does include test cases for the cross-stack references, so hopefully this doesn't regress again!

[kylelaker]

The specific issue was that the Bucket Policy of the imported target log bucket depended on attributes of the current stack’s bucket (the account and its ARN); and that bucket then depended on the target log bucket (to configure log delivery)

[ashemedai]

@kylelaker Thanks for the heads up! Good to know, was still digging into the code, but not too familiar with aws-cdk's codebase yet, so figured I at least report it. Hadn't checked the latest commits though.

[kylelaker]

@ashemedai I think the necessary fix landed in v2.61.0, which was released very recently. Would you mind giving that a try and seeing if it resolves your issue?

[ashemedai]

@kylelaker Just ran my set up and it works as it should with 2.61.0. Thanks!

[github-actions]

⚠️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.