Skip to content

Commit

Permalink
Merge pull request #606 from amplify-education/bugfix/AT-10914-hosted…
Browse files Browse the repository at this point in the history
…-zone-filtering

AT-10914: Fix hosted zone filtering
  • Loading branch information
rddimon committed Dec 1, 2023
2 parents 4f920d0 + a856cea commit dba13b4
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 11 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [7.3.1] - 2023-12-01

### Fixed
- Fixed hosted zone filtering.

## [7.3.0] - 2023-11-30

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "serverless-domain-manager",
"version": "7.3.0",
"version": "7.3.1",
"engines": {
"node": ">=14"
},
Expand Down
6 changes: 4 additions & 2 deletions src/aws/acm-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import Globals from "../globals";
import DomainConfig = require("../models/domain-config");
import {getAWSPagedResults} from "../utils";
import Logging from "../logging";

const certStatuses = [
CertificateStatus.PENDING_VALIDATION,
Expand Down Expand Up @@ -47,8 +48,9 @@ class ACMWrapper {
certificateArn = this.getCertArnByCertName(certificates, certificateName);
} else {
certificateName = domain.givenDomainName;
certificateArn = this.getCertArnByDomainName(certificates, certificateName);
certificateArn = ACMWrapper.getCertArnByDomainName(certificates, certificateName);
}
Logging.logInfo(`Found a certificate ARN: '${certificateArn}'`);
} catch (err) {
throw Error(`Could not search certificates in Certificate Manager.\n${err.message}`);
}
Expand All @@ -66,7 +68,7 @@ class ACMWrapper {
return null;
}

private getCertArnByDomainName(certificates, domainName): string {
private static getCertArnByDomainName(certificates, domainName): string {
// The more specific name will be the longest
let nameLength = 0;
let certificateArn;
Expand Down
5 changes: 4 additions & 1 deletion src/aws/route53-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,20 @@ class Route53Wrapper {
"NextMarker",
new ListHostedZonesCommand({})
);
Logging.logInfo(`Founded hosted zones list: ${hostedZones.map(zone => zone.Name)}.`);
} catch (err) {
throw new Error(`Unable to list hosted zones in Route53.\n${err.message}`);
}

// removing the first part of the domain name, api.test.com => test.com
const domainNameHost = domain.givenDomainName.substring(domain.givenDomainName.indexOf(".") + 1);
const targetHostedZone = hostedZones
.filter((hostedZone) => {
return !isPrivateDefined || isHostedZonePrivate === hostedZone.Config.PrivateZone;
})
.filter((hostedZone) => {
const hostedZoneName = hostedZone.Name.replace(/\.$/, "");
return domain.givenDomainName.endsWith(hostedZoneName);
return domainNameHost.endsWith(hostedZoneName);
})
.sort((zone1, zone2) => zone2.Name.length - zone1.Name.length)
.shift();
Expand Down
2 changes: 0 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,6 @@ class ServerlessCustomDomain {
await this.s3Wrapper.assertTlsCertObjectExists(domain);
}
if (!domain.certificateArn) {
const searchName = domain.certificateName || domain.givenDomainName;
Logging.logInfo(`Searching for a certificate with the '${searchName}' domain`);
domain.certificateArn = await acm.getCertArn(domain);
}
domain.domainInfo = await apiGateway.createCustomDomain(domain);
Expand Down
2 changes: 1 addition & 1 deletion test/integration-tests/debug/pr-example/serverless.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ custom:
autoDomain: true
basePath: ""
domainName: ${env:PLUGIN_IDENTIFIER}-http-${env:RANDOM_STRING}.${env:TEST_DOMAIN}
stage: "dev"
createRoute53Record: true
endpointType: REGIONAL

package:
patterns:
Expand Down
13 changes: 9 additions & 4 deletions test/unit-tests/aws/route53-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,19 +165,24 @@ describe("Route53 wrapper checks", () => {
{
CallerReference: "",
Config: {PrivateZone: false},
Id: testId,
Name: "test_domain",
Id: "no_valid",
Name: "api.test_domain",
}, {
CallerReference: "",
Config: {PrivateZone: true},
Id: "dummy_host_id",
Name: "test_domain",
}
}, {
CallerReference: "",
Config: {PrivateZone: false},
Id: testId,
Name: "test_domain",
},
]
});

const dc = new DomainConfig(getDomainConfig({
domainName: "test_domain"
domainName: "devapi.test_domain"
}));

const actualId = await new Route53Wrapper().getRoute53HostedZoneId(dc, false);
Expand Down

0 comments on commit dba13b4

Please sign in to comment.