AT-10914: Fix hosted zone filtering

https://amplify-education.atlassian.net/browse/AT-10914
amplify-education · Dec 1, 2023 · 08ec346 · 08ec346
1 parent 4f920d0
commit 08ec346
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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 

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-domain-manager",
-  "version": "7.3.0",
+  "version": "7.3.1",
   "engines": {
     "node": ">=14"
   },

diff --git a/src/aws/acm-wrapper.ts b/src/aws/acm-wrapper.ts
@@ -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,
@@ -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}`);
         }
@@ -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;

diff --git a/src/aws/route53-wrapper.ts b/src/aws/route53-wrapper.ts
@@ -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();

diff --git a/src/index.ts b/src/index.ts
@@ -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);

diff --git a/test/integration-tests/debug/pr-example/serverless.yml b/test/integration-tests/debug/pr-example/serverless.yml
@@ -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:

diff --git a/test/unit-tests/aws/route53-wrapper.test.ts b/test/unit-tests/aws/route53-wrapper.test.ts
@@ -217,6 +217,41 @@ describe("Route53 wrapper checks", () => {
         expect(commandCalls.length).to.equal(1);
     });
 
+    it("get route53 hosted zones with overlaps", async () => {
+        const testId = "test_host_id"
+        const Route53Mock = mockClient(Route53Client);
+        Route53Mock.on(ListHostedZonesCommand).resolves({
+            HostedZones: [
+                {
+                    CallerReference: "1",
+                    Config: {PrivateZone: false},
+                    Id: "dummy_host_id",
+                    Name: "dummy_domain",
+                }, {
+                    CallerReference: "2",
+                    Config: {PrivateZone: false},
+                    Id: "not_valid",
+                    Name: "api.test_domain",
+                }, {
+                    CallerReference: "3",
+                    Config: {PrivateZone: false},
+                    Id: testId,
+                    Name: "test_domain",
+                }
+            ]
+        });
+
+        const dc = new DomainConfig(getDomainConfig({
+            domainName: "devapi.test_domain"
+        }));
+
+        const actualId = await new Route53Wrapper().getRoute53HostedZoneId(dc);
+        expect(actualId).to.equal(testId);
+
+        const commandCalls = Route53Mock.commandCalls(ListHostedZonesCommand, {});
+        expect(commandCalls.length).to.equal(1);
+    });
+
     it("get route53 hosted zone id failure", async () => {
         const Route53Mock = mockClient(Route53Client);
         Route53Mock.on(ListHostedZonesCommand).rejects(null);