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

(apigatewayv2): ApiMapping does not add a dependency on DomainName #15464

Closed
misterjoshua opened this issue Jul 8, 2021 · 2 comments · Fixed by #16201
Closed

(apigatewayv2): ApiMapping does not add a dependency on DomainName #15464

misterjoshua opened this issue Jul 8, 2021 · 2 comments · Fixed by #16201
Assignees
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@misterjoshua
Copy link
Contributor

ApiMapping does not declare a dependency on the DomainName it maps to an API. So, when both are created in the same stack, CloudFormation may fail to create the mapping with an error like this:

Invalid domain name identifier specified (Service: AmazonApiGatewayV2; Status Code: 404; Error Code: NotFoundException; Re quest ID: <removed>; Proxy: null)

Reproduction Steps

import * as apigatewayv2 from '@aws-cdk/aws-apigatewayv2';
import * as apigatewayv2_integration from '@aws-cdk/aws-apigatewayv2-integrations';
import * as acm from '@aws-cdk/aws-certificatemanager';
import * as cdk from '@aws-cdk/core';

const BUG_DOMAIN_NAME = 'yourdomain.com';
const BUG_WILDCARD_CERT_ARN = 'arn:aws:acm:ca-central-1:1111111111111111:certificate/x';

class BugStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props: cdk.StackProps = {}) {
    super(scope, id, props);

    const api = new apigatewayv2.HttpApi(this, 'Api', {
      apiName: cdk.Stack.of(this).stackName,
      defaultIntegration: new apigatewayv2_integration.HttpProxyIntegration({
        url: 'https://www.example.com',
        method: apigatewayv2.HttpMethod.GET,
      }),
    });

    // Cloudformation races, so this increases odds that a mapping will beat the domain name
    for (let i = 0; i < 10; i++) {
      const domainName = new apigatewayv2.DomainName(this, `DomainName${i}`, {
        domainName: `bug${i}.${BUG_DOMAIN_NAME}`,
        certificate: acm.Certificate.fromCertificateArn(this, `Certificate${i}`, BUG_WILDCARD_CERT_ARN),
      });

      new apigatewayv2.ApiMapping(this, `ApiMapping${i}`, {
        api,
        domainName,
      });
    }
  }
}

const app = new cdk.App();
new BugStack(app, 'BugStack');

app.synth();

What did you expect to happen?

I expected the stack to deploy.

What actually happened?

The stack failed to deploy with an error:

Invalid domain name identifier specified (Service: AmazonApiGatewayV2; Status Code: 404; Error Code: NotFoundException; Re quest ID: <removed>; Proxy: null)

Environment

  • CDK CLI Version : 1.111.0
  • Framework Version: 1.111.0
  • Node.js Version: v14.15.4
  • OS : Windows
  • Language (Version): Typescript 3.9.7

Other

I can work around this by manually adding a dependency between the ApiMapping and the DomainName, but I would have expected the L2 constructs to do this for me.


This is 🐛 Bug Report

@misterjoshua misterjoshua added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 8, 2021
@github-actions github-actions bot added the @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 label Jul 8, 2021
@BenChaimberg
Copy link
Contributor

The name property of DomainName should always be a ref to ensure this dependency exists.

this.name = props.domainName ?? resource.ref;

As described in the issue, the workaround is to take a manual dependency of the ApiMapping on the DomainName (apiMapping.node.addDependency(domainName)).

@BenChaimberg BenChaimberg added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 9, 2021
nija-at pushed a commit that referenced this issue Aug 24, 2021
When an ApiMapping resource is deployed using the Domain defined in the
DomainName resource, the DomainName resource must be deployed before the
ApiMapping resource.

Since the current logic uses the CloudFormation Output of DomainName as
a fall back, preferring the user provided string first, this dependency
is not expressed in the resulting template.

Remove the preference for the user provided string, will inform
synthesis that the dependency must be declared.

fixes #15464
@mergify mergify bot closed this as completed in #16201 Sep 9, 2021
mergify bot pushed a commit that referenced this issue Sep 9, 2021
When an ApiMapping resource is deployed using the Domain defined in the
DomainName resource, the DomainName resource must be deployed before the
ApiMapping resource.

Since the current logic uses the CloudFormation Output of DomainName as
a fall back, preferring the user provided string first, this dependency
is not expressed in the resulting template.

Remove the preference for the user provided string, will inform
synthesis that the dependency must be declared.

fixes #15464


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Sep 9, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants