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

CloudFrontToS3 error when providing existing s3 bucket #1114

Open
maximedemarey-wb opened this issue May 6, 2024 · 10 comments
Open

CloudFrontToS3 error when providing existing s3 bucket #1114

maximedemarey-wb opened this issue May 6, 2024 · 10 comments
Labels
bug Something isn't working needs-triage The issue or PR still needs to be triaged

Comments

@maximedemarey-wb
Copy link

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

  • CDK CLI Version : 2.122
  • CDK Framework Version: 2.140
  • AWS Solutions Constructs Version : 2.56.0
  • OS : macOS 14
  • Language : Typescript

Other


This is 🐛 Bug Report

@maximedemarey-wb maximedemarey-wb added bug Something isn't working needs-triage The issue or PR still needs to be triaged labels May 6, 2024
@biffgaut
Copy link
Contributor

biffgaut commented May 6, 2024

Thanks, we'll take a look.

@maximedemarey-wb
Copy link
Author

maximedemarey-wb commented May 6, 2024

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
Copy link
Contributor

biffgaut commented May 6, 2024

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

@maximedemarey-wb
Copy link
Author

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

const deployS3BucketStack = new DeployBucketStack(
    ...
  );

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

@biffgaut
Copy link
Contributor

biffgaut commented May 6, 2024

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
Copy link
Author

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
Copy link
Contributor

biffgaut commented May 7, 2024

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,
});

@proxy-hatch
Copy link

Came looking for solution to the circular dependency problem.

@biffgaut
Copy link
Contributor

We were so focused on the original issue and the circular reference preventing us from replicating it that we didn't think to dive deep on the circular reference itself. We'll take a look and see if we can learn more about it. Thanks.

@biffgaut
Copy link
Contributor

The issue isn't actually within the construct, as the code below sees the same behavior. With a little trial and error and template review, the issue seems to be granting the OAI permission to read the S3 bucket. This updates the BucketPolicy in the DeployBucketStack to grant access to a canonical user in the CloudFrontStack, creating the dependency of the deploy stack on the cloudfront stack. Setting up the S3 bucket at the origin then attempts to create the reverse dependency and the CDK objects. It's interesting that it's the CDK code that catches the circular reference and refuses to generate a template - I'm more familiar with CloudFormation enforcing circular reference problems. In other situations the CDK has used custom resources to break circular dependencies like this - you could try to open an issue on their Github repo.

FWIW - I removed the grantRead call and still saw the error, apparently the S3Origin code will execute grantRead anyway.

interface IEnvironmentConfig extends cdk.StackProps {
  s3Bucket: s3.IBucket
}

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

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

    this.s3Bucket = new s3.Bucket(this, 'FrontendBucket', {
      encryption: s3.BucketEncryption.S3_MANAGED,
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
      enforceSSL: true,
      removalPolicy: cdk.RemovalPolicy.DESTROY
    });
  }
}

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

    const originAccessIdentity = new cf.OriginAccessIdentity(this, "OAI");
    props.s3Bucket.grantRead(originAccessIdentity);

    const distribution = new cf.CloudFrontWebDistribution(this, "ContentDistribution", {
      originConfigs: [
        {
          s3OriginSource: {
            s3BucketSource: props.s3Bucket,
            originAccessIdentity
          },
          behaviors: [{ isDefaultBehavior: true }],
        }
      ]
    });

  }
}

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

const app = new cdk.App();

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

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


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage The issue or PR still needs to be triaged
Projects
None yet
Development

No branches or pull requests

3 participants