Skip to content

Commit

Permalink
fix(apigatewayv2): ApiMapping does not depend on DomainName (#16201)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
Niranjan Jayakar committed Sep 9, 2021
1 parent 87f62b7 commit 1e247d8
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class DomainName extends Resource implements IDomainName {
],
};
const resource = new CfnDomainName(this, 'Resource', domainNameProps);
this.name = props.domainName ?? resource.ref;
this.name = resource.ref;
this.regionalDomainName = Token.asString(resource.getAtt('RegionalDomainName'));
this.regionalHostedZoneId = Token.asString(resource.getAtt('RegionalHostedZoneId'));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe('ApiMapping', () => {
ApiId: {
Ref: 'ApiF70053CD',
},
DomainName: 'example.com',
Stage: '$default',
});
});
Expand Down Expand Up @@ -58,7 +57,6 @@ describe('ApiMapping', () => {
ApiId: {
Ref: 'ApiF70053CD',
},
DomainName: 'example.com',
Stage: 'beta',
ApiMappingKey: 'beta',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ describe('DomainName', () => {
ApiId: {
Ref: 'ApiF70053CD',
},
DomainName: 'example.com',
DomainName: {
Ref: 'DNFDC76583',
},
Stage: 'beta',
ApiMappingKey: 'beta',
});
Expand Down Expand Up @@ -139,7 +141,9 @@ describe('DomainName', () => {
ApiId: {
Ref: 'ApiF70053CD',
},
DomainName: 'example.com',
DomainName: {
Ref: 'DNFDC76583',
},
Stage: '$default',
});
});
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-apigatewayv2/test/http/stage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ describe('HttpStage with domain mapping', () => {
},
});

expect(stage.domainUrl).toBe(`https://${domainName}/`);
expect(stack.resolve(stage.domainUrl)).toEqual({
'Fn::Join': ['', [
'https://', { Ref: 'DNFDC76583' }, '/',
]],
});
});

test('domainUrl throws error if domainMapping is not configured', () => {
Expand Down

0 comments on commit 1e247d8

Please sign in to comment.