awslabs / aws-solutions-constructs

The AWS Solutions Constructs Library is an open-source extension of the AWS Cloud Development Kit (AWS CDK) that provides multi-service, well-architected patterns for quickly defining solutions
https://docs.aws.amazon.com/solutions/latest/constructs/
Apache License 2.0
1.19k stars 240 forks source link

CloudFrontToS3 error when providing existing s3 bucket #1114

Open maximedemarey-wb opened 2 months ago

maximedemarey-wb commented 2 months ago

When providing an existingBucketObj to the CloudFrontToS3 construct, I get an error :

Reproduction Steps

    this.s3Bucket = new s3.Bucket(this, 'FrontendBucket', {
      bucketName: `test`,
      encryption: s3.BucketEncryption.S3_MANAGED,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
    });
const cloudfrontS3 = new CloudFrontToS3(
      this,
      `cloudfront-s3`,
      {
        cloudFrontDistributionProps: {
        ...
        },
        existingBucketObj: props.s3Bucket,
      }
    );

Error Log

if (resource.cfnOptions.metadata?.cfn_nag?.rules_to_suppress) {
                              ^
TypeError: Cannot read properties of undefined (reading 'metadata')
    at Object.addCfnSuppressRules
    at Object.createCloudFrontDistributionForS3

Environment

Other


This is :bug: Bug Report

biffgaut commented 2 months ago

Thanks, we'll take a look.

maximedemarey-wb commented 2 months ago

I just found a clue, the problem only seems to be present when the s3 bucket is created in a CDK stack different from the cloudfrontToS3 stack

biffgaut commented 2 months ago

How is the external bucket shared with the stack containing the aws-cloudfront-s3 construct?

maximedemarey-wb commented 2 months ago

it is shared via a parameter in the cloufront stack constructor :

const deployS3BucketStack = new DeployBucketStack(
    ...
  );

  new CloudFrontStack(app, `CloudFrontStack`, {
    ...
    s3Bucket: deployS3BucketStack.s3Bucket,
  });
biffgaut commented 2 months ago

I'm attempting to replicate your situation in my own client with the following 3 source files:

first-stack.ts

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

export class FirstStack extends cdk.NestedStack {
  public readonly remoteBucket: s3.Bucket;

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

    this.remoteBucket = new s3.Bucket(this, 'FirstStackBucket', {
      bucketName: "sc-issue1114-remote-bucket",
      encryption: s3.BucketEncryption.S3_MANAGED,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
      removalPolicy: cdk.RemovalPolicy.DESTROY
    });
  }
}

second-stack.ts

import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as s3 from 'aws-cdk-lib/aws-s3';
import { CloudFrontToS3 } from '@aws-solutions-constructs/aws-cloudfront-s3';

export interface secondStackProps extends cdk.NestedStackProps {
  otherBucket: s3.Bucket
}

export class SecondStack extends cdk.NestedStack {
  constructor(scope: Construct, id: string, props?: secondStackProps) {
    super(scope, id, props);

    const remoteBucket = props?.otherBucket;

    new CloudFrontToS3(this, 'test-construct', {
      existingBucketObj: remoteBucket
    });
  }
}

issue1114-stack.ts

import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as s3 from 'aws-cdk-lib/aws-s3';
import { FirstStack } from './first-stack';
import { SecondStack } from './second-stack';

export class Issue1114Stack extends cdk.Stack {
  readonly frontEndBucket: s3.Bucket;

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

    const first = new FirstStack(this, 'first-stack', {})

    new SecondStack(this, 'second-stack', {
      otherBucket: first.remoteBucket
    });

  }
}

The result is circular reference error when I deploy the top stack because the generated CFN template for Issue1114Stack creates dependencies between the two stacks:

{
 "Resources": {
  "firststackNestedStackfirststackNestedStackResource2ADA2ADF": {
   "Type": "AWS::CloudFormation::Stack",
   "Properties": {
    "Parameters": {
     "referencetoIssue1114Stacksecon...6FEABD2FRef": {
      "Fn::GetAtt": [
       "secondstackNestedStacksecondstackNestedStackResource4FB1EDB8",
       "Outputs.Issue1114StacksecondstacktestconstructCloudFrontDistribution6FEABD2FRef"
      ]
     }
    }
    ...
   }
  },
  "secondstackNestedStacksecondstackNestedStackResource4FB1EDB8": {
   "Type": "AWS::CloudFormation::Stack",
   "Properties": {
    "Parameters": {
     "referencetoIssue1114Stackfirststack...132RegionalDomainName": {
      "Fn::GetAtt": [
       "firststackNestedStackfirststackNestedStackResource2ADA2ADF",
       "Outputs.Issue1114StackfirststackFirstStackBucket70FAE132RegionalDomainName"
      ]
     }
    }
    ...
   }
   ...
  }
 }
}

Am I missing something about your deployment? Are you taking some additional action to avoid this situation?

Thanks

maximedemarey-wb commented 2 months ago

I do not use NestedStack but Stack :

app.ts

const app = new cdk.App();

const deployS3BucketStack = new DeployBucketStack(
  app,
  `DeployBucketStack`,
  {
  ...
  }
);

new CloudFrontStack(app, `CloudFrontStack `, {
  s3Bucket: deployS3BucketStack.s3Bucket,
});

s3 stack

export default class DeployBucketStack extends cdk.Stack {
  public readonly s3Bucket: s3.IBucket;

  constructor(scope: Construct, id: string, props: IEnvironmentConfig) {
    super(scope, id, props);

    this.s3Bucket = new s3.Bucket(this, 'FrontendBucket', {
      bucketName: `test`,
      encryption: s3.BucketEncryption.S3_MANAGED,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
    });
  }
}

cloudfrontToS3 stack

export default class CloudFrontStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props: IEnvironmentConfig) {
    super(scope, id, props);

    const cloudfrontS3 = new CloudFrontToS3(
      this,
      `cloudfront-s3`,
      {
        cloudFrontDistributionProps: {
        ...
        },
        existingBucketObj: props.s3Bucket,
      }
    );
  }
}

Thanks,

biffgaut commented 1 month ago

I created a one file app with as much of your code as I could (I had to make up IEnvironmentConfig). When I try to launch it I still get a circular dependency between stacks when the aws-cloudfront-s3 constructs references the RegionalDomainName of the bucket passed in. Can you figure out what is different about your code from the code below that enables you to avoid the circular reference error?

#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as s3 from 'aws-cdk-lib/aws-s3';
import { CloudFrontToS3 } from '@aws-solutions-constructs/aws-cloudfront-s3';

interface IEnvironmentConfig extends cdk.StackProps {
  valueOne?: string,
  s3Bucket?: s3.IBucket
}

class DeployBucketStack extends cdk.Stack {
  public readonly s3Bucket: s3.IBucket;

  constructor(scope: Construct, id: string, props: IEnvironmentConfig) {
    super(scope, id, props);

    this.s3Bucket = new s3.Bucket(this, 'FrontendBucket', {
      bucketName: `test`,
      encryption: s3.BucketEncryption.S3_MANAGED,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
    });
  }
}

class CloudFrontStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props: IEnvironmentConfig) {
    super(scope, id, props);

    const cloudfrontS3 = new CloudFrontToS3(
      this,
      `cloudfront-s3`,
      {
        cloudFrontDistributionProps: {},
        existingBucketObj: props.s3Bucket,
      }
    );
  }
}

// *************************
//  APP BEGINS HERE
// *************************

const app = new cdk.App();

const deployS3BucketStack = new DeployBucketStack(
  app,
  `DeployBucketStack`,
  {}
);

new CloudFrontStack(app, `CloudFrontStack`, {
  s3Bucket: deployS3BucketStack.s3Bucket,
});