fix: revert changes to custom resources of passing hostedzoneids (#5540)

Sorry again for this mistake 🙇 I did not anticipate that it breaks the permissions required to create environment and services. Previously we added the logic to pass all the HostedZoneId as params in the custom resource in #5315.  But this introduces a bug as @Lou1415926 described in this issue #5535 (comment) So reverting this changes as these are not necessary because `listHostedZonesByName` always fetches all the public hosted zones followed by private hosted zones.  And also I think these modifications for filtering of hosted zones are not required in custom resources because `listHostedZonesByName` api call always lists all the public hosted zones first and at last it lists private hosted zones. The logic we have in all the custom resources always works to fetch public hosted zone.   ```  const data = await appRoute53Client.listHostedZonesByName({ DNSName: domainName, MaxItems: "1", }).promise();   ```
aws · Dec 11, 2023 · 8de7d2b · 8de7d2b
1 parent ae710d5
commit 8de7d2b
Show file tree

Hide file tree

Showing 40 changed files with 100 additions and 433 deletions.
diff --git a/cf-custom-resources/lib/custom-domain-app-runner.js b/cf-custom-resources/lib/custom-domain-app-runner.js
@@ -95,10 +95,7 @@ exports.handler = async function (event, context) {
             }),
         });
         appRunnerClient = new AWS.AppRunner();
-        appHostedZoneID = props.RootHostedZoneId
-        if (!appHostedZoneID){
-            appHostedZoneID = await domainHostedZoneID(appDNSName);
-        }
+        appHostedZoneID = await domainHostedZoneID(appDNSName);
         switch (event.RequestType) {
             case "Create":
             case "Update":

diff --git a/cf-custom-resources/lib/custom-domain.js b/cf-custom-resources/lib/custom-domain.js
@@ -98,10 +98,7 @@ const writeCustomDomainRecord = async function (
   accessDNS,
   accessHostedZone,
   aliasTypes,
-  action,
-  rootHostedZoneId,
-  appHostedZoneId,
-  envHostedZoneId
+  action
 ) {
   const actions = [];
   for (const alias of aliases) {
@@ -114,8 +111,7 @@ const writeCustomDomainRecord = async function (
           accessDNS,
           accessHostedZone,
           aliasType.domain,
-          action,
-          envHostedZoneId
+          action
         ));
         break;
       case aliasTypes.AppDomainZone:
@@ -125,8 +121,7 @@ const writeCustomDomainRecord = async function (
           accessDNS,
           accessHostedZone,
           aliasType.domain,
-          action,
-          appHostedZoneId
+          action
         ));
         break;
       case aliasTypes.RootDomainZone:
@@ -136,8 +131,7 @@ const writeCustomDomainRecord = async function (
           accessDNS,
           accessHostedZone,
           aliasType.domain,
-          action,
-          rootHostedZoneId
+          action
         ));
         break;
       // We'll skip if it is the other alias type since it will be in another account's route53.
@@ -153,10 +147,9 @@ const writeARecord = async function (
   accessDNS,
   accessHostedZone,
   domain,
-  action,
-  hostedZoneID
+  action
 ) {
-  let hostedZoneId = hostedZoneID || hostedZoneCache.get(domain);
+  let hostedZoneId = hostedZoneCache.get(domain);
   if (!hostedZoneId) {
     const hostedZones = await route53
       .listHostedZonesByName({
@@ -240,9 +233,6 @@ exports.handler = async function (event, context) {
           props.PublicAccessHostedZone,
           aliasTypes,
           changeRecordAction.Upsert,
-          props.RootHostedZoneId,
-          props.AppHostedZoneId,
-          props.EnvHostedZoneId
         );
         break;
       case "Update":
@@ -254,9 +244,6 @@ exports.handler = async function (event, context) {
           props.PublicAccessHostedZone,
           aliasTypes,
           changeRecordAction.Upsert,
-          props.RootHostedZoneId,
-          props.AppHostedZoneId,
-          props.EnvHostedZoneId
         );
         // After upserting new aliases, delete unused ones. For example: previously we have ["foo.com", "bar.com"],
         // and now the aliases param is updated to just ["foo.com"] then we'll delete "bar.com".
@@ -274,9 +261,6 @@ exports.handler = async function (event, context) {
           props.PublicAccessHostedZone,
           aliasTypes,
           changeRecordAction.Delete,
-          props.RootHostedZoneId,
-          props.AppHostedZoneId,
-          props.EnvHostedZoneId
         );
         break;
       case "Delete":
@@ -287,10 +271,7 @@ exports.handler = async function (event, context) {
           props.PublicAccessDNS,
           props.PublicAccessHostedZone,
           aliasTypes,
-          changeRecordAction.Delete,
-          props.RootHostedZoneId,
-          props.AppHostedZoneId,
-          props.EnvHostedZoneId
+          changeRecordAction.Delete
         );
         break;
       default:

diff --git a/cf-custom-resources/lib/dns-cert-validator.js b/cf-custom-resources/lib/dns-cert-validator.js
@@ -212,8 +212,6 @@ const validateCertificate = async function(
     options,
     envRoute53,
     appRoute53,
-    rootHostedZoneId,
-    appHostedZoneId,
     envHostedZoneId,
     certificateARN,
     acm
@@ -223,8 +221,6 @@ const validateCertificate = async function(
       options,
       envRoute53,
       appRoute53,
-      rootHostedZoneId,
-      appHostedZoneId,
       envHostedZoneId
   );
 
@@ -245,9 +241,7 @@ const updateHostedZoneRecords = async function (
   options,
   envRoute53,
   appRoute53,
-  rootHostedZoneId,
-  appHostedZoneId,
-  envHostedZoneId,
+  envHostedZoneId
 ) {
   const promises = [];
   for (const option of options) {
@@ -271,7 +265,6 @@ const updateHostedZoneRecords = async function (
             record: option.ResourceRecord,
             action: action,
             domainName: domainType.domain,
-            hostedZoneId: appHostedZoneId,
           })
         );
         break;
@@ -282,7 +275,6 @@ const updateHostedZoneRecords = async function (
             record: option.ResourceRecord,
             action: action,
             domainName: domainType.domain,
-            hostedZoneId: rootHostedZoneId,
           })
         );
         break;
@@ -301,8 +293,6 @@ const deleteHostedZoneRecords = async function (
   envRoute53,
   appRoute53,
   acm,
-  rootHostedZoneId,
-  appHostedZoneId,
   envHostedZoneId
 ) {
   let listCertificatesInput = {};
@@ -364,8 +354,6 @@ const deleteHostedZoneRecords = async function (
         filteredRecordOption,
         envRoute53,
         appRoute53,
-        rootHostedZoneId,
-        appHostedZoneId,
         envHostedZoneId
     );
   } catch (e) {
@@ -428,8 +416,6 @@ const deleteCertificate = async function (
   arn,
   certDomain,
   region,
-  rootHostedZoneId,
-  appHostedZoneId,
   envHostedZoneId,
   rootDnsRole
 ) {
@@ -477,8 +463,6 @@ const deleteCertificate = async function (
       envRoute53,
       appRoute53,
       acm,
-      rootHostedZoneId,
-      appHostedZoneId,
       envHostedZoneId
     );
 
@@ -642,7 +626,7 @@ exports.certificateRequestHandler = async function (event, context) {
         );
         responseData.Arn = physicalResourceId = response.CertificateArn;  // Set physicalResourceId as soon as we can.
         options = await waitForValidationOptionsToBeReady(response.CertificateArn, sansToUse, acm);
-        await validateCertificate(options, envRoute53, appRoute53, props.RootHostedZoneId, props.AppHostedZoneId, props.EnvHostedZoneId, response.CertificateArn, acm);
+        await validateCertificate(options, envRoute53, appRoute53, props.EnvHostedZoneId, response.CertificateArn, acm);
         break;
       case "Update":
         // Exit early if cert doesn't change.
@@ -660,7 +644,7 @@ exports.certificateRequestHandler = async function (event, context) {
         );
         responseData.Arn = physicalResourceId = response.CertificateArn;
         options = await waitForValidationOptionsToBeReady(response.CertificateArn, sansToUse, acm);
-        await validateCertificate(options, envRoute53, appRoute53, props.RootHostedZoneId, props.AppHostedZoneId, props.EnvHostedZoneId, response.CertificateArn, acm);
+        await validateCertificate(options, envRoute53, appRoute53, props.EnvHostedZoneId, response.CertificateArn, acm);
         break;
       case "Delete":
         // If the resource didn't create correctly, the physical resource ID won't be the
@@ -670,8 +654,6 @@ exports.certificateRequestHandler = async function (event, context) {
             physicalResourceId,
             certDomain,
             props.Region,
-            props.RootHostedZoneId, 
-            props.AppHostedZoneId,
             props.EnvHostedZoneId,
             props.RootDNSRole
           );

diff --git a/cf-custom-resources/lib/dns-delegation.js b/cf-custom-resources/lib/dns-delegation.js
@@ -90,16 +90,15 @@ const createSubdomainInRoot = async function (
   domainName,
   subDomain,
   nameServers,
-  rootDnsRole,
-  hostedZoneId
+  rootDnsRole
 ) {
   const route53 = new aws.Route53({
     credentials: new aws.ChainableTemporaryCredentials({
       params: { RoleArn: rootDnsRole },
       masterCredentials: new aws.EnvironmentCredentials("AWS"),
     }),
   });
-  if (!hostedZoneId) {
+
   const hostedZones = await route53
     .listHostedZonesByName({
       DNSName: domainName,
@@ -116,8 +115,8 @@ const createSubdomainInRoot = async function (
 
   // HostedZoneIDs are of the form /hostedzone/1234455, but the actual
   // ID is after the last slash.
-  hostedZoneId = domainHostedZone.Id.split("/").pop();
-  }
+  const hostedZoneId = domainHostedZone.Id.split("/").pop();
+
   const changeBatch = await route53
     .changeResourceRecordSets({
       ChangeBatch: {
@@ -159,16 +158,15 @@ const deleteSubdomainInRoot = async function (
   requestId,
   domainName,
   subDomain,
-  rootDnsRole,
-  hostedZoneId
+  rootDnsRole
 ) {
   const route53 = new aws.Route53({
     credentials: new aws.ChainableTemporaryCredentials({
       params: { RoleArn: rootDnsRole },
       masterCredentials: new aws.EnvironmentCredentials("AWS"),
     }),
   });
-  if (!hostedZoneId) {
+
   const hostedZones = await route53
     .listHostedZonesByName({
       DNSName: domainName,
@@ -185,8 +183,8 @@ const deleteSubdomainInRoot = async function (
 
   // HostedZoneIDs are of the form /hostedzone/1234455, but the actual
   // ID is after the last slash.
-  hostedZoneId = domainHostedZone.Id.split("/").pop();
-  }
+  const hostedZoneId = domainHostedZone.Id.split("/").pop();
+
   // Find the recordsets for this subdomain, and then remove it
   // from the hosted zone.
   const recordSets = await route53
@@ -277,17 +275,15 @@ exports.domainDelegationHandler = async function (event, context) {
           props.DomainName,
           props.SubdomainName,
           props.NameServers,
-          props.RootDNSRole,
-          props.RootHostedZoneId
+          props.RootDNSRole
         );
         break;
       case "Delete":
         await deleteSubdomainInRoot(
           event.RequestId,
           props.DomainName,
           props.SubdomainName,
-          props.RootDNSRole,
-          props.RootHostedZoneId
+          props.RootDNSRole
         );
         break;
       default:

diff --git a/cf-custom-resources/lib/wkld-cert-validator.js b/cf-custom-resources/lib/wkld-cert-validator.js
@@ -10,7 +10,7 @@ const ATTEMPTS_CERTIFICATE_VALIDATED = 19;
 const ATTEMPTS_CERTIFICATE_NOT_IN_USE = 12;
 const DELAY_CERTIFICATE_VALIDATED_IN_S = 30;
 
-let rootHostedZoneID,appHostedZoneID,envHostedZoneID, appName, envName, serviceName, certificateDomain, domainTypes, rootDNSRole, domainName, isCloudFrontCert;
+let envHostedZoneID, appName, envName, serviceName, certificateDomain, domainTypes, rootDNSRole, domainName, isCloudFrontCert;
 let defaultSleep = function (ms) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 };
@@ -168,8 +168,6 @@ exports.handler = async function (event, context) {
   const aliases = new Set(props.Aliases);
 
   // Initialize global variables.
-  rootHostedZoneID = props.RootHostedZoneId;
-  appHostedZoneID = props.AppHostedZoneId;
   envHostedZoneID = props.EnvHostedZoneId;
   envName = props.EnvName;
   appName = props.AppName;
@@ -750,23 +748,17 @@ async function domainResources(alias) {
     };
   }
   if (domainTypes.AppDomainZone.regex.test(alias)) {
-    if (!appHostedZoneID){
-      appHostedZoneID = await hostedZoneID.app()
-    }
     return {
       domain: domainTypes.AppDomainZone.domain,
       route53Client: clients.app.route53(),
-      hostedZoneID: appHostedZoneID,
+      hostedZoneID: await hostedZoneID.app(),
     };
   }
   if (domainTypes.RootDomainZone.regex.test(alias)) {
-    if (!rootHostedZoneID){
-      rootHostedZoneID = await hostedZoneID.root()
-    }
     return {
       domain: domainTypes.RootDomainZone.domain,
       route53Client: clients.root.route53(),
-      hostedZoneID: rootHostedZoneID,
+      hostedZoneID: await hostedZoneID.root(),
     };
   }
   throw new UnrecognizedDomainTypeError(`unrecognized domain type for ${alias}`);

diff --git a/cf-custom-resources/lib/wkld-custom-domain.js b/cf-custom-resources/lib/wkld-custom-domain.js
@@ -6,7 +6,7 @@ const ATTEMPTS_VALIDATION_OPTIONS_READY = 10;
 const ATTEMPTS_RECORD_SETS_CHANGE = 10;
 const DELAY_RECORD_SETS_CHANGE_IN_S = 30;
 
-let rootHostedZoneID,appHostedZoneID,envHostedZoneID, appName, envName, serviceName, domainTypes, rootDNSRole, domainName;
+let envHostedZoneID, appName, envName, serviceName, domainTypes, rootDNSRole, domainName;
 let defaultSleep = function (ms) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 };
@@ -157,8 +157,6 @@ exports.handler = async function (event, context) {
   const aliases = new Set(props.Aliases);
 
   // Initialize global variables.
-  rootHostedZoneID = props.RootHostedZoneId;
-  appHostedZoneID = props.AppHostedZoneId;
   envHostedZoneID = props.EnvHostedZoneId;
   envName = props.EnvName;
   appName = props.AppName;
@@ -446,23 +444,17 @@ async function domainResources(alias) {
     };
   }
   if (domainTypes.AppDomainZone.regex.test(alias)) {
-    if (!appHostedZoneID){
-      appHostedZoneID = await hostedZoneID.app()
-    }
     return {
       domain: domainTypes.AppDomainZone.domain,
       route53Client: clients.app.route53(),
-      hostedZoneID: appHostedZoneID,
+      hostedZoneID: await hostedZoneID.app(),
     };
   }
   if (domainTypes.RootDomainZone.regex.test(alias)) {
-    if (!rootHostedZoneID){
-      rootHostedZoneID = await hostedZoneID.root()
-    }
     return {
       domain: domainTypes.RootDomainZone.domain,
       route53Client: clients.root.route53(),
-      hostedZoneID: rootHostedZoneID,
+      hostedZoneID: await hostedZoneID.root(),
     };
   }
   throw new UnrecognizedDomainTypeError(`unrecognized domain type for ${alias}`);