[v16] fix RDS Aurora enrollment security group tips

lib/srv/discovery/common/database.go

@@ -466,11 +466,12 @@ func MetadataFromRDSV2Cluster(rdsCluster *rdstypes.DBCluster, rdsInstance *rdsty
 		Region:    parsedARN.Region,
 		AccountID: parsedARN.AccountID,
 		RDS: types.RDS{
-			ClusterID:  aws.StringValue(rdsCluster.DBClusterIdentifier),
-			ResourceID: aws.StringValue(rdsCluster.DbClusterResourceId),
-			IAMAuth:    aws.BoolValue(rdsCluster.IAMDatabaseAuthenticationEnabled),
-			Subnets:    subnets,
-			VPCID:      vpcID,
+			ClusterID:      aws.StringValue(rdsCluster.DBClusterIdentifier),
+			ResourceID:     aws.StringValue(rdsCluster.DbClusterResourceId),
+			IAMAuth:        aws.BoolValue(rdsCluster.IAMDatabaseAuthenticationEnabled),
+			Subnets:        subnets,
+			VPCID:          vpcID,
+			SecurityGroups: rdsSecurityGroupInfo(rdsCluster.VpcSecurityGroups),
 		},
 	}, nil
 }

lib/srv/discovery/common/database_test.go

@@ -572,6 +572,11 @@ func TestDatabaseFromRDSV2Cluster(t *testing.T) {
 		Endpoint:                         aws.String("localhost"),
 		ReaderEndpoint:                   aws.String("reader.host"),
 		Port:                             aws.Int32(3306),
+		VpcSecurityGroups: []rdsTypesV2.VpcSecurityGroupMembership{
+			{VpcSecurityGroupId: aws.String("")},
+			{VpcSecurityGroupId: aws.String("sg-1")},
+			{VpcSecurityGroupId: aws.String("sg-2")},
+		},
 		CustomEndpoints: []string{
 			"myendpoint1.cluster-custom-example.us-east-1.rds.amazonaws.com",
 			"myendpoint2.cluster-custom-example.us-east-1.rds.amazonaws.com",
@@ -589,6 +594,10 @@ func TestDatabaseFromRDSV2Cluster(t *testing.T) {
 			ClusterID:  "cluster-1",
 			ResourceID: "resource-1",
 			IAMAuth:    true,
+			SecurityGroups: []string{
+				"sg-1",
+				"sg-2",
+			},
 		},
 	}
 
@@ -673,7 +682,11 @@ func TestDatabaseFromRDSV2Cluster(t *testing.T) {
 					ResourceID: "resource-1",
 					IAMAuth:    true,
 					Subnets:    []string{"subnet-123", "subnet-456"},
-					VPCID:      "vpc-123",
+					SecurityGroups: []string{
+						"sg-1",
+						"sg-2",
+					},
+					VPCID: "vpc-123",
 				},
 			},
 		})

web/packages/shared/utils/text.test.ts

@@ -22,6 +22,8 @@ test('pluralize', () => {
   expect(pluralize(0, 'apple')).toBe('apples');
   expect(pluralize(1, 'apple')).toBe('apple');
   expect(pluralize(2, 'apple')).toBe('apples');
+  expect(pluralize(undefined, 'apple')).toBe('apples');
+  expect(pluralize(null, 'apple')).toBe('apples');
 });
 
 test('capitalizeFirstLetter', () => {

web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx

@@ -201,20 +201,17 @@ export const SelectSecurityGroups = ({
 
 function withTips(
   securityGroups: SecurityGroup[],
-  db: AwsRdsDatabase
+  db?: AwsRdsDatabase
 ): SecurityGroupWithRecommendation[] {
-  if (!db || !securityGroups) {
-    // return early without trying to add tips if there's no selected db or
-    // security groups given.
-    return securityGroups;
-  }
+  // if db is undefined, which is possible in the auto-discovery flow, we can
+  // still recommend security groups that allow outbound internet access.
   const trustedGroups = getTrustedSecurityGroups(securityGroups, db);
   return securityGroups.map(group => {
     const isTrusted = trustedGroups.has(group.id);
     const isOutboundAllowed = allowsOutbound(group);
     return {
       ...group,
-      tips: getTips(db, isTrusted, isOutboundAllowed),
+      tips: getTips(isTrusted, isOutboundAllowed),
       // we recommend when either are true because security group rules are
       // additive, meaning they can select multiple groups for a combined effect
       // of satisfying the database inbound rules and the ECS task outbound
@@ -224,15 +221,11 @@ function withTips(
   });
 }
 
-function getTips(
-  db: AwsRdsDatabase,
-  isTrusted: boolean,
-  allowsOutbound: boolean
-): string[] {
+function getTips(isTrusted: boolean, allowsOutbound: boolean): string[] {
   const result: string[] = [];
   if (isTrusted) {
     result.push(
-      `The inbound rules of the database ${pluralize(db.securityGroups.length, 'security group')} allow traffic from this security group`
+      'The database security group inbound rules allow traffic from this security group'
     );
   }
   if (allowsOutbound) {
@@ -243,6 +236,9 @@ function getTips(
 
 function allowsOutbound(sg: SecurityGroup): boolean {
   return sg.outboundRules.some(rule => {
+    if (!rule) {
+      return false;
+    }
     const havePorts = allowsOutboundToPorts(rule);
     // this is a heuristic, because an exhaustive analysis is non-trivial.
     return havePorts && rule.cidrs.some(cidr => cidr.cidr === '0.0.0.0/0');
@@ -266,29 +262,32 @@ function allowsOutboundToPorts(rule: SecurityGroupRule): boolean {
 
 function getTrustedSecurityGroups(
   securityGroups: SecurityGroup[],
-  db: AwsRdsDatabase
+  db?: AwsRdsDatabase
 ): Set<string> {
+  const trustedGroups = new Set<string>();
+  if (!db || !db.securityGroups || !db.uri) {
+    return trustedGroups;
+  }
+
+  const dbPort = getPort(db);
   const securityGroupsById = new Map(
     securityGroups.map(group => [group.id, group])
   );
-
-  const dbPort = getPort(db);
-  const trustedGroups = new Set<string>();
   db.securityGroups.forEach(groupId => {
     const group = securityGroupsById.get(groupId);
     if (!group) {
       return;
     }
     group.inboundRules.forEach(rule => {
-      if (!rule.groups?.length) {
+      if (!rule.groups.length) {
         // we only care about rules that reference other security groups.
         return;
       }
       if (!ruleAllowsPort(rule, dbPort)) {
         // a group is only trusted if it is trusted for the relevant port.
         return;
       }
-      rule.groups?.forEach(({ groupId }) => {
+      rule.groups.forEach(({ groupId }) => {
         trustedGroups.add(groupId);
       });
     });

web/packages/teleport/src/Discover/Shared/SecurityGroupPicker/SecurityGroupPicker.tsx

@@ -67,7 +67,7 @@ export const SecurityGroupPicker = ({
     useState<ViewRulesSelection>();
 
   function onCloseRulesDialog() {
-    setViewRulesSelection(null);
+    setViewRulesSelection(undefined);
   }
 
   if (attempt.status === 'failed') {
@@ -160,7 +160,7 @@ export const SecurityGroupPicker = ({
                   altKey: 'tooltip',
                   headerText: '',
                   render: (sg: SecurityGroupWithRecommendation) => {
-                    if (sg.recommended) {
+                    if (sg.recommended && sg.tips?.length) {
                       return (
                         <Cell>
                           <ToolTipInfo>
@@ -173,7 +173,7 @@ export const SecurityGroupPicker = ({
                         </Cell>
                       );
                     }
-                    return null;
+                    return <Cell />;
                   },
                 },
               ]

web/packages/teleport/src/services/integrations/integrations.test.ts

@@ -121,6 +121,8 @@ test('fetchAwsDatabases response', async () => {
         accountId: 'account-id-1',
         resourceId: 'resource-id-1',
         vpcId: 'vpc-123',
+        subnets: [],
+        securityGroups: [],
       },
       {
         engine: 'mysql',
@@ -131,6 +133,8 @@ test('fetchAwsDatabases response', async () => {
         accountId: undefined,
         resourceId: undefined,
         vpcId: undefined,
+        subnets: [],
+        securityGroups: [],
       },
       {
         engine: 'mysql',
@@ -141,6 +145,8 @@ test('fetchAwsDatabases response', async () => {
         accountId: undefined,
         resourceId: undefined,
         vpcId: undefined,
+        subnets: [],
+        securityGroups: [],
       },
     ],
     nextToken: 'next-token',

web/packages/teleport/src/services/integrations/integrations.ts

@@ -439,10 +439,10 @@ export function makeAwsDatabase(json: any): AwsRdsDatabase {
     uri,
     status: aws?.status,
     labels: labels ?? [],
-    subnets: aws?.rds?.subnets,
+    subnets: aws?.rds?.subnets ?? [],
     resourceId: aws?.rds?.resource_id,
     vpcId: aws?.rds?.vpc_id,
-    securityGroups: aws?.rds?.security_groups,
+    securityGroups: aws?.rds?.security_groups ?? [],
     accountId: aws?.account_id,
     region: aws?.region,
   };