CloudFrontToS3: OriginAccessControl already exists

[shellscape]

When using the CloudFrontToS3 construct, there's an undocumented requirement around the id constructor parameter: The inner workings of this construct will only consider 9 characters of that ID passed in. Take an identifier like this dev-svc-uploads-assets-cdn which is completely reasonable. That gets transformed into dev-s-cdn. When you're deploying more than one CDN, that ends up in an AlreadyExists error. This is due to the inner workings in the core:

• createCloudFrontDistributionForS3 is called from

  aws-solutions-constructs/source/patterns/@aws-solutions-constructs/aws-cloudfront-s3/lib/index.ts

  Line 180 in c297b11

  const cloudFrontDistributionForS3Response = defaults.createCloudFrontDistributionForS3(this, id, cloudFrontDistributionForS3Props);

• a new CfnOriginAccessControl construct is added, and the code tries to get fancy by creating it's own internal unique ID for the OAC here with generatePhysicalOacName here https://github.com/awslabs/aws-solutions-constructs/blob/c297b118628b63607ba9f9beaaf2234af586deac/source/patterns/%40aws-solutions-constructs/core/lib/cloudfront-distribution-helper.ts#L149C15-L149C38
• that redirects to generatePhysicalName here https://github.com/awslabs/aws-solutions-constructs/blob/main/source/patterns/%40aws-solutions-constructs/core/lib/utils.ts#L181 which slices and dices the original id passed in

The crux of this is that the function that tries to generate a unique ID doesn't follow CDK core's own algorithms, which it really should as that's one that doesn't run into conflicts.

Alternatively, we'd have properties we could provide that would override the ID generation.

At the moment, we have to use a patched dependency to get around this if we want to pass proper IDs that match our environment standards (in our case {env}-{project}-{thing name}-{thing type}

At the least, the user should know that their IDs are being squashed for resource IDs under the hood. Imagine trying to find this resource by name/id in the console and wondering where the heck it is because the ID you passed was turned into something unrecognizable.

Reproduction Steps

const result = new CloudFrontToS3(scope, 'dev-svc-uploads-assets-cdn', { ... });
const result = new CloudFrontToS3(scope, 'dev-svc-uploads-media-cdn', { ... });

Error Log

dev-svc-uploads-stack failed: Error: The stack named dev-svc-uploads-stack failed to deploy: UPDATE_ROLLBACK_COMPLETE: Resource handler returned message: "Resource of type 'AWS::CloudFront::OriginAccessControl' with identifier 'aws-cloudfront-s3-dev-s-cdn-466204b0-4968-11ef-acca-0affef817d45' already exists." (RequestToken: ecff76f4-c8bb-4910-11ed-fd67a61d9d6c, HandlerErrorCode: AlreadyExists)

Environment

• **CDK CLI Version :2.1005.0
• **CDK Framework Version: 2.185.0
• **AWS Solutions Constructs Version : 2.79.1
• **OS : Mac
• **Language :TS

Other

---

This is 🐛 Bug Report

[biffgaut]

Thanks for the note. Unfortunately I'm not able to reproduce your issue with the code provided. The following stack successfully launches for me:

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

    const result = new CloudFrontToS3(this, 'dev-svc-uploads-assets-cdn', {  });
    const resultTwo = new CloudFrontToS3(this, 'dev-svc-uploads-media-cdn', { });
  }
}

This creates 2 CloudFront distributions and 2 OACs using Constructs 2.79.1 and CDK lib 2.185.0. I assume there is something else in the stack (perhaps in the {...}) exacerbating the issue. Can you provide any more detail?

[shellscape]

Here's the properties we're passing (sans specific dynamic values which I'm not inclined to share since it would identify a few endpoints)

cloudFrontDistributionProps: {
      defaultBehavior: {
        viewerProtocolPolicy: ViewerProtocolPolicy.REDIRECT_TO_HTTPS
      },
      domainNames: [domainName],
      certificate,
      priceClass: PriceClass.PRICE_CLASS_ALL
    },
    existingBucketObj: bucket,
    logCloudFrontAccessLog: true,
    responseHeadersPolicyProps: {
      corsBehavior: {
        accessControlAllowCredentials: false,
        accessControlAllowHeaders: ['content-type', 'x-amz-user-agent'],
        accessControlAllowMethods: ['GET', 'HEAD', 'OPTIONS'],
        accessControlAllowOrigins: ['*'],
        accessControlExposeHeaders: [],
        accessControlMaxAge: Duration.seconds(600),
        originOverride: true
      },
      customHeadersBehavior: {
        customHeaders: [
          {
            header: 'Cache-Control',
            value: 'public, max-age=31536000, immutable',
            override: true
          }
        ]
      }
    }

We're able to consistently reproduce this on various stacks in our deployments. We're currently operating with a package patch that allows us to override the OAC ID being used in the underlying solutions code (same spots as shared in the OP)

[biffgaut]

Adding the props had no impact on my names, I'm still not seeing the error you're seeing. The nature of GeneratePhysicalName() leads to the last 5 characters of the construct ID being embedded in the name:

aws-cloudfront-s3-dev-s-cdn-6820d620-09ad-11f0-9f30-0e68ec2a59a3
aws-cloudfront-s3-dev-a-cdn-6820d620-09ad-11f0-9f30-0e68ec2a59a3

In this case "s-cdn" and "a-cdn", the last 5 characters in the two IDs you list above. Why you are getting "s-cdn" twice with the two IDs you list above is a mystery. Is it possible that there is a third instantiation of the construct somewhere in the stack? Are those not the exact names you use in your code?

If there is a way to make the last 5 characters of the IDs of each construct unique, that would be an immediate fix while we consider more significant changes to address the issue in the future.

[biffgaut]

Aside from examining the last 5 characters of the IDs (see previous comment):

The CDK MakeUniqueResourceName() function is very similar to our GeneratePhysicalName(). Both assemble a list of ids into a long ID, then delete enough characters from the middle to fit in the maximum length. It adds a hash of the long ID at the end, we add the GUID of the stack at the end. The hash ensures that the id is unique even if trimming the middle of the long ID results in non-unique IDs. The risk of this is increased because the max length of the OAC physical name is only 64 characters. But the CDK function creates resource names unique within stack - it won't work for physical names that need to be unique across stacks. Because we add the Stack ID, more than one instance of stack can be launched in a single account - without the stack ID, physical names would collide between the two stacks. As our implementation preceded the CDK L2 OAC construct by several months, we use CfnOriginAccessControl which requires that we specify a physical name.

There are several paths forward. We can look at adding a hash to our GeneratePhysicalName() function, so it is then the best of both worlds. If we were to just do this it would be destructive change to existing stacks, as resource names would change. So we would probably put it behind a feature flag. We've been considering feature flag changes for a little while now, but it would not be a quick fix.

We could also accept an S3OriginAccessControlProps object that allows clients to override the generated name (while we use the L1 CfnOriginAccessControlProps they largely overlap and we anticipate moving to the CDK L2 OAC construct in the future). Our sense is that most client are not going to want to assume responsibility for creating a reproducible, unique name, but it would give you a path forward.