fix: only add PSC ipType if PSC is enabled (#388)

With CAS-based instances being supported soon and also using dnsName, we can no longer rely on the presence of dnsName in the connectSettings API as identifying that PSC is enabled on an instance. Instead we should check pscEnabled from the response as source of truth. Check if pscEnabled is True before setting PSC ip type. Moving parseIPAddresses into private method on SQLAdminFetcher
GoogleCloudPlatform · Sep 25, 2024 · 28905c9 · 28905c9
1 parent 680c2e9
commit 28905c9
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 184 deletions.
diff --git a/src/ip-addresses.ts b/src/ip-addresses.ts
@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {sqladmin_v1beta4} from '@googleapis/sqladmin';
 import {CloudSQLConnectorError} from './errors';
 
 export enum IpAddressTypes {
@@ -57,36 +56,6 @@ const getPSCIpAddress = (ipAddresses: IpAddresses) => {
   return ipAddresses.psc;
 };
 
-export function parseIpAddresses(
-  ipResponse: sqladmin_v1beta4.Schema$IpMapping[] | undefined,
-  dnsName: string | null | undefined
-): IpAddresses {
-  const ipAddresses: IpAddresses = {};
-  if (ipResponse) {
-    for (const ip of ipResponse) {
-      if (ip.type === 'PRIMARY' && ip.ipAddress) {
-        ipAddresses.public = ip.ipAddress;
-      }
-      if (ip.type === 'PRIVATE' && ip.ipAddress) {
-        ipAddresses.private = ip.ipAddress;
-      }
-    }
-  }
-
-  if (dnsName) {
-    ipAddresses.psc = dnsName;
-  }
-
-  if (!ipAddresses.public && !ipAddresses.private && !ipAddresses.psc) {
-    throw new CloudSQLConnectorError({
-      message: 'Cannot connect to instance, it has no supported IP addresses',
-      code: 'ENOSQLADMINIPADDRESS',
-    });
-  }
-
-  return ipAddresses;
-}
-
 export function selectIpAddress(
   ipAddresses: IpAddresses,
   type: IpAddressTypes | unknown

diff --git a/src/sqladmin-fetcher.ts b/src/sqladmin-fetcher.ts
@@ -19,7 +19,7 @@ const {Sqladmin} = sqladmin_v1beta4;
 import {InstanceConnectionInfo} from './instance-connection-info';
 import {SslCert} from './ssl-cert';
 import {parseCert} from './crypto';
-import {IpAddresses, parseIpAddresses} from './ip-addresses';
+import {IpAddresses} from './ip-addresses';
 import {CloudSQLConnectorError} from './errors';
 import {getNearestExpiration} from './time';
 import {AuthTypes} from './auth-types';
@@ -124,6 +124,40 @@ export class SQLAdminFetcher {
     }
   }
 
+  private parseIpAddresses(
+    ipResponse: sqladmin_v1beta4.Schema$IpMapping[] | undefined,
+    dnsName: string | null | undefined,
+    pscEnabled: boolean | null | undefined
+  ): IpAddresses {
+    const ipAddresses: IpAddresses = {};
+    if (ipResponse) {
+      for (const ip of ipResponse) {
+        if (ip.type === 'PRIMARY' && ip.ipAddress) {
+          ipAddresses.public = ip.ipAddress;
+        }
+        if (ip.type === 'PRIVATE' && ip.ipAddress) {
+          ipAddresses.private = ip.ipAddress;
+        }
+      }
+    }
+
+    // Resolve dnsName into IP address for PSC enabled instances.
+    // Note that we have to check for PSC enablement because CAS instances
+    // also set the dnsName field.
+    if (dnsName && pscEnabled) {
+      ipAddresses.psc = dnsName;
+    }
+
+    if (!ipAddresses.public && !ipAddresses.private && !ipAddresses.psc) {
+      throw new CloudSQLConnectorError({
+        message: 'Cannot connect to instance, it has no supported IP addresses',
+        code: 'ENOSQLADMINIPADDRESS',
+      });
+    }
+
+    return ipAddresses;
+  }
+
   async getInstanceMetadata({
     projectId,
     regionId,
@@ -146,9 +180,10 @@ export class SQLAdminFetcher {
       });
     }
 
-    const ipAddresses = parseIpAddresses(
+    const ipAddresses = this.parseIpAddresses(
       res.data.ipAddresses,
-      res.data.dnsName
+      res.data.dnsName,
+      res.data.pscEnabled
     );
 
     const {serverCaCert} = res.data;

diff --git a/test/ip-addresses.ts b/test/ip-addresses.ts
@@ -13,138 +13,7 @@
 // limitations under the License.
 
 import t from 'tap';
-import {
-  IpAddressTypes,
-  parseIpAddresses,
-  selectIpAddress,
-} from '../src/ip-addresses';
-
-t.throws(
-  () => parseIpAddresses(undefined, undefined),
-  {code: 'ENOSQLADMINIPADDRESS'},
-  'should throw if no argument'
-);
-
-t.throws(
-  () => parseIpAddresses([], undefined),
-  {code: 'ENOSQLADMINIPADDRESS'},
-  'should throw if no ip is found'
-);
-
-t.same(
-  parseIpAddresses(
-    [
-      {
-        ipAddress: '0.0.0.0',
-        type: 'PRIMARY',
-      },
-    ],
-    undefined
-  ),
-  {
-    public: '0.0.0.0',
-  },
-  'should return a public ip successfully'
-);
-
-t.same(
-  parseIpAddresses(
-    [
-      {
-        ipAddress: '0.0.0.0',
-        type: 'PRIMARY',
-      },
-      {
-        ipAddress: '0.0.0.1',
-        type: 'OUTGOING',
-      },
-    ],
-    undefined
-  ),
-  {
-    public: '0.0.0.0',
-  },
-  'should return a public ip from a list'
-);
-
-t.same(
-  parseIpAddresses(
-    [
-      {
-        ipAddress: '0.0.0.2',
-        type: 'PRIVATE',
-      },
-      {
-        ipAddress: '0.0.0.1',
-        type: 'OUTGOING',
-      },
-    ],
-    undefined
-  ),
-  {
-    private: '0.0.0.2',
-  },
-  'should return a private ip from a list'
-);
-
-t.same(
-  parseIpAddresses(
-    [
-      {
-        ipAddress: '0.0.0.0',
-        type: 'PRIMARY',
-      },
-      {
-        ipAddress: '0.0.0.2',
-        type: 'PRIVATE',
-      },
-      {
-        ipAddress: '0.0.0.1',
-        type: 'OUTGOING',
-      },
-    ],
-    undefined
-  ),
-  {
-    private: '0.0.0.2',
-    public: '0.0.0.0',
-  },
-  'should return a both public and private ips if available'
-);
-
-t.same(
-  parseIpAddresses([], 'abcde.12345.us-central1.sql.goog'),
-  {
-    psc: 'abcde.12345.us-central1.sql.goog',
-  },
-  'should return a psc ip from a defined dnsName'
-);
-
-t.same(
-  parseIpAddresses(
-    [
-      {
-        ipAddress: '0.0.0.0',
-        type: 'PRIMARY',
-      },
-      {
-        ipAddress: '0.0.0.2',
-        type: 'PRIVATE',
-      },
-      {
-        ipAddress: '0.0.0.1',
-        type: 'OUTGOING',
-      },
-    ],
-    'abcde.12345.us-central1.sql.goog'
-  ),
-  {
-    private: '0.0.0.2',
-    public: '0.0.0.0',
-    psc: 'abcde.12345.us-central1.sql.goog',
-  },
-  'should return a public, private and psc ips if available'
-);
+import {IpAddressTypes, selectIpAddress} from '../src/ip-addresses';
 
 t.throws(
   () => selectIpAddress({}, IpAddressTypes.PUBLIC),

diff --git a/test/sqladmin-fetcher.ts b/test/sqladmin-fetcher.ts
@@ -28,6 +28,7 @@ interface IpAddress {
 interface SQLAdminClientGetResponse {
   dnsName?: string;
   ipAddresses?: IpAddress[];
+  pscEnabled?: boolean;
   region?: string;
   serverCaCert?: {} | ReturnType<typeof serverCaCertResponse>;
 }
@@ -97,16 +98,22 @@ const mockSQLAdminGetInstanceMetadata = (
 
   sqlAdminClient.get = () => ({
     data: {
+      dnsName: 'abcde.12345.us-central1.sql.goog',
       ipAddresses: [
         {
           type: 'PRIMARY',
           ipAddress: '0.0.0.0',
         },
+        {
+          type: 'PRIVATE',
+          ipAddress: '10.0.0.1',
+        },
         {
           type: 'OUTGOING',
           ipAddress: '0.0.0.1',
         },
       ],
+      pscEnabled: true,
       region: regionId,
       serverCaCert: serverCaCertResponse(instanceId),
       ...overrides,
@@ -175,6 +182,8 @@ t.test('getInstanceMetadata', async t => {
     {
       ipAddresses: {
         public: '0.0.0.0',
+        private: '10.0.0.1',
+        psc: 'abcde.12345.us-central1.sql.goog',
       },
       serverCaCert: {
         cert: '-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----',
@@ -195,33 +204,26 @@ t.test('getInstanceMetadata custom SQL Admin API endpoint', async t => {
   );
 });
 
-t.test('getInstanceMetadata private ip', async t => {
+// dnsName without PSC enabled should result in no PSC ip type
+t.test('getInstanceMetadata no ip', async t => {
   const instanceConnectionInfo: InstanceConnectionInfo = {
-    projectId: 'private-ip-project',
+    projectId: 'no-ip-project',
     regionId: 'us-east1',
-    instanceId: 'private-ip-instance',
+    instanceId: 'no-ip-instance',
   };
   mockSQLAdminGetInstanceMetadata(instanceConnectionInfo, {
-    ipAddresses: [
-      {
-        type: 'PRIVATE',
-        ipAddress: '0.0.0.0',
-      },
-    ],
+    dnsName: 'abcde.12345.us-central1.sql.goog',
+    ipAddresses: [],
+    pscEnabled: false,
   });
 
   const fetcher = new SQLAdminFetcher();
-  const instanceMetadata = await fetcher.getInstanceMetadata(
-    instanceConnectionInfo
-  );
-  t.match(
-    instanceMetadata,
+  t.rejects(
+    fetcher.getInstanceMetadata(instanceConnectionInfo),
     {
-      ipAddresses: {
-        private: '0.0.0.0',
-      },
+      code: 'ENOSQLADMINIPADDRESS',
     },
-    'should return expected instance metadata containing private ip'
+    'should throw no ip type found'
   );
 });