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

feat(apigatewayv2): http api - domain url for a stage #15973

Merged
merged 8 commits into from
Aug 12, 2021
Prev Previous commit
Next Next commit
feat(apigatewayv2): Updated domainName string to IDomainName, added e…
…rror if _addDomainMapping() is called more than once, and renamed customDomainUrl to domainUrl
samtwil committed Aug 12, 2021
commit 323eaa7e669fcd7ffbe05284cc77d73db26358fd
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apigatewayv2/lib/common/api-mapping.ts
Original file line number Diff line number Diff line change
@@ -85,7 +85,7 @@ export class ApiMapping extends Resource implements IApiMapping {
/**
* API domain name
*/
public readonly domainName?: string;
public readonly domainName: IDomainName;

constructor(scope: Construct, id: string, props: ApiMappingProps) {
super(scope, id);
@@ -126,6 +126,6 @@ export class ApiMapping extends Resource implements IApiMapping {

this.apiMappingId = resource.ref;
this.mappingKey = props.apiMappingKey;
this.domainName = props.domainName.name;
this.domainName = props.domainName;
}
}
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-apigatewayv2/lib/common/base.ts
Original file line number Diff line number Diff line change
@@ -50,6 +50,9 @@ export abstract class StageBase extends Resource implements IStage {
* @internal
*/
protected _addDomainMapping(domainMapping: DomainMappingOptions) {
if (this.apiMapping) {
throw new Error('_addDomainMapping cannot be called more than once for a Stage');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new Error('_addDomainMapping cannot be called more than once for a Stage');
throw new Error('Only one ApiMapping allowed per Stage');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test case for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nija-at What is the recommended pattern for testing a protected function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the problem, it might be a bit convoluted.
I think it's fine to skip the test case for this. Sorry for the mixed message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a problem! Thanks for the response

}
this.apiMapping = new ApiMapping(this, `${domainMapping.domainName}${domainMapping.mappingKey}`, {
api: this.baseApi,
domainName: domainMapping.domainName,
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-apigatewayv2/lib/http/stage.ts
Original file line number Diff line number Diff line change
@@ -180,14 +180,14 @@ export class HttpStage extends HttpStageBase {
/**
* The custom domain URL to this stage
*/
public get customDomainUrl(): string {
public get domainUrl(): string {
if (!this.apiMapping) {
throw new Error('customDomainUrl is not available when domain mapping has not been added');
throw new Error('domainUrl is not available when domain mapping has not been added');
samtwil marked this conversation as resolved.
Show resolved Hide resolved
}
if (!this.apiMapping.domainName) {
throw new Error('Unable to build customDomainUrl due to invalid domainName');
if (!this.apiMapping.domainName.name) {
throw new Error('Unable to build domainUrl due to invalid domainName');
}

return `https://${this.apiMapping.domainName}/${this.apiMapping.mappingKey ?? ''}`;
return `https://${this.apiMapping.domainName.name}/${this.apiMapping.mappingKey ?? ''}`;
}
}
134 changes: 68 additions & 66 deletions packages/@aws-cdk/aws-apigatewayv2/test/http/stage.test.ts
Original file line number Diff line number Diff line change
@@ -4,9 +4,6 @@ import { Metric } from '@aws-cdk/aws-cloudwatch';
import { Stack } from '@aws-cdk/core';
import { DomainName, HttpApi, HttpStage } from '../../lib';

const domainName = 'example.com';
const certArn = 'arn:aws:acm:us-east-1:111111111111:certificate';

describe('HttpStage', () => {
test('default', () => {
const stack = new Stack();
@@ -61,69 +58,6 @@ describe('HttpStage', () => {
expect(betaStage.url.endsWith('/')).toBe(false);
});

test('customDomainUrl returns the correct path', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const dn = new DomainName(stack, 'DN', {
domainName,
certificate: Certificate.fromCertificateArn(stack, 'cert', certArn),
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
domainMapping: {
domainName: dn,
},
});

expect(stage.customDomainUrl.endsWith(`${domainName}/`)).toBe(true);
});

test('customDomainUrl throws error if domainMapping is not configured', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
});

const t = () => {
stage.customDomainUrl;
};

expect(t).toThrow('customDomainUrl is not available when domain mapping has not been added');
});

test('customDomainUrl throws error if domainName is invalid', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const dn = new DomainName(stack, 'DN', {
domainName: '',
certificate: Certificate.fromCertificateArn(stack, 'cert', certArn),
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
domainMapping: {
domainName: dn,
},
});

const t = () => {
stage.customDomainUrl;
};

expect(t).toThrow('Unable to build customDomainUrl due to invalid domainName');
});

test('get metric', () => {
// GIVEN
const stack = new Stack();
@@ -182,4 +116,72 @@ describe('HttpStage', () => {
const metricNames = metrics.map(m => m.metricName);
expect(metricNames).toEqual(['4xx', '5xx', 'DataProcessed', 'Latency', 'IntegrationLatency', 'Count']);
});
});

describe('HttpStage with domain mapping', () => {
const domainName = 'example.com';
const certArn = 'arn:aws:acm:us-east-1:111111111111:certificate';

test('domainUrl returns the correct path', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const dn = new DomainName(stack, 'DN', {
domainName,
certificate: Certificate.fromCertificateArn(stack, 'cert', certArn),
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
domainMapping: {
domainName: dn,
},
});

expect(stage.domainUrl.endsWith(`${domainName}/`)).toBe(true);
nija-at marked this conversation as resolved.
Show resolved Hide resolved
});

test('domainUrl throws error if domainMapping is not configured', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
});

const t = () => {
stage.domainUrl;
};

expect(t).toThrow(/domainUrl is not available when domain mapping has not been added/);
});

test('domainUrl throws error if domainName is invalid', () => {
const stack = new Stack();
const api = new HttpApi(stack, 'Api', {
createDefaultStage: false,
});

const dn = new DomainName(stack, 'DN', {
domainName: '',
certificate: Certificate.fromCertificateArn(stack, 'cert', certArn),
});

const stage = new HttpStage(stack, 'DefaultStage', {
httpApi: api,
domainMapping: {
domainName: dn,
},
});

const t = () => {
stage.domainUrl;
};

expect(t).toThrow(/Unable to build domainUrl due to invalid domainName/);
});
});