Address CR, rename vpcId to awsVpcId

gravitational · Jul 31, 2024 · 31ec814 · 31ec814
1 parent 852298b
commit 31ec814
Show file tree

Hide file tree

Showing 13 changed files with 144 additions and 101 deletions.
diff --git a/web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.ts b/web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.ts
@@ -122,6 +122,7 @@ export function useCreateDatabase() {
       ...(agentMeta as DbMeta),
       resourceName: createdDb.name,
       awsRegion: createdDb.awsRegion,
+      awsVpcId: createdDb.awsVpcId,
       agentMatcherLabels: dbPollingResult.labels,
       db: dbPollingResult,
       serviceDeployedMethod:
@@ -163,10 +164,9 @@ export function useCreateDatabase() {
       });
   }
 
-  function fetchDatabaseServers(query: string, limit = 1000) {
+  function fetchDatabaseServers(query: string) {
     const request = {
       query,
-      limit,
     };
     return ctx.databaseService.fetchDatabases(clusterId, request);
   }
@@ -224,6 +224,7 @@ export function useCreateDatabase() {
           awsRegion: db.awsRegion,
           agentMatcherLabels: db.labels,
           selectedAwsRdsDb: db.awsRds,
+          awsVpcId: db.awsVpcId,
         });
         setAttempt({ status: 'success' });
         return;

diff --git a/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/AutoDeploy.tsx b/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/AutoDeploy.tsx
@@ -112,7 +112,9 @@ export function AutoDeploy({ toggleDeployMethod }: DeployServiceProp) {
         .deployDatabaseServices(integrationName, {
           region: dbMeta.awsRegion,
           taskRoleArn,
-          deployments: [{ vpcId: dbMeta.vpcId, subnetIds: selectedSubnetIds }],
+          deployments: [
+            { vpcId: dbMeta.awsVpcId, subnetIds: selectedSubnetIds },
+          ],
         })
         .then(url => {
           setAttempt({ status: 'success' });

diff --git a/...packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx b/...packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx
@@ -26,7 +26,7 @@ import { HoverTooltip, ToolTipInfo } from 'shared/components/ToolTip';
 import useAttempt from 'shared/hooks/useAttemptNext';
 import { getErrMessage } from 'shared/utils/errorType';
 
-import { P } from 'design/Text/Text';
+import { P, P3 } from 'design/Text/Text';
 
 import {
   integrationService,
@@ -83,7 +83,7 @@ export const SelectSecurityGroups = ({
     run(() =>
       integrationService
         .fetchSecurityGroups(dbMeta.awsIntegration.name, {
-          vpcId: dbMeta.vpcId,
+          vpcId: dbMeta.awsVpcId,
           region: dbMeta.awsRegion,
           nextToken: sgTableData.nextToken,
         })
@@ -165,19 +165,26 @@ export const SelectSecurityGroups = ({
             onSelectSecurityGroup={onSelectSecurityGroup}
             selectedSecurityGroups={selectedSecurityGroups}
           />
-          <HoverTooltip
-            tipContent="Refreshing security groups will reset selections"
-            anchorOrigin={{ vertical: 'top', horizontal: 'left' }}
-          >
-            <ButtonSecondary
-              onClick={() => fetchSecurityGroups({ refresh: true })}
-              mt={2}
-              px={2}
-              disabled={disabled}
+          <Flex alignItems="center" gap={3} mt={2}>
+            <HoverTooltip
+              tipContent="Refreshing security groups will reset selections"
+              anchorOrigin={{ vertical: 'top', horizontal: 'left' }}
             >
-              <Icons.Refresh size="medium" mr={2} /> Refresh
-            </ButtonSecondary>
-          </HoverTooltip>
+              <ButtonSecondary
+                onClick={() => fetchSecurityGroups({ refresh: true })}
+                px={2}
+                disabled={disabled}
+              >
+                <Icons.Refresh size="medium" mr={2} /> Refresh
+              </ButtonSecondary>
+            </HoverTooltip>
+            {selectedSecurityGroups.length > 0 && (
+              <P3>
+                <b>Num Selected Security Groups</b>:{' '}
+                {selectedSecurityGroups.length}
+              </P3>
+            )}
+          </Flex>
         </Box>
       )}
     </>

diff --git a/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSubnetIds.tsx b/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSubnetIds.tsx
@@ -18,7 +18,15 @@
 
 import React, { useState, useEffect } from 'react';
 
-import { Text, Flex, Box, Indicator, ButtonSecondary, Subtitle3 } from 'design';
+import {
+  Text,
+  Flex,
+  Box,
+  Indicator,
+  ButtonSecondary,
+  Subtitle3,
+  P3,
+} from 'design';
 import * as Icons from 'design/Icon';
 import { FetchStatus } from 'design/DataTable/types';
 import { HoverTooltip, ToolTipInfo } from 'shared/components/ToolTip';
@@ -80,7 +88,7 @@ export function SelectSubnetIds({
     run(() =>
       integrationService
         .fetchAwsSubnets(dbMeta.awsIntegration.name, clusterId, {
-          vpcId: dbMeta.vpcId,
+          vpcId: dbMeta.awsVpcId,
           region: dbMeta.awsRegion,
           nextToken: tableData.nextToken,
         })
@@ -153,19 +161,25 @@ export function SelectSubnetIds({
             onSelectSubnet={handleSelectSubnet}
             selectedSubnets={selectedSubnetIds}
           />
-          <HoverTooltip
-            tipContent="Refreshing subnets will reset selections"
-            anchorOrigin={{ vertical: 'top', horizontal: 'left' }}
-          >
-            <ButtonSecondary
-              onClick={() => fetchSubnets({ refresh: true })}
-              mt={2}
-              px={2}
-              disabled={disabled}
+          <Flex alignItems="center" gap={3} mt={2}>
+            <HoverTooltip
+              tipContent="Refreshing subnets will reset selections"
+              anchorOrigin={{ vertical: 'top', horizontal: 'left' }}
             >
-              <Icons.Refresh size="medium" mr={2} /> Refresh
-            </ButtonSecondary>
-          </HoverTooltip>
+              <ButtonSecondary
+                onClick={() => fetchSubnets({ refresh: true })}
+                px={2}
+                disabled={disabled}
+              >
+                <Icons.Refresh size="medium" mr={2} /> Refresh
+              </ButtonSecondary>
+            </HoverTooltip>
+            {selectedSubnetIds.length > 0 && (
+              <P3>
+                <b>Num Selected Subnets</b>: {selectedSubnetIds.length}
+              </P3>
+            )}
+          </Flex>
         </Box>
       )}
     </>

diff --git a/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/AutoEnrollment.tsx b/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/AutoEnrollment.tsx
@@ -59,7 +59,7 @@ const emptyTableData = (): TableData => ({
   fetchStatus: 'disabled',
   instancesStartKey: '',
   clustersStartKey: '',
-  oneOfError: '', // todo handle
+  oneOfError: '',
 });
 
 export function AutoEnrollment({
@@ -75,6 +75,8 @@ export function AutoEnrollment({
   fetchAttempt: Attempt;
   onFetchAttempt(a: Attempt): void;
 }) {
+  const hasDatabaseServiceForVpc = !!vpc?.ecsServiceDashboardURL;
+
   const ctx = useTeleport();
   const clusterId = ctx.storeUser.getClusterId();
 
@@ -88,15 +90,17 @@ export function AutoEnrollment({
   const [tableData, setTableData] = useState<TableData>();
 
   useEffect(() => {
-    if (!vpc) {
-      // Clear table, vpc will be null on new region select.
-      setTableData(emptyTableData());
-      onFetchAttempt({ status: '' });
-      setCreateDiscoveryConfigAttempt({ status: '' });
+    if (hasDatabaseServiceForVpc) {
+      // No need to fetch rds's since in place of rds table
+      // we will render a info banner that a db service
+      // already exists.
       return;
     }
-    // Start with empty table data for new vpc's.
-    fetchRdsDatabases(emptyTableData(), vpc);
+
+    if (vpc) {
+      // Start with empty table data for new vpc's.
+      fetchRdsDatabases(emptyTableData(), vpc);
+    }
   }, [vpc]);
 
   function fetchNextPage() {
@@ -156,7 +160,7 @@ export function AutoEnrollment({
     if (!cfg.isCloud) {
       updateAgentMeta({
         ...(agentMeta as DbMeta),
-        vpcId: vpc.id,
+        awsVpcId: vpc.id,
         awsRegion: region,
         autoDiscovery: {},
       });
@@ -194,7 +198,7 @@ export function AutoEnrollment({
         autoDiscovery: {
           config: discoveryConfig,
         },
-        vpcId: vpc.id,
+        awsVpcId: vpc.id,
         awsRegion: region,
       });
     } catch (err) {
@@ -208,12 +212,10 @@ export function AutoEnrollment({
     }
   }
 
-  const hasDatabaseServiceForVpc = !!vpc?.ecsServiceDashboardURL;
   const selectedVpc = !!vpc;
   const showTable =
     selectedVpc &&
     !hasDatabaseServiceForVpc &&
-    !!tableData &&
     fetchAttempt.status !== 'failed';
 
   return (
@@ -232,7 +234,7 @@ export function AutoEnrollment({
       )}
       {showTable && (
         <>
-          {tableData.oneOfError && (
+          {tableData?.oneOfError && (
             <Alert>
               <Flex alignItems="center" gap={2}>
                 {tableData.oneOfError}
@@ -244,13 +246,11 @@ export function AutoEnrollment({
               </Flex>
             </Alert>
           )}
-          {fetchAttempt.status !== 'processing' && (
-            <Text mt={3}>List of databases that will be auto enrolled:</Text>
-          )}
+          <Text mt={3}>List of databases that will be auto enrolled:</Text>
           <DatabaseList
             wantAutoDiscover={true}
-            items={tableData.items}
-            fetchStatus={tableData.fetchStatus}
+            items={tableData?.items || []}
+            fetchStatus={tableData?.fetchStatus || 'loading'}
             fetchNextPage={fetchNextPage}
           />
         </>

diff --git a/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx b/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx
@@ -47,6 +47,12 @@ export function EnrollRdsDatabase() {
   const clusterId = ctx.storeUser.getClusterId();
 
   const { agentMeta, emitErrorEvent } = useDiscover();
+
+  // This attempt is used for both fetching vpc's and for
+  // fetching databases since each fetching is done at separate
+  // times and relies on one fetch result (vpcs) to be complete
+  // before performing the next fetch (databases, but only after user
+  // has selected a vpc).
   const { attempt: fetchAttempt, setAttempt: setFetchAttempt } = useAttempt('');
 
   const [vpcs, setVpcs] = useState<Vpc[] | undefined>();
@@ -90,7 +96,7 @@ export function EnrollRdsDatabase() {
       } while (nextPage);
 
       setVpcs(fetchedVpcs);
-      setFetchAttempt({ status: 'success' });
+      setFetchAttempt({ status: '' });
     } catch (err) {
       const message = getErrMessage(err);
       setFetchAttempt({
@@ -119,7 +125,7 @@ export function EnrollRdsDatabase() {
   const hasIamPermError = isIamPermError(fetchAttempt);
   const showVpcSelector = !hasIamPermError && !!vpcs;
   const showAutoEnrollToggle =
-    fetchAttempt.status === 'success' && !!selectedVpc;
+    fetchAttempt.status !== 'failed' && !!selectedVpc;
   const hasVpcs = vpcs?.length > 0;
 
   const mainContentProps = {
@@ -162,6 +168,10 @@ export function EnrollRdsDatabase() {
         />
       )}
       {showVpcSelector && !hasVpcs && (
+        // TODO(lisa): negative margin was required since the
+        // AwsRegionSelector added too much bottom margin.
+        // Refactor AwsRegionSelector so margins can be controlled
+        // outside of the component (or use flex columns with gap prop)
         <P mt={-3}>
           There are no VPCs defined in the selected region. Try another region.
         </P>
@@ -183,9 +193,12 @@ export function EnrollRdsDatabase() {
         />
       )}
       {wantAutoDiscover ? (
-        <AutoEnrollment {...mainContentProps} />
+        <AutoEnrollment {...mainContentProps} key={mainContentProps.vpc?.id} />
       ) : (
-        <SingleEnrollment {...mainContentProps} />
+        <SingleEnrollment
+          {...mainContentProps}
+          key={mainContentProps.vpc?.id}
+        />
       )}
     </Box>
   );

diff --git a/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/SingleEnrollment.tsx b/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/SingleEnrollment.tsx
@@ -25,12 +25,11 @@ import { getErrMessage } from 'shared/utils/errorType';
 import { useDiscover } from 'teleport/Discover/useDiscover';
 import {
   AwsRdsDatabase,
-  RdsEngineIdentifier,
   Regions,
   Vpc,
   integrationService,
+  getRdsEngineIdentifier,
 } from 'teleport/services/integrations';
-import { DatabaseEngine } from 'teleport/Discover/SelectResource';
 import { Database } from 'teleport/services/databases';
 
 import { ActionButtons } from '../../Shared';
@@ -85,19 +84,14 @@ export function SingleEnrollment({
 
   const { agentMeta, resourceSpec, emitErrorEvent } = useDiscover();
 
-  const [tableData, setTableData] = useState<TableData>(() => emptyTableData());
+  const [tableData, setTableData] = useState<TableData>();
   const [selectedDb, setSelectedDb] = useState<CheckedAwsRdsDatabase>();
 
   useEffect(() => {
-    if (!vpc) {
-      // Clear table, vpc will be null on new region select.
-      setTableData(emptyTableData());
-      clearAttempt();
-      setSelectedDb(null);
-      return;
+    if (vpc) {
+      // Start with empty table data for new vpc's.
+      fetchRdsDatabases(emptyTableData(), vpc);
     }
-    // Start with empty table data for new vpc's.
-    fetchRdsDatabases(emptyTableData(), vpc);
   }, [vpc]);
 
   function fetchNextPage() {
@@ -187,6 +181,7 @@ export function SingleEnrollment({
         labels: selectedDb.labels,
         awsRds: selectedDb,
         awsRegion: region,
+        awsVpcId: vpc.id,
       },
       // Corner case where if registering db fails a user can:
       //   1) change region, which will list new databases or
@@ -201,13 +196,11 @@ export function SingleEnrollment({
     <>
       {showTable && (
         <>
-          {fetchAttempt.status !== 'processing' && (
-            <Text mt={3}>Select an RDS to enroll:</Text>
-          )}
+          <Text mt={3}>Select an RDS to enroll:</Text>
           <DatabaseList
             wantAutoDiscover={false}
-            items={tableData.items}
-            fetchStatus={tableData.fetchStatus}
+            items={tableData?.items || []}
+            fetchStatus={tableData?.fetchStatus || 'loading'}
             selectedDatabase={selectedDb}
             onSelectDatabase={setSelectedDb}
             fetchNextPage={fetchNextPage}
@@ -231,16 +224,3 @@ export function SingleEnrollment({
     </>
   );
 }
-
-function getRdsEngineIdentifier(engine: DatabaseEngine): RdsEngineIdentifier {
-  switch (engine) {
-    case DatabaseEngine.MySql:
-      return 'mysql';
-    case DatabaseEngine.Postgres:
-      return 'postgres';
-    case DatabaseEngine.AuroraMysql:
-      return 'aurora-mysql';
-    case DatabaseEngine.AuroraPostgres:
-      return 'aurora-postgres';
-  }
-}