From 623f9a5ab312e7169886c646a0a3ed3d1625fdc1 Mon Sep 17 00:00:00 2001 From: Gavin Frazar Date: Thu, 24 Oct 2024 23:10:08 -0700 Subject: [PATCH] fix RDS Aurora enrollment security group tips --- lib/srv/discovery/common/database.go | 11 +++--- lib/srv/discovery/common/database_test.go | 15 +++++++- web/packages/shared/utils/text.test.ts | 2 + .../AutoDeploy/SelectSecurityGroups.tsx | 37 +++++++++---------- .../SecurityGroupPicker.tsx | 6 +-- .../integrations/integrations.test.ts | 6 +++ .../src/services/integrations/integrations.ts | 4 +- 7 files changed, 51 insertions(+), 30 deletions(-) diff --git a/lib/srv/discovery/common/database.go b/lib/srv/discovery/common/database.go index a8e2890e8fa3e..4eb2caf81ce71 100644 --- a/lib/srv/discovery/common/database.go +++ b/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 } diff --git a/lib/srv/discovery/common/database_test.go b/lib/srv/discovery/common/database_test.go index 1d1f145a6903d..c7f0fbadf9626 100644 --- a/lib/srv/discovery/common/database_test.go +++ b/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", }, }, }) diff --git a/web/packages/shared/utils/text.test.ts b/web/packages/shared/utils/text.test.ts index f9dc2729ff7db..0cd297f7b20d5 100644 --- a/web/packages/shared/utils/text.test.ts +++ b/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', () => { diff --git a/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx b/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx index 37bac8790a36c..b11ce6f538237 100644 --- a/web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/SelectSecurityGroups.tsx +++ b/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,21 +262,24 @@ function allowsOutboundToPorts(rule: SecurityGroupRule): boolean { function getTrustedSecurityGroups( securityGroups: SecurityGroup[], - db: AwsRdsDatabase + db?: AwsRdsDatabase ): Set { + const trustedGroups = new Set(); + 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(); 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; } @@ -288,7 +287,7 @@ function getTrustedSecurityGroups( // 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); }); }); diff --git a/web/packages/teleport/src/Discover/Shared/SecurityGroupPicker/SecurityGroupPicker.tsx b/web/packages/teleport/src/Discover/Shared/SecurityGroupPicker/SecurityGroupPicker.tsx index 18ef801517be2..d5749551ca417 100644 --- a/web/packages/teleport/src/Discover/Shared/SecurityGroupPicker/SecurityGroupPicker.tsx +++ b/web/packages/teleport/src/Discover/Shared/SecurityGroupPicker/SecurityGroupPicker.tsx @@ -67,7 +67,7 @@ export const SecurityGroupPicker = ({ useState(); 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 ( @@ -173,7 +173,7 @@ export const SecurityGroupPicker = ({ ); } - return null; + return ; }, }, ] diff --git a/web/packages/teleport/src/services/integrations/integrations.test.ts b/web/packages/teleport/src/services/integrations/integrations.test.ts index 3d647ff7a5e46..ed9e65c7d9d2c 100644 --- a/web/packages/teleport/src/services/integrations/integrations.test.ts +++ b/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', diff --git a/web/packages/teleport/src/services/integrations/integrations.ts b/web/packages/teleport/src/services/integrations/integrations.ts index e4da3ee126944..8179f020ff739 100644 --- a/web/packages/teleport/src/services/integrations/integrations.ts +++ b/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, };