From cfbf007b3cb487e58013ff93eb3eb3bb65d3e636 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Fri, 29 Oct 2021 14:07:59 -0400 Subject: [PATCH 01/13] feat: integrating appstream with rstudio-alb --- .../application-load-balancer.cfn.yml | 66 ++++++++++++------- .../src/templates/onboard-account.cfn.yml | 54 +++++++++++++-- .../lib/alb/__tests__/alb-service.test.js | 15 +++-- .../base-raas-services/lib/alb/alb-service.js | 57 +++++++++++----- 4 files changed, 144 insertions(+), 48 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index 698c0a66b1..472a1dc20d 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -18,41 +18,57 @@ Parameters: ACMSSLCertARN: Type: String Description: The ARN of the AWS Certificate Manager SSL Certificate to associate with the Load Balancer + IsAppStreamEnabled: + Type: String + AllowedValues: [true, false] + Description: Is AppStream enabled for this workspace + +Conditions: + AppStreamEnabled: !Equals [!Ref IsAppStreamEnabled, 'true'] + AppStreamDisabled: !Equals [!Ref IsAppStreamEnabled, 'false'] + Resources: ALBListener: - Type: AWS::ElasticLoadBalancingV2::Listener - Properties: - DefaultActions: - - Type: fixed-response - FixedResponseConfig: - ContentType: "text/plain" - MessageBody: "Forbidden" - StatusCode: "403" - LoadBalancerArn: - Ref: ApplicationLoadBalancer - Port: 443 - Protocol: HTTPS - SslPolicy: ELBSecurityPolicy-2016-08 - Certificates: - - CertificateArn: !Ref ACMSSLCertARN + Type: AWS::ElasticLoadBalancingV2::Listener + Properties: + DefaultActions: + - Type: fixed-response + FixedResponseConfig: + ContentType: 'text/plain' + MessageBody: 'Forbidden' + StatusCode: '403' + LoadBalancerArn: + Ref: ApplicationLoadBalancer + Port: 443 + Protocol: HTTPS + SslPolicy: ELBSecurityPolicy-2016-08 + Certificates: + - CertificateArn: !Ref ACMSSLCertARN ApplicationLoadBalancer: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Name: !Ref Namespace - Scheme: internet-facing # or internal + Scheme: + - !If + - AppStreamEnabled + - internal + - internet-facing Subnets: - - Ref: Subnet1 - - Ref: Subnet2 - SecurityGroups: - - Ref: ALBSecurityGroup + - Ref: Subnet1 + - Ref: Subnet2 + SecurityGroups: + - Ref: ALBSecurityGroup ALBSecurityGroup: Type: AWS::EC2::SecurityGroup Properties: SecurityGroupIngress: - - CidrIp: "0.0.0.0/0" - FromPort: 443 - ToPort: 443 - IpProtocol: tcp + - !If + - AppStreamEnabled + - !Ref 'AWS::NoValue' + - CidrIp: '0.0.0.0/0' + FromPort: 443 + ToPort: 443 + IpProtocol: tcp GroupDescription: ALB SecurityGroup VpcId: !Ref VPC Outputs: @@ -67,4 +83,4 @@ Outputs: Value: !Ref ALBListener ALBSecurityGroupId: Description: Security Group of Application Load Balancer Listener - Value: !Ref ALBSecurityGroup \ No newline at end of file + Value: !Ref ALBSecurityGroup diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml index ca87aa58e6..e984a1e336 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml @@ -40,7 +40,7 @@ Parameters: Type: String Default: 10.0.0.0/19 - VpcPublicSubnet2Cidr: + PublicSubnet2Cidr: Description: Please enter the IP range (CIDR notation) for the public subnet 2 in the 2nd Availability Zone Type: String Default: 10.0.32.0/19 @@ -67,6 +67,12 @@ Parameters: Description: Please enter the IP range (CIDR notation) for the Workspace subnet. This value is only used if AppStream is enabled. Type: String Default: 10.0.64.0/19 + + # Range from 10.0.96.0 to 10.0.127.255 + WorkspaceSubnet2Cidr: + Description: Please enter the IP range (CIDR notation) for the Workspace subnet 2. This value is only used if AppStream is enabled. + Type: String + Default: 10.0.96.0/19 AppStreamFleetDesiredInstances: Description: The desired number of streaming instances. @@ -123,9 +129,9 @@ Metadata: default: Deployment Configuration Parameters: - VpcCidr - - VpcPublicSubnet1Cidr - - VpcPublicSubnet2Cidr - PublicSubnetCidr + - PublicSubnet2Cidr + Conditions: isAppStream: !Equals - !Ref EnableAppStream @@ -629,10 +635,11 @@ Resources: PublicSubnet2: Type: AWS::EC2::Subnet + Condition: isNotAppStream Properties: VpcId: !Ref VPC AvailabilityZone: !Select [1, !GetAZs ] - CidrBlock: !Ref VpcPublicSubnet2Cidr + CidrBlock: !Ref PublicSubnet2Cidr MapPublicIpOnLaunch: true Tags: - Key: Name @@ -665,6 +672,7 @@ Resources: PublicSubnet2RouteTableAssociation: Type: AWS::EC2::SubnetRouteTableAssociation + Condition: isNotAppStream Properties: RouteTableId: !Ref PublicRouteTable SubnetId: !Ref PublicSubnet2 @@ -732,6 +740,19 @@ Resources: Tags: - Key: Name Value: Private Workspace Subnet + + # For an ALB - You must specify subnets from at least two Availability Zones. + PrivateWorkspaceSubnet2: + Type: AWS::EC2::Subnet + Condition: isAppStreamAndCustomDomain + Properties: + VpcId: !Ref VPC + AvailabilityZone: !Select [1, !GetAZs ''] + CidrBlock: !Ref WorkspaceSubnet2Cidr + MapPublicIpOnLaunch: false + Tags: + - Key: Name + Value: Private Workspace Subnet 2 PrivateWorkspaceRouteTable: Type: 'AWS::EC2::RouteTable' @@ -788,6 +809,10 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet + - !If + - isAppStreamAndCustomDomain + - !Ref PrivateWorkspaceSubnet2 + - !Ref 'AWS::NoValue' PrivateDnsEnabled: true VpcEndpointType: Interface ServiceName: !Sub 'com.amazonaws.${AWS::Region}.kms' @@ -801,6 +826,10 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet + - !If + - isAppStreamAndCustomDomain + - !Ref PrivateWorkspaceSubnet2 + - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.sts' @@ -814,6 +843,10 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet + - !If + - isAppStreamAndCustomDomain + - !Ref PrivateWorkspaceSubnet2 + - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.ec2' @@ -827,6 +860,10 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet + - !If + - isAppStreamAndCustomDomain + - !Ref PrivateWorkspaceSubnet2 + - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.cloudformation' @@ -883,6 +920,10 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet + - !If + - isAppStreamAndCustomDomain + - !Ref PrivateWorkspaceSubnet2 + - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.ssm' @@ -1057,6 +1098,11 @@ Outputs: Description: Workspace subnet Condition: isAppStream Value: !Ref PrivateWorkspaceSubnet + + PrivateWorkspaceSubnet2: + Description: Second subnet for ALB + Condition: isAppStreamAndCustomDomain + Value: !Ref PrivateWorkspaceSubnet2 AppStreamSecurityGroup: Description: AppStream Security Group diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index cee73d70a1..486f6d280e 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -253,7 +253,10 @@ describe('ALBService', () => { }); describe('getStackCreationInput', () => { - const resolvedInputParams = [{ Key: 'ACMSSLCertARN', Value: 'Value' }]; + const resolvedInputParams = [ + { Key: 'ACMSSLCertARN', Value: 'Value' }, + { Key: 'IsAppStreamEnabled', Value: 'false' }, + ]; const resolvedVars = { namespace: 'namespace' }; it('should pass and return the stack creation input with success', async () => { service.findAwsAccountDetails = jest.fn(() => { @@ -291,6 +294,10 @@ describe('ALBService', () => { ParameterKey: 'VPC', ParameterValue: 'vpc-096b034133955abba', }, + { + ParameterKey: 'IsAppStreamEnabled', + ParameterValue: 'false', + }, ], TemplateBody: ['template'], Tags: [ @@ -649,7 +656,7 @@ describe('ALBService', () => { throw new Error(`Error describing subnet. VPC does not exist`); }); try { - await service.findSubnet2({}, {}, ''); + await service.findSubnet2({}, {}, '', 'true'); } catch (err) { expect(err.message).toContain('Error describing subnet. VPC does not exist'); } @@ -664,7 +671,7 @@ describe('ALBService', () => { }; }); try { - await service.findSubnet2({}, {}, 'test-vpc'); + await service.findSubnet2({}, {}, 'test-vpc', 'false'); } catch (err) { expect(err.message).toContain( 'Error provisioning environment. Reason: Subnet2 not found for the VPC - test-vpc', @@ -680,7 +687,7 @@ describe('ALBService', () => { }, }; }); - const response = await service.findSubnet2({}, {}, 'test-vpc'); + const response = await service.findSubnet2({}, {}, 'test-vpc', true); expect(response).toEqual('test-subnet-id'); }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index 876edd6a61..ba04130748 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -84,11 +84,17 @@ class ALBService extends Service { */ async getStackCreationInput(requestContext, resolvedVars, resolvedInputParams, projectId) { const awsAccountDetails = await this.findAwsAccountDetails(requestContext, projectId); - const subnet2 = await this.findSubnet2(requestContext, resolvedVars, awsAccountDetails.vpcId); const [cfnTemplateService] = await this.service(['cfnTemplateService']); const [template] = await Promise.all([cfnTemplateService.getTemplate('application-load-balancer')]); const cfnParams = []; const certificateArn = _.find(resolvedInputParams, o => o.Key === 'ACMSSLCertARN'); + const isAppStreamEnabled = _.find(resolvedInputParams, o => o.Key === 'IsAppStreamEnabled'); + const subnet2 = await this.findSubnet2( + requestContext, + resolvedVars, + awsAccountDetails.vpcId, + isAppStreamEnabled.Value, + ); const addParam = (key, v) => cfnParams.push({ ParameterKey: key, ParameterValue: v }); addParam('Namespace', resolvedVars.namespace); @@ -96,6 +102,7 @@ class ALBService extends Service { addParam('Subnet2', subnet2); addParam('ACMSSLCertARN', certificateArn.Value); addParam('VPC', awsAccountDetails.vpcId); + addParam('IsAppStreamEnabled', isAppStreamEnabled.Value); const input = { StackName: resolvedVars.namespace, @@ -327,26 +334,46 @@ class ALBService extends Service { } /** - * Method to get the EC2 SDK client for the target aws account + * Method to get the second subnet ID + * For an ALB - You must specify subnets from at least two Availability Zones. * * @param requestContext * @param resolvedVars * @param vpcId + * @param appStreamEnabled * @returns {Promise} */ - async findSubnet2(requestContext, resolvedVars, vpcId) { - const params = { - Filters: [ - { - Name: 'vpc-id', - Values: [vpcId], - }, - { - Name: 'tag:aws:cloudformation:logical-id', - Values: ['PublicSubnet2'], - }, - ], - }; + async findSubnet2(requestContext, resolvedVars, vpcId, appStreamEnabled) { + const isAppStreamEnabled = appStreamEnabled === 'true'; + let params; + if (isAppStreamEnabled) { + params = { + Filters: [ + { + Name: 'vpc-id', + Values: [vpcId], + }, + { + Name: 'tag:aws:cloudformation:logical-id', + Values: ['PrivateWorkspaceSubnet2'], + }, + ], + }; + } else { + params = { + Filters: [ + { + Name: 'vpc-id', + Values: [vpcId], + }, + { + Name: 'tag:aws:cloudformation:logical-id', + Values: ['PublicSubnet2'], + }, + ], + }; + } + const ec2Client = await this.getEc2Sdk(requestContext, resolvedVars); const response = await ec2Client.describeSubnets(params).promise(); const subnetId = _.get(response.Subnets[0], 'SubnetId', null); From d1cd2f3c1ecdee25d1c5c7c7c59756ba4cba53d0 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Fri, 29 Oct 2021 14:24:30 -0400 Subject: [PATCH 02/13] fix: scheme needs to be string --- .../src/templates/application-load-balancer.cfn.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index 472a1dc20d..53f998f3fa 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -51,8 +51,8 @@ Resources: Scheme: - !If - AppStreamEnabled - - internal - - internet-facing + - 'internal' + - 'internet-facing' Subnets: - Ref: Subnet1 - Ref: Subnet2 From 484e5091dae8b4fea4b92fe99727445a070979d0 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Fri, 29 Oct 2021 17:38:44 -0400 Subject: [PATCH 03/13] feat: ALB rules for AppStream RStudio --- .../application-load-balancer.cfn.yml | 6 +- .../base-raas-services/lib/alb/alb-service.js | 77 ++++++++++++------- .../lib/plugins/env-provisioning-plugin.js | 3 +- .../steps/launch-product/launch-product.js | 2 +- .../terminate-launch-dependency.js | 16 +++- 5 files changed, 66 insertions(+), 38 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index 53f998f3fa..a24cbe9bc2 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -48,11 +48,7 @@ Resources: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Name: !Ref Namespace - Scheme: - - !If - - AppStreamEnabled - - 'internal' - - 'internet-facing' + Scheme: !If [AppStreamEnabled, 'internal', 'internet-facing'] Subnets: - Ref: Subnet1 - Ref: Subnet2 diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index ba04130748..45b0767030 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -18,6 +18,7 @@ const Service = require('@aws-ee/base-services-container/lib/service'); const settingKeys = { domainName: 'domainName', + isAppStreamEnabled: 'isAppStreamEnabled', }; class ALBService extends Service { @@ -201,6 +202,7 @@ class ALBService extends Service { * @returns {Promise} */ async createListenerRule(prefix, requestContext, resolvedVars, targetGroupArn) { + const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); const deploymentItem = await this.getAlbDetails(requestContext, resolvedVars.projectId); const albRecord = JSON.parse(deploymentItem.value); const listenerArn = albRecord.listenerArn; @@ -215,20 +217,29 @@ class ALBService extends Service { Type: 'forward', }, ], - Conditions: [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, - }, - { - Field: 'source-ip', - SourceIpConfig: { - Values: [resolvedVars.cidr], - }, - }, - ], + Conditions: isAppStreamEnabled + ? [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + ] + : [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: [resolvedVars.cidr], + }, + }, + ], Tags: resolvedVars.tags, }; const albClient = await this.getAlbSdk(requestContext, resolvedVars); @@ -454,22 +465,32 @@ class ALBService extends Service { */ async modifyRule(requestContext, resolvedVars) { const subdomain = this.getHostname(resolvedVars.prefix, resolvedVars.envId); + const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); try { const params = { - Conditions: [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, - }, - { - Field: 'source-ip', - SourceIpConfig: { - Values: resolvedVars.cidr, - }, - }, - ], + Conditions: isAppStreamEnabled + ? [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + ] + : [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: [resolvedVars.cidr], + }, + }, + ], RuleArn: resolvedVars.ruleARN, }; const { externalId } = await this.findAwsAccountDetails(requestContext, resolvedVars.projectId); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js index 6b8d416e7b..85bc65b8ad 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js @@ -242,9 +242,8 @@ async function updateEnvOnProvisioningSuccess({ const environmentDnsService = await container.find('environmentDnsService'); const settings = await container.find('settings'); if (settings.getBoolean(settingKeys.isAppStreamEnabled)) { - const privateIp = _.find(outputs, o => o.OutputKey === 'Ec2WorkspacePrivateIp').OutputValue; const hostedZoneId = await getHostedZone(requestContext, environmentScService, existingEnvRecord); - await environmentDnsService.createPrivateRecord(requestContext, 'rstudio', envId, privateIp, hostedZoneId); + await environmentDnsService.createPrivateRecord(requestContext, 'rstudio', envId, dnsName, hostedZoneId); } else { await environmentDnsService.createRecord('rstudio', envId, dnsName); } diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/launch-product/launch-product.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/launch-product/launch-product.js index 328482c9ca..f7863a29d5 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/launch-product/launch-product.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/launch-product/launch-product.js @@ -394,4 +394,4 @@ class LaunchProduct extends StepBase { } } -module.exports = LaunchProduct; \ No newline at end of file +module.exports = LaunchProduct; diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index dee5bd1c15..0991bc7aaa 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -93,11 +93,23 @@ class TerminateLaunchDependency extends StepBase { const albExists = await albService.checkAlbExists(requestContext, projectId); const deploymentItem = await albService.getAlbDetails(requestContext, projectId); const deploymentValue = JSON.parse(deploymentItem.value); + const dnsName = deploymentValue.albDnsName; if (albExists) { try { - const dnsName = deploymentValue.albDnsName; - await environmentDnsService.deleteRecord('rstudio', envId, dnsName); + const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); + if (isAppStreamEnabled) { + const memberAccount = await environmentScService.getMemberAccount(requestContext, environment); + await environmentDnsService.deletePrivateRecord( + requestContext, + 'rstudio', + envId, + dnsName, + memberAccount.route53HostedZone, + ); + } else { + await environmentDnsService.deleteRecord('rstudio', envId, dnsName); + } this.print({ msg: 'Route53 record deleted successfully', }); From c2d6fe9983d9e91ebb832b92e2f010a860adb2c3 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Tue, 2 Nov 2021 22:58:42 -0400 Subject: [PATCH 04/13] feat: appstream integration for RStudio ALB --- .../application-load-balancer.cfn.yml | 3 + .../src/templates/onboard-account.cfn.yml | 13 +-- .../lib/alb/__tests__/alb-service.test.js | 4 +- .../base-raas-services/lib/alb/alb-service.js | 8 +- .../environment/environment-dns-service.js | 72 ++++++++++++++ .../lib/plugins/env-provisioning-plugin.js | 12 ++- .../check-launch-dependency.js | 84 ++++++++++++++++ .../terminate-launch-dependency.js | 98 ++++++++++++++++++- 8 files changed, 279 insertions(+), 15 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index a24cbe9bc2..e8820555ad 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -74,6 +74,9 @@ Outputs: ALBDNSName: Description: DNS Name of Application Load Balancer Value: !GetAtt ApplicationLoadBalancer.DNSName + ALBHostedZoneId: + Description: Hosted Zone ID of Application Load Balancer + Value: !GetAtt ApplicationLoadBalancer.HostedZoneId ListenerArn: Description: ARN of Application Load Balancer Listener Value: !Ref ALBListener diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml index e984a1e336..c02c60bd80 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml @@ -137,6 +137,7 @@ Conditions: - !Ref EnableAppStream - true isNotAppStream: !Not [Condition: isAppStream] + hasCustomDomain: !Not [!Equals [!Ref "DomainName", ""]] isAppStreamAndCustomDomain: !And - !Not [!Equals [!Ref "DomainName", ""]] - !Condition isAppStream @@ -190,8 +191,8 @@ Resources: - ec2:DescribeImages - ec2:DescribeInstances - ec2:DescribeSecurityGroups - - ec2:RevokeSecurityGroupIngress - - ec2:AuthorizeSecurityGroupIngress + - ec2:RevokeSecurityGroup* + - ec2:AuthorizeSecurityGroup* - ec2-instance-connect:SendSSHPublicKey Resource: '*' - PolicyName: cfn-access @@ -453,8 +454,8 @@ Resources: Action: - ec2:CreateSecurityGroup - ec2:DeleteSecurityGroup - - ec2:AuthorizeSecurityGroupIngress - - ec2:RevokeSecurityGroupIngress + - ec2:AuthorizeSecurityGroup* + - ec2:RevokeSecurityGroup* Resource: - !Sub 'arn:aws:ec2:${AWS::Region}:${AWS::AccountId}:vpc/*' - !Sub 'arn:aws:ec2:${AWS::Region}:${AWS::AccountId}:security-group/*' @@ -992,7 +993,7 @@ Resources: IpProtocol: '-1' - DestinationSecurityGroupId: !Ref WorkspaceSecurityGroup IpProtocol: '-1' - + AppStreamSecurityGroupEgress: Type: AWS::EC2::SecurityGroupEgress Condition: isAppStream @@ -1115,7 +1116,7 @@ Outputs: Description: SageMaker Security Group Condition: isAppStream Value: !Ref SageMakerSecurityGroup - + AppStreamFleet: Description: AppStream Fleet Condition: isAppStream diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index 486f6d280e..b8765c34af 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -93,6 +93,9 @@ describe('ALBService', () => { }; service.getAlbSdk = jest.fn().mockResolvedValue(albClient); service.getEc2Sdk = jest.fn().mockResolvedValue(ec2Client); + service.checkIfAppStreamEnabled = jest.fn(() => { + return false; + }); }); afterEach(() => { @@ -557,7 +560,6 @@ describe('ALBService', () => { }); service.getAlbSdk = jest.fn().mockResolvedValue(albClient); const response = await service.modifyRule({}, resolvedVars); - expect(albClient.modifyRule).toHaveBeenCalledWith(params); expect(response).toEqual({}); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index 45b0767030..0758e1922d 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -191,6 +191,10 @@ class ALBService extends Service { return deploymentItem; } + async checkIfAppStreamEnabled() { + return this.settings.get(settingKeys.isAppStreamEnabled); + } + /** * Method to create listener rule. The method creates rule using the ALB SDK client. * Tags are read form the resolvedVars so the billing will happen properly @@ -202,7 +206,7 @@ class ALBService extends Service { * @returns {Promise} */ async createListenerRule(prefix, requestContext, resolvedVars, targetGroupArn) { - const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); + const isAppStreamEnabled = this.checkIfAppStreamEnabled(); const deploymentItem = await this.getAlbDetails(requestContext, resolvedVars.projectId); const albRecord = JSON.parse(deploymentItem.value); const listenerArn = albRecord.listenerArn; @@ -465,7 +469,7 @@ class ALBService extends Service { */ async modifyRule(requestContext, resolvedVars) { const subdomain = this.getHostname(resolvedVars.prefix, resolvedVars.envId); - const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); + const isAppStreamEnabled = this.checkIfAppStreamEnabled(); try { const params = { Conditions: isAppStreamEnabled diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js index 2ada1961fd..2c09099e6d 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js @@ -42,6 +42,24 @@ class EnvironmentDnsService extends Service { await this.changeResourceRecordSets(route53Client, hostedZoneId, action, subdomain, 'A', privateIp); } + async changePrivateRecordSetALB(requestContext, action, prefix, id, hostedZoneId, albHostedZoneId, recordValue) { + const environmentScService = await this.service('environmentScService'); + const route53Client = await environmentScService.getClientSdkWithEnvMgmtRole( + requestContext, + { id }, + { clientName: 'Route53', options: { apiVersion: '2017-07-24' } }, + ); + const subdomain = this.getHostname(prefix, id); + await this.changeResourceRecordSetsPrivateALB( + route53Client, + hostedZoneId, + action, + subdomain, + albHostedZoneId, + recordValue, + ); + } + async changeRecordSet(action, prefix, id, publicDnsName) { const aws = await this.service('aws'); const route53Client = new aws.sdk.Route53(); @@ -70,6 +88,60 @@ class EnvironmentDnsService extends Service { await route53Client.changeResourceRecordSets(params).promise(); } + async changeResourceRecordSetsPrivateALB( + route53Client, + hostedZoneId, + action, + subdomain, + albHostedZoneId, + recordValue, + ) { + const params = { + HostedZoneId: hostedZoneId, + ChangeBatch: { + Changes: [ + { + Action: action, + ResourceRecordSet: { + Name: subdomain, + Type: 'A', + AliasTarget: { + HostedZoneId: albHostedZoneId, + DNSName: recordValue, + EvaluateTargetHealth: false, + }, + }, + }, + ], + }, + }; + await route53Client.changeResourceRecordSets(params).promise(); + } + + async createPrivateRecordForDNS(requestContext, prefix, id, albHostedZoneId, albDnsName, hostedZoneId) { + await this.changePrivateRecordSetALB( + requestContext, + 'CREATE', + prefix, + id, + hostedZoneId, + albHostedZoneId, + albDnsName, + ); + } + + async deletePrivateRecordForDNS(requestContext, prefix, id, albHostedZoneId, albDnsName, hostedZoneId) { + await this.changePrivateRecordSetALB( + requestContext, + 'DELETE', + prefix, + id, + hostedZoneId, + albHostedZoneId, + albDnsName, + ); + } + async createPrivateRecord(requestContext, prefix, id, privateIp, hostedZoneId) { await this.changePrivateRecordSet(requestContext, 'CREATE', prefix, id, privateIp, hostedZoneId); } diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js index 85bc65b8ad..23a9790bc0 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js @@ -236,14 +236,22 @@ async function updateEnvOnProvisioningSuccess({ if (!deploymentItem) { throw new Error(`Error provisioning environment. Reason: No ALB found for this AWS account`); } - const dnsName = JSON.parse(deploymentItem.value).albDnsName; + const deploymentValue = JSON.parse(deploymentItem.value); + const dnsName = deploymentValue.albDnsName; const targetGroupArn = _.find(outputs, o => o.OutputKey === 'TargetGroupARN').OutputValue; // Create DNS record for RStudio workspaces const environmentDnsService = await container.find('environmentDnsService'); const settings = await container.find('settings'); if (settings.getBoolean(settingKeys.isAppStreamEnabled)) { const hostedZoneId = await getHostedZone(requestContext, environmentScService, existingEnvRecord); - await environmentDnsService.createPrivateRecord(requestContext, 'rstudio', envId, dnsName, hostedZoneId); + await environmentDnsService.createPrivateRecordForDNS( + requestContext, + 'rstudio', + envId, + deploymentValue.albHostedZoneId, + dnsName, + hostedZoneId, + ); } else { await environmentDnsService.createRecord('rstudio', envId, dnsName); } diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js index e73b5b0e16..82ad04b464 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js @@ -39,6 +39,7 @@ const outPayloadKeys = { const settingKeys = { envMgmtRoleArn: 'envMgmtRoleArn', + isAppStreamEnabled: 'isAppStreamEnabled', }; const pluginConstants = { @@ -215,10 +216,16 @@ class CheckLaunchDependency extends StepBase { listenerArn: _.get(stackOutputs, 'ListenerArn', null), albDnsName: _.get(stackOutputs, 'ALBDNSName', null), albSecurityGroup: _.get(stackOutputs, 'ALBSecurityGroupId', null), + albHostedZoneId: _.get(stackOutputs, 'ALBHostedZoneId', null), albDependentWorkspacesCount: 0, }; if (albLock) { await albService.saveAlbDetails(awsAccountId, albDetails); + if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { + // Allow ALB and AppStream security groups to interact with each other + await this.authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); + await this.authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); + } } else { throw new Error(`Error provisioning environment. Reason: ALB lock does not exist or expired`); } @@ -227,6 +234,70 @@ class CheckLaunchDependency extends StepBase { }); } + /** + * Method to allow ingress access from AppStream to ALB + * + * @param requestContext + * @param resolvedVars + * @param albDetails + */ + async authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails) { + try { + // Assign AppStream security group to ALB security group ingress + const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); + const params = { + GroupId: albDetails.albSecurityGroup, + IpPermissions: [ + { + IpProtocol: '-1', + UserIdGroupPairs: [ + { + GroupId: appStreamSecurityGroupId, + }, + ], + }, + ], + }; + const [albService] = await this.mustFindServices(['albService']); + const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); + await ec2Client.authorizeSecurityGroupEgress(params).promise(); + } catch (e) { + throw new Error(`Assigning AppStream security group to ALB security group failed with error - ${e.message}`); + } + } + + /** + * Method to allow egress access from AppStream to ALB + * + * @param requestContext + * @param resolvedVars + * @param albDetails + */ + async authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails) { + try { + // Assign ALB security group to AppStream security group engress (allow ALB egress from appstream) + const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); + const params = { + GroupId: appStreamSecurityGroupId, + IpPermissions: [ + { + IpProtocol: '-1', + UserIdGroupPairs: [ + { + GroupId: albDetails.albSecurityGroup, + }, + ], + }, + ], + }; + const [albService] = await this.mustFindServices(['albService']); + const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); + await ec2Client.authorizeSecurityGroupEgress(params).promise(); + } catch (e) { + throw new Error(`Assigning ALB security group to AppStream security group failed with error - ${e.message}`); + } + } + /** * Method to get CFT template output. The method gets CFT URL, read the CFT from S3 * and parses the tmplate using js-yaml library @@ -336,6 +407,19 @@ class CheckLaunchDependency extends StepBase { return cfnClient; } + /** + * Method to get appstream security group ID for the target aws account + * + * @param requestContext + * @param resolvedVars + * @returns {Promise} + */ + async getAppStreamSecurityGroupId(requestContext, resolvedVars) { + const [albService] = await this.mustFindServices(['albService']); + const { appStreamSecurityGroupId } = await albService.findAwsAccountDetails(requestContext, resolvedVars.projectId); + return appStreamSecurityGroupId; + } + /** * Method to get role arn for the target aws account * diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index 0991bc7aaa..aeae31fa04 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -36,6 +36,7 @@ const inPayloadKeys = { const settingKeys = { envMgmtRoleArn: 'envMgmtRoleArn', + isAppStreamEnabled: 'isAppStreamEnabled', }; const pluginConstants = { @@ -100,11 +101,13 @@ class TerminateLaunchDependency extends StepBase { const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); if (isAppStreamEnabled) { const memberAccount = await environmentScService.getMemberAccount(requestContext, environment); - await environmentDnsService.deletePrivateRecord( + await environmentDnsService.deletePrivateRecordForDNS( requestContext, 'rstudio', envId, dnsName, + deploymentValue.albHostedZoneId, + dnsName, memberAccount.route53HostedZone, ); } else { @@ -212,7 +215,7 @@ class TerminateLaunchDependency extends StepBase { const [albLock] = await Promise.all([this.state.optionalString('ALB_LOCK')]); if (albLock) { // eslint-disable-next-line no-return-await - return await this.terminateStack(requestContext, projectId, externalId, albRecord.albStackName); + return await this.terminateStack(requestContext, projectId, externalId, albRecord); } throw new Error(`Error terminating environment. Reason: ALB lock does not exist or expired`); } @@ -225,10 +228,22 @@ class TerminateLaunchDependency extends StepBase { * @param requestContext * @param projectId * @param externalId - * @param stackName + * @param albDetails * @returns {Promise<>} */ - async terminateStack(requestContext, projectId, externalId, stackName) { + async terminateStack(requestContext, projectId, externalId, albDetails) { + // Before we perform ALB stack deletion, we need to remove SG association with AppStream if it exists + if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { + // creating resolvedvars object with the necessary Metadata + const resolvedVars = { + projectId, + externalId, + }; + await this.revokeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); + await this.revokeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); + } + + const stackName = albDetails.albStackName; const cfn = await this.getCloudFormationService(requestContext, projectId, externalId); const params = { StackName: stackName, @@ -248,6 +263,68 @@ class TerminateLaunchDependency extends StepBase { ); } + /** + * Method to revoke ingress access from AppStream to ALB + * + * @param requestContext + * @param resolvedVars + * @param albDetails + */ + async revokeAlbAppStreamIngress(requestContext, resolvedVars, albDetails) { + try { + const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); + const params = { + GroupId: albDetails.albSecurityGroup, + IpPermissions: [ + { + IpProtocol: '-1', + UserIdGroupPairs: [ + { + GroupId: appStreamSecurityGroupId, + }, + ], + }, + ], + }; + const [albService] = await this.mustFindServices(['albService']); + const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); + await ec2Client.revokeSecurityGroupEgress(params).promise(); + } catch (e) { + throw new Error(`Revoking AppStream security group from ALB security group failed with error - ${e.message}`); + } + } + + /** + * Method to revoke egress access from AppStream to ALB + * + * @param requestContext + * @param resolvedVars + * @param albDetails + */ + async revokeAppStreamAlbEgress(requestContext, resolvedVars, albDetails) { + try { + const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); + const params = { + GroupId: appStreamSecurityGroupId, + IpPermissions: [ + { + IpProtocol: '-1', + UserIdGroupPairs: [ + { + GroupId: albDetails.albSecurityGroup, + }, + ], + }, + ], + }; + const [albService] = await this.mustFindServices(['albService']); + const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); + await ec2Client.revokeSecurityGroupEgress(params).promise(); + } catch (e) { + throw new Error(`Revoking ALB security group from AppStream security group failed with error - ${e.message}`); + } + } + /** * Method to get the cloud formation service client for the target aws account * @@ -273,6 +350,19 @@ class TerminateLaunchDependency extends StepBase { return cfnClient; } + /** + * Method to get appstream security group ID for the target aws account + * + * @param requestContext + * @param resolvedVars + * @returns {Promise} + */ + async getAppStreamSecurityGroupId(requestContext, resolvedVars) { + const [albService] = await this.mustFindServices(['albService']); + const { appStreamSecurityGroupId } = await albService.findAwsAccountDetails(requestContext, resolvedVars.projectId); + return appStreamSecurityGroupId; + } + /** * Method to get role arn for the target aws account * From 0ff9d57eb61a1495649714f102702ef7feee182c Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Thu, 4 Nov 2021 13:48:12 -0400 Subject: [PATCH 05/13] fix: ingress rule for ALB AppStream --- .../application-load-balancer.cfn.yml | 3 -- .../lib/alb/__tests__/alb-service.test.js | 18 -------- .../base-raas-services/lib/alb/alb-service.js | 16 ++++++- .../environment/environment-dns-service.js | 2 +- .../lib/plugins/env-provisioning-plugin.js | 7 +++- .../check-launch-dependency.js | 42 +++++++++++-------- .../terminate-launch-dependency.js | 10 +++-- 7 files changed, 53 insertions(+), 45 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index e8820555ad..a24cbe9bc2 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -74,9 +74,6 @@ Outputs: ALBDNSName: Description: DNS Name of Application Load Balancer Value: !GetAtt ApplicationLoadBalancer.DNSName - ALBHostedZoneId: - Description: Hosted Zone ID of Application Load Balancer - Value: !GetAtt ApplicationLoadBalancer.HostedZoneId ListenerArn: Description: ARN of Application Load Balancer Listener Value: !Ref ALBListener diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index b8765c34af..f209016691 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -525,24 +525,6 @@ describe('ALBService', () => { ruleARN: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:listener-rule/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2/9683b2d02a6cabee', }; - const params = { - Conditions: [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: ['rtsudio-test.example.com'], - }, - }, - { - Field: 'source-ip', - SourceIpConfig: { - Values: ['10.0.0.0/32'], - }, - }, - ], - RuleArn: - 'arn:aws:elasticloadbalancing:us-west-2:123456789012:listener-rule/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2/9683b2d02a6cabee', - }; service.getHostname = jest.fn(() => { return 'rtsudio-test.example.com'; }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index 0758e1922d..96fe984e1c 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -191,7 +191,7 @@ class ALBService extends Service { return deploymentItem; } - async checkIfAppStreamEnabled() { + checkIfAppStreamEnabled() { return this.settings.get(settingKeys.isAppStreamEnabled); } @@ -438,6 +438,20 @@ class ALBService extends Service { return albClient; } + /** + * Method to get the ALB HostedZone ID for the target aws account + * + * @param requestContext + * @param resolvedVars + * @returns {Promise<>} + */ + async getAlbHostedZoneID(requestContext, resolvedVars, albArn) { + const albClient = await this.getAlbSdk(requestContext, resolvedVars); + const params = { LoadBalancerArns: [albArn] }; + const response = await albClient.describeLoadBalancers(params).promise(); + return response.LoadBalancers[0].CanonicalHostedZoneId; + } + /** * Method to get role arn for the target aws account * diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js index 2c09099e6d..e8849d96ef 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-dns-service.js @@ -107,7 +107,7 @@ class EnvironmentDnsService extends Service { Type: 'A', AliasTarget: { HostedZoneId: albHostedZoneId, - DNSName: recordValue, + DNSName: `dualstack.${recordValue}`, EvaluateTargetHealth: false, }, }, diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js index 23a9790bc0..38764783e5 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/env-provisioning-plugin.js @@ -244,11 +244,16 @@ async function updateEnvOnProvisioningSuccess({ const settings = await container.find('settings'); if (settings.getBoolean(settingKeys.isAppStreamEnabled)) { const hostedZoneId = await getHostedZone(requestContext, environmentScService, existingEnvRecord); + const albHostedZoneId = await albService.getAlbHostedZoneID( + requestContext, + resolvedVars, + deploymentValue.albArn, + ); await environmentDnsService.createPrivateRecordForDNS( requestContext, 'rstudio', envId, - deploymentValue.albHostedZoneId, + albHostedZoneId, dnsName, hostedZoneId, ); diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js index 82ad04b464..eef2b3aae8 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js @@ -133,6 +133,12 @@ class CheckLaunchDependency extends StepBase { * @returns {Promise<>} */ async provisionAlb(requestContext, resolvedVars, projectId, resolvedInputParams, maxAlbWorkspacesCount) { + // Added additional check if lock exists before staring deployment + const [albLock] = await Promise.all([this.state.optionalString('ALB_LOCK')]); + if (!albLock) { + throw new Error(`Error provisioning environment. Reason: ALB lock does not exist or expired`); + } + const [albService] = await this.mustFindServices(['albService']); const count = await albService.albDependentWorkspacesCount(requestContext, projectId); const albExists = await albService.checkAlbExists(requestContext, projectId); @@ -154,14 +160,10 @@ class CheckLaunchDependency extends StepBase { projectId, ); // Create Stack - // Added additional check if lock exists before staring deployment - const [albLock] = await Promise.all([this.state.optionalString('ALB_LOCK')]); - if (albLock) { - // eslint-disable-next-line no-return-await - return await this.deployStack(requestContext, resolvedVars, stackInput); - } - throw new Error(`Error provisioning environment. Reason: ALB lock does not exist or expired`); + // eslint-disable-next-line no-return-await + return await this.deployStack(requestContext, resolvedVars, stackInput); } + return null; } @@ -219,16 +221,16 @@ class CheckLaunchDependency extends StepBase { albHostedZoneId: _.get(stackOutputs, 'ALBHostedZoneId', null), albDependentWorkspacesCount: 0, }; - if (albLock) { - await albService.saveAlbDetails(awsAccountId, albDetails); - if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { - // Allow ALB and AppStream security groups to interact with each other - await this.authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); - await this.authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); - } - } else { + if (!albLock) { throw new Error(`Error provisioning environment. Reason: ALB lock does not exist or expired`); } + await albService.saveAlbDetails(awsAccountId, albDetails); + if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { + // Allow ALB and AppStream security groups to interact with each other + await this.authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); + await this.authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); + } + this.print({ msg: `Dependency Details Updated Successfully`, }); @@ -260,9 +262,11 @@ class CheckLaunchDependency extends StepBase { }; const [albService] = await this.mustFindServices(['albService']); const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.authorizeSecurityGroupEgress(params).promise(); + await ec2Client.authorizeSecurityGroupIngress(params).promise(); } catch (e) { - throw new Error(`Assigning AppStream security group to ALB security group failed with error - ${e.message}`); + throw new Error( + `Assigning AppStream security group to ALB security group ingress failed with error - ${e.message}`, + ); } } @@ -294,7 +298,9 @@ class CheckLaunchDependency extends StepBase { const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); await ec2Client.authorizeSecurityGroupEgress(params).promise(); } catch (e) { - throw new Error(`Assigning ALB security group to AppStream security group failed with error - ${e.message}`); + throw new Error( + `Assigning ALB security group to AppStream security group egress failed with error - ${e.message}`, + ); } } diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index aeae31fa04..e3bbac0f44 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -288,9 +288,11 @@ class TerminateLaunchDependency extends StepBase { }; const [albService] = await this.mustFindServices(['albService']); const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.revokeSecurityGroupEgress(params).promise(); + await ec2Client.revokeSecurityGroupIngress(params).promise(); } catch (e) { - throw new Error(`Revoking AppStream security group from ALB security group failed with error - ${e.message}`); + throw new Error( + `Revoking AppStream security group from ALB security group ingress failed with error - ${e.message}`, + ); } } @@ -321,7 +323,9 @@ class TerminateLaunchDependency extends StepBase { const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); await ec2Client.revokeSecurityGroupEgress(params).promise(); } catch (e) { - throw new Error(`Revoking ALB security group from AppStream security group failed with error - ${e.message}`); + throw new Error( + `Revoking ALB security group from AppStream security group egress failed with error - ${e.message}`, + ); } } From 0ea554b1a5c2ab59df637415cc306fae709b0c46 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Thu, 4 Nov 2021 14:46:24 -0400 Subject: [PATCH 06/13] fix: delete private DNS record --- .../terminate-launch-dependency.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index e3bbac0f44..0b7f572983 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -85,6 +85,13 @@ class TerminateLaunchDependency extends StepBase { // Setting project id to use while polling for status this.state.setKey('PROJECT_ID', projectId); + + // creating resolvedvars object with the necessary Metadata + const resolvedVars = { + projectId, + externalId, + }; + // convert output array to object. Return {} if no outputs found const environmentOutputs = await this.cfnOutputsArrayToObject(_.get(environment, 'outputs', [])); const connectionType = _.get(environmentOutputs, 'MetaConnection1Type', ''); @@ -101,12 +108,16 @@ class TerminateLaunchDependency extends StepBase { const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); if (isAppStreamEnabled) { const memberAccount = await environmentScService.getMemberAccount(requestContext, environment); + const albHostedZoneId = await albService.getAlbHostedZoneID( + requestContext, + resolvedVars, + deploymentValue.albArn, + ); await environmentDnsService.deletePrivateRecordForDNS( requestContext, 'rstudio', envId, - dnsName, - deploymentValue.albHostedZoneId, + albHostedZoneId, dnsName, memberAccount.route53HostedZone, ); @@ -150,11 +161,6 @@ class TerminateLaunchDependency extends StepBase { // Termination should not be affected in such scenarios if (!_.isEmpty(ruleArn)) { try { - // creating resolvedvars object with the necessary Metadata - const resolvedVars = { - projectId, - externalId, - }; await lockService.tryWriteLockAndRun({ id: `alb-rule-${deploymentItem.id}` }, async () => { const listenerArn = deploymentValue.listenerArn; await albService.deleteListenerRule(requestContext, resolvedVars, ruleArn, listenerArn); From 21635a6591ed6f31a67ba5e2ebe72a864a9078f0 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Fri, 5 Nov 2021 10:47:14 -0400 Subject: [PATCH 07/13] add ingress in ALB CFN template --- .../application-load-balancer.cfn.yml | 6 +++- .../base-raas-services/lib/alb/alb-service.js | 1 + .../check-launch-dependency.js | 35 ------------------- .../terminate-launch-dependency.js | 34 ------------------ 4 files changed, 6 insertions(+), 70 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index a24cbe9bc2..bde09ff71f 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -22,6 +22,9 @@ Parameters: Type: String AllowedValues: [true, false] Description: Is AppStream enabled for this workspace + AppStreamSG: + Type: String + Description: AppStream Security Group ID Conditions: AppStreamEnabled: !Equals [!Ref IsAppStreamEnabled, 'true'] @@ -60,7 +63,8 @@ Resources: SecurityGroupIngress: - !If - AppStreamEnabled - - !Ref 'AWS::NoValue' + - SourceSecurityGroupId: !Ref AppStreamSG + IpProtocol: '-1' - CidrIp: '0.0.0.0/0' FromPort: 443 ToPort: 443 diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index 96fe984e1c..ef1ed93302 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -104,6 +104,7 @@ class ALBService extends Service { addParam('ACMSSLCertARN', certificateArn.Value); addParam('VPC', awsAccountDetails.vpcId); addParam('IsAppStreamEnabled', isAppStreamEnabled.Value); + addParam('AppStreamSG', awsAccountDetails.appStreamSecurityGroupId); const input = { StackName: resolvedVars.namespace, diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js index eef2b3aae8..08173d49b4 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js @@ -228,7 +228,6 @@ class CheckLaunchDependency extends StepBase { if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { // Allow ALB and AppStream security groups to interact with each other await this.authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); - await this.authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); } this.print({ @@ -236,40 +235,6 @@ class CheckLaunchDependency extends StepBase { }); } - /** - * Method to allow ingress access from AppStream to ALB - * - * @param requestContext - * @param resolvedVars - * @param albDetails - */ - async authorizeAlbAppStreamIngress(requestContext, resolvedVars, albDetails) { - try { - // Assign AppStream security group to ALB security group ingress - const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); - const params = { - GroupId: albDetails.albSecurityGroup, - IpPermissions: [ - { - IpProtocol: '-1', - UserIdGroupPairs: [ - { - GroupId: appStreamSecurityGroupId, - }, - ], - }, - ], - }; - const [albService] = await this.mustFindServices(['albService']); - const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.authorizeSecurityGroupIngress(params).promise(); - } catch (e) { - throw new Error( - `Assigning AppStream security group to ALB security group ingress failed with error - ${e.message}`, - ); - } - } - /** * Method to allow egress access from AppStream to ALB * diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index 0b7f572983..0fa0c6c080 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -246,7 +246,6 @@ class TerminateLaunchDependency extends StepBase { externalId, }; await this.revokeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); - await this.revokeAlbAppStreamIngress(requestContext, resolvedVars, albDetails); } const stackName = albDetails.albStackName; @@ -269,39 +268,6 @@ class TerminateLaunchDependency extends StepBase { ); } - /** - * Method to revoke ingress access from AppStream to ALB - * - * @param requestContext - * @param resolvedVars - * @param albDetails - */ - async revokeAlbAppStreamIngress(requestContext, resolvedVars, albDetails) { - try { - const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); - const params = { - GroupId: albDetails.albSecurityGroup, - IpPermissions: [ - { - IpProtocol: '-1', - UserIdGroupPairs: [ - { - GroupId: appStreamSecurityGroupId, - }, - ], - }, - ], - }; - const [albService] = await this.mustFindServices(['albService']); - const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.revokeSecurityGroupIngress(params).promise(); - } catch (e) { - throw new Error( - `Revoking AppStream security group from ALB security group ingress failed with error - ${e.message}`, - ); - } - } - /** * Method to revoke egress access from AppStream to ALB * From e15c63cb1af2f436806c43e8b6bb273e771af95e Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Mon, 8 Nov 2021 08:18:08 -0500 Subject: [PATCH 08/13] fix: separating ALB subnets from workspaces' --- .../application-load-balancer.cfn.yml | 109 ++++++++++++++++-- .../src/templates/onboard-account.cfn.yml | 73 +----------- .../base-raas-services/lib/alb/alb-service.js | 65 +---------- 3 files changed, 111 insertions(+), 136 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index bde09ff71f..1a9f60daa8 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -9,12 +9,6 @@ Parameters: VPC: Description: The VPC in which the ALB will reside Type: AWS::EC2::VPC::Id - Subnet1: - Description: The VPC Subnet1 in which the ALB will reside - Type: AWS::EC2::Subnet::Id - Subnet2: - Description: The VPC Subnet2 in which the ALB will reside - Type: AWS::EC2::Subnet::Id ACMSSLCertARN: Type: String Description: The ARN of the AWS Certificate Manager SSL Certificate to associate with the Load Balancer @@ -25,12 +19,109 @@ Parameters: AppStreamSG: Type: String Description: AppStream Security Group ID + Default: AppStream Security Group + + # For an ALB - You must specify subnets from at least two Availability Zones. + # AWS recommends Availability Zone subnet for your load balancer to have a CIDR block with at least a /27 bitmask + # Assigning bitmask of /25 to be safe and allow 128 hosts per subnet + # Keeping non-overlapping between public/private CIDR ranges to account for failed non-AppStream env termination + + # Range from 10.0.96.0 to 10.0.96.127 + PublicSubnet1Cidr: + Type: String + Default: 10.0.96.0/25 + + # Range from 10.0.96.128 to 10.0.96.255 + PublicSubnet2Cidr: + Type: String + Default: 10.0.96.128/25 + + # Range from 10.0.97.0 to 10.0.97.127 + PrivateSubnet1Cidr: + Type: String + Default: 10.0.97.0/25 + + # Range from 10.0.97.128 to 10.0.97.255 + PrivateSubnet2Cidr: + Type: String + Default: 10.0.97.128/25 Conditions: AppStreamEnabled: !Equals [!Ref IsAppStreamEnabled, 'true'] AppStreamDisabled: !Equals [!Ref IsAppStreamEnabled, 'false'] Resources: + PublicSubnet1: + Type: AWS::EC2::Subnet + Condition: AppStreamDisabled + Properties: + VpcId: !Ref VPC + AvailabilityZone: !Select [0, !GetAZs ] + CidrBlock: !Ref PublicSubnet1Cidr + MapPublicIpOnLaunch: true + Tags: + - Key: Name + Value: !Sub ${Namespace} ALB public subnet 1 + + PublicSubnet2: + Type: AWS::EC2::Subnet + Condition: AppStreamDisabled + Properties: + VpcId: !Ref VPC + AvailabilityZone: !Select [1, !GetAZs ] + CidrBlock: !Ref PublicSubnet2Cidr + MapPublicIpOnLaunch: true + Tags: + - Key: Name + Value: !Sub ${Namespace} ALB public subnet 2 + + # PublicRouteTable: + # Type: AWS::EC2::RouteTable + # Condition: AppStreamDisabled + # Properties: + # VpcId: !Ref VPC + # Tags: + # - Key: Name + # Value: !Sub ${Namespace} public routes + + # PublicSubnet1RouteTableAssociation: + # Type: AWS::EC2::SubnetRouteTableAssociation + # Condition: AppStreamDisabled + # Properties: + # RouteTableId: !Ref PublicRouteTable + # SubnetId: !Ref PublicSubnet1 + + # PublicSubnet2RouteTableAssociation: + # Type: AWS::EC2::SubnetRouteTableAssociation + # Condition: AppStreamDisabled + # Properties: + # RouteTableId: !Ref PublicRouteTable + # SubnetId: !Ref PublicSubnet2 + + PrivateSubnet1: + Type: AWS::EC2::Subnet + Condition: AppStreamEnabled + Properties: + VpcId: !Ref VPC + AvailabilityZone: !Select [0, !GetAZs ''] + CidrBlock: !Ref PrivateSubnet1Cidr + MapPublicIpOnLaunch: false + Tags: + - Key: Name + Value: !Sub ${Namespace} ALB private subnet 1 + + PrivateSubnet2: + Type: AWS::EC2::Subnet + Condition: AppStreamEnabled + Properties: + VpcId: !Ref VPC + AvailabilityZone: !Select [1, !GetAZs ''] + CidrBlock: !Ref PrivateSubnet2Cidr + MapPublicIpOnLaunch: false + Tags: + - Key: Name + Value: !Sub ${Namespace} ALB private subnet 2 + ALBListener: Type: AWS::ElasticLoadBalancingV2::Listener Properties: @@ -47,16 +138,17 @@ Resources: SslPolicy: ELBSecurityPolicy-2016-08 Certificates: - CertificateArn: !Ref ACMSSLCertARN + ApplicationLoadBalancer: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Name: !Ref Namespace Scheme: !If [AppStreamEnabled, 'internal', 'internet-facing'] Subnets: - - Ref: Subnet1 - - Ref: Subnet2 + !If [AppStreamEnabled, [!Ref PrivateSubnet1, !Ref PrivateSubnet2], [!Ref PublicSubnet1, !Ref PublicSubnet2]] SecurityGroups: - Ref: ALBSecurityGroup + ALBSecurityGroup: Type: AWS::EC2::SecurityGroup Properties: @@ -71,6 +163,7 @@ Resources: IpProtocol: tcp GroupDescription: ALB SecurityGroup VpcId: !Ref VPC + Outputs: LoadBalancerArn: Description: ARN of Application Load Balancer diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml index c02c60bd80..7a09a499dc 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml @@ -40,11 +40,6 @@ Parameters: Type: String Default: 10.0.0.0/19 - PublicSubnet2Cidr: - Description: Please enter the IP range (CIDR notation) for the public subnet 2 in the 2nd Availability Zone - Type: String - Default: 10.0.32.0/19 - LaunchConstraintRolePrefix: Description: Role name prefix to use when creating a launch constraint role in the on-boarded account Type: String @@ -68,12 +63,6 @@ Parameters: Type: String Default: 10.0.64.0/19 - # Range from 10.0.96.0 to 10.0.127.255 - WorkspaceSubnet2Cidr: - Description: Please enter the IP range (CIDR notation) for the Workspace subnet 2. This value is only used if AppStream is enabled. - Type: String - Default: 10.0.96.0/19 - AppStreamFleetDesiredInstances: Description: The desired number of streaming instances. Type: Number @@ -113,6 +102,8 @@ Parameters: Type: String Default: '' + # Note: While adding additional CIDRs/Subnets ensure they don't overlap with those used by RStudio ALB (10.0.96.0 to 10.0.97.255) + Metadata: AWS::CloudFormation::Interface: ParameterGroups: @@ -130,8 +121,7 @@ Metadata: Parameters: - VpcCidr - PublicSubnetCidr - - PublicSubnet2Cidr - + Conditions: isAppStream: !Equals - !Ref EnableAppStream @@ -634,18 +624,6 @@ Resources: - Key: Name Value: !Sub ${Namespace} public subnet 1 - PublicSubnet2: - Type: AWS::EC2::Subnet - Condition: isNotAppStream - Properties: - VpcId: !Ref VPC - AvailabilityZone: !Select [1, !GetAZs ] - CidrBlock: !Ref PublicSubnet2Cidr - MapPublicIpOnLaunch: true - Tags: - - Key: Name - Value: !Sub ${Namespace} public subnet 2 - PublicRouteTable: Type: AWS::EC2::RouteTable Condition: isNotAppStream @@ -671,13 +649,6 @@ Resources: RouteTableId: !Ref PublicRouteTable SubnetId: !Ref PublicSubnet1 - PublicSubnet2RouteTableAssociation: - Type: AWS::EC2::SubnetRouteTableAssociation - Condition: isNotAppStream - Properties: - RouteTableId: !Ref PublicRouteTable - SubnetId: !Ref PublicSubnet2 - EncryptionKey: Type: AWS::KMS::Key Properties: @@ -742,18 +713,7 @@ Resources: - Key: Name Value: Private Workspace Subnet - # For an ALB - You must specify subnets from at least two Availability Zones. - PrivateWorkspaceSubnet2: - Type: AWS::EC2::Subnet - Condition: isAppStreamAndCustomDomain - Properties: - VpcId: !Ref VPC - AvailabilityZone: !Select [1, !GetAZs ''] - CidrBlock: !Ref WorkspaceSubnet2Cidr - MapPublicIpOnLaunch: false - Tags: - - Key: Name - Value: Private Workspace Subnet 2 + PrivateWorkspaceRouteTable: Type: 'AWS::EC2::RouteTable' @@ -810,10 +770,6 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet - - !If - - isAppStreamAndCustomDomain - - !Ref PrivateWorkspaceSubnet2 - - !Ref 'AWS::NoValue' PrivateDnsEnabled: true VpcEndpointType: Interface ServiceName: !Sub 'com.amazonaws.${AWS::Region}.kms' @@ -827,10 +783,6 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet - - !If - - isAppStreamAndCustomDomain - - !Ref PrivateWorkspaceSubnet2 - - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.sts' @@ -844,10 +796,6 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet - - !If - - isAppStreamAndCustomDomain - - !Ref PrivateWorkspaceSubnet2 - - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.ec2' @@ -861,10 +809,6 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet - - !If - - isAppStreamAndCustomDomain - - !Ref PrivateWorkspaceSubnet2 - - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.cloudformation' @@ -921,10 +865,6 @@ Resources: Properties: SubnetIds: - !Ref PrivateWorkspaceSubnet - - !If - - isAppStreamAndCustomDomain - - !Ref PrivateWorkspaceSubnet2 - - !Ref 'AWS::NoValue' VpcEndpointType: Interface PrivateDnsEnabled: true ServiceName: !Sub 'com.amazonaws.${AWS::Region}.ssm' @@ -1100,11 +1040,6 @@ Outputs: Condition: isAppStream Value: !Ref PrivateWorkspaceSubnet - PrivateWorkspaceSubnet2: - Description: Second subnet for ALB - Condition: isAppStreamAndCustomDomain - Value: !Ref PrivateWorkspaceSubnet2 - AppStreamSecurityGroup: Description: AppStream Security Group Condition: isAppStream diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index ef1ed93302..b752d408a1 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -90,21 +90,18 @@ class ALBService extends Service { const cfnParams = []; const certificateArn = _.find(resolvedInputParams, o => o.Key === 'ACMSSLCertARN'); const isAppStreamEnabled = _.find(resolvedInputParams, o => o.Key === 'IsAppStreamEnabled'); - const subnet2 = await this.findSubnet2( - requestContext, - resolvedVars, - awsAccountDetails.vpcId, - isAppStreamEnabled.Value, - ); const addParam = (key, v) => cfnParams.push({ ParameterKey: key, ParameterValue: v }); addParam('Namespace', resolvedVars.namespace); - addParam('Subnet1', awsAccountDetails.subnetId); - addParam('Subnet2', subnet2); addParam('ACMSSLCertARN', certificateArn.Value); addParam('VPC', awsAccountDetails.vpcId); addParam('IsAppStreamEnabled', isAppStreamEnabled.Value); - addParam('AppStreamSG', awsAccountDetails.appStreamSecurityGroupId); + addParam( + 'AppStreamSG', + _.isUndefined(awsAccountDetails.appStreamSecurityGroupId) + ? 'AppStreamNotConfigured' + : awsAccountDetails.appStreamSecurityGroupId, + ); const input = { StackName: resolvedVars.namespace, @@ -349,56 +346,6 @@ class ALBService extends Service { return `${prefix}-${id}.${domainName}`; } - /** - * Method to get the second subnet ID - * For an ALB - You must specify subnets from at least two Availability Zones. - * - * @param requestContext - * @param resolvedVars - * @param vpcId - * @param appStreamEnabled - * @returns {Promise} - */ - async findSubnet2(requestContext, resolvedVars, vpcId, appStreamEnabled) { - const isAppStreamEnabled = appStreamEnabled === 'true'; - let params; - if (isAppStreamEnabled) { - params = { - Filters: [ - { - Name: 'vpc-id', - Values: [vpcId], - }, - { - Name: 'tag:aws:cloudformation:logical-id', - Values: ['PrivateWorkspaceSubnet2'], - }, - ], - }; - } else { - params = { - Filters: [ - { - Name: 'vpc-id', - Values: [vpcId], - }, - { - Name: 'tag:aws:cloudformation:logical-id', - Values: ['PublicSubnet2'], - }, - ], - }; - } - - const ec2Client = await this.getEc2Sdk(requestContext, resolvedVars); - const response = await ec2Client.describeSubnets(params).promise(); - const subnetId = _.get(response.Subnets[0], 'SubnetId', null); - if (!subnetId) { - throw new Error(`Error provisioning environment. Reason: Subnet2 not found for the VPC - ${vpcId}`); - } - return subnetId; - } - /** * Method to get the EC2 SDK client for the target aws account * From aeabae5e5d9d8f5ba93696905ba42b387952d172 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Mon, 8 Nov 2021 08:41:23 -0500 Subject: [PATCH 09/13] comment cleanup --- .../application-load-balancer.cfn.yml | 23 ------------------- .../check-launch-dependency.js | 2 +- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index 1a9f60daa8..e2e6f743c6 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -75,29 +75,6 @@ Resources: - Key: Name Value: !Sub ${Namespace} ALB public subnet 2 - # PublicRouteTable: - # Type: AWS::EC2::RouteTable - # Condition: AppStreamDisabled - # Properties: - # VpcId: !Ref VPC - # Tags: - # - Key: Name - # Value: !Sub ${Namespace} public routes - - # PublicSubnet1RouteTableAssociation: - # Type: AWS::EC2::SubnetRouteTableAssociation - # Condition: AppStreamDisabled - # Properties: - # RouteTableId: !Ref PublicRouteTable - # SubnetId: !Ref PublicSubnet1 - - # PublicSubnet2RouteTableAssociation: - # Type: AWS::EC2::SubnetRouteTableAssociation - # Condition: AppStreamDisabled - # Properties: - # RouteTableId: !Ref PublicRouteTable - # SubnetId: !Ref PublicSubnet2 - PrivateSubnet1: Type: AWS::EC2::Subnet Condition: AppStreamEnabled diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js index 08173d49b4..b2c74e70ac 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js @@ -244,7 +244,7 @@ class CheckLaunchDependency extends StepBase { */ async authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails) { try { - // Assign ALB security group to AppStream security group engress (allow ALB egress from appstream) + // Assign ALB security group to AppStream security group egress (allow ALB egress from appstream) const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); const params = { GroupId: appStreamSecurityGroupId, From 8d864e7222a23ec96187d522aeeea17db4096cfa Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Tue, 9 Nov 2021 10:55:34 -0500 Subject: [PATCH 10/13] changes per review --- .../application-load-balancer.cfn.yml | 27 +++- .../src/templates/onboard-account.cfn.yml | 7 + .../lib/alb/__tests__/alb-service.test.js | 64 ++------- .../base-raas-services/lib/alb/alb-service.js | 134 +++++++++++------- .../__tests__/aws-cfn-service.test.js | 6 + .../lib/aws-accounts/aws-cfn-service.js | 2 +- .../lib/schema/create-aws-accounts.json | 3 + .../lib/schema/update-aws-accounts.json | 3 + .../provision-account/provision-account.js | 1 + .../check-launch-dependency.js | 38 ----- .../terminate-launch-dependency.js | 45 +----- 11 files changed, 135 insertions(+), 195 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml index e2e6f743c6..31c3ccc5c9 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/application-load-balancer.cfn.yml @@ -19,7 +19,9 @@ Parameters: AppStreamSG: Type: String Description: AppStream Security Group ID - Default: AppStream Security Group + PublicRouteTableId: + Type: String + Description: Public Route Table ID # For an ALB - You must specify subnets from at least two Availability Zones. # AWS recommends Availability Zone subnet for your load balancer to have a CIDR block with at least a /27 bitmask @@ -75,6 +77,29 @@ Resources: - Key: Name Value: !Sub ${Namespace} ALB public subnet 2 + PublicSubnet1RouteTableAssociation: + Type: AWS::EC2::SubnetRouteTableAssociation + Condition: AppStreamDisabled + Properties: + RouteTableId: !Ref PublicRouteTableId + SubnetId: !Ref PublicSubnet1 + + PublicSubnet2RouteTableAssociation: + Type: AWS::EC2::SubnetRouteTableAssociation + Condition: AppStreamDisabled + Properties: + RouteTableId: !Ref PublicRouteTableId + SubnetId: !Ref PublicSubnet2 + + AppStreamSecurityGroupEgress: + Type: AWS::EC2::SecurityGroupEgress + Condition: AppStreamEnabled + Properties: + GroupId: !Ref AppStreamSG + DestinationSecurityGroupId: !Ref ALBSecurityGroup + Description: 'Allow AppStream egress to ALB' + IpProtocol: '-1' + PrivateSubnet1: Type: AWS::EC2::Subnet Condition: AppStreamEnabled diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml index 7a09a499dc..3bf34f0e51 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml @@ -413,6 +413,8 @@ Resources: Action: - ec2:DescribeInstanceStatus - ec2:DescribeInstances + - ec2:DescribeRouteTables + - ec2:AssociateRouteTable Resource: '*' # For the actions listed above IAM does not support resource-level permissions and requires all resources to be chosen - Effect: Allow Action: @@ -435,6 +437,7 @@ Resources: Action: - ec2:CreateSubnet - ec2:DeleteSubnet + - ec2:ModifySubnetAttribute - ec2:AssociateSubnetCidrBlock - ec2:DisassociateSubnetCidrBlock Resource: @@ -1029,6 +1032,10 @@ Outputs: Description: The arn of the role SWB uses to check permissions status Value: !GetAtt [CfnStatusRole, Arn] + PublicRouteTableId: + Description: The public route table assigned to the workspace VPC + Value: !Ref PublicRouteTable + #------------AppStream Output Below------- PrivateAppStreamSubnet: Description: AppStream subnet diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index f209016691..0ca2015d83 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -271,9 +271,6 @@ describe('ALBService', () => { cfnTemplateService.getTemplate.mockImplementationOnce(() => { return ['template']; }); - jest.spyOn(service, 'findSubnet2').mockImplementationOnce(() => { - return 'test-subnet-2'; - }); const apiResponse = { StackName: resolvedVars.namespace, Parameters: [ @@ -281,14 +278,6 @@ describe('ALBService', () => { ParameterKey: 'Namespace', ParameterValue: 'namespace', }, - { - ParameterKey: 'Subnet1', - ParameterValue: 'subnet-0a661d9f417ecff3f', - }, - { - ParameterKey: 'Subnet2', - ParameterValue: 'test-subnet-2', - }, { ParameterKey: 'ACMSSLCertARN', ParameterValue: 'Value', @@ -301,6 +290,14 @@ describe('ALBService', () => { ParameterKey: 'IsAppStreamEnabled', ParameterValue: 'false', }, + { + ParameterKey: 'AppStreamSG', + ParameterValue: 'AppStreamNotConfigured', + }, + { + ParameterKey: 'PublicRouteTableId', + ParameterValue: undefined, + }, ], TemplateBody: ['template'], Tags: [ @@ -318,9 +315,6 @@ describe('ALBService', () => { projectService.mustFind.mockImplementationOnce(() => { throw service.boom.notFound(`project with id "test-id" does not exist`, true); }); - jest.spyOn(service, 'findSubnet2').mockImplementationOnce(() => { - return 'test-subnet'; - }); try { await service.getStackCreationInput({}, resolvedVars, resolvedInputParams, ''); } catch (err) { @@ -633,46 +627,4 @@ describe('ALBService', () => { expect(response).toEqual(3); }); }); - - describe('findSubnet2', () => { - it('should fail when describe subnet API call throws error', async () => { - ec2Client.describeSubnets = jest.fn().mockImplementation(() => { - throw new Error(`Error describing subnet. VPC does not exist`); - }); - try { - await service.findSubnet2({}, {}, '', 'true'); - } catch (err) { - expect(err.message).toContain('Error describing subnet. VPC does not exist'); - } - }); - - it('should fail when subnet not found', async () => { - ec2Client.describeSubnets = jest.fn().mockImplementation(() => { - return { - promise: () => { - return { Subnets: [] }; - }, - }; - }); - try { - await service.findSubnet2({}, {}, 'test-vpc', 'false'); - } catch (err) { - expect(err.message).toContain( - 'Error provisioning environment. Reason: Subnet2 not found for the VPC - test-vpc', - ); - } - }); - - it('should return subnet id on success', async () => { - ec2Client.describeSubnets = jest.fn().mockImplementation(() => { - return { - promise: () => { - return { Subnets: [{ SubnetId: 'test-subnet-id' }] }; - }, - }; - }); - const response = await service.findSubnet2({}, {}, 'test-vpc', true); - expect(response).toEqual('test-subnet-id'); - }); - }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js index b752d408a1..0cdae8f2f9 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/alb-service.js @@ -102,6 +102,10 @@ class ALBService extends Service { ? 'AppStreamNotConfigured' : awsAccountDetails.appStreamSecurityGroupId, ); + addParam( + 'PublicRouteTableId', + _.isUndefined(awsAccountDetails.appStreamSecurityGroupId) ? awsAccountDetails.publicRouteTableId : 'N/A', + ); const input = { StackName: resolvedVars.namespace, @@ -190,7 +194,7 @@ class ALBService extends Service { } checkIfAppStreamEnabled() { - return this.settings.get(settingKeys.isAppStreamEnabled); + return this.settings.getBoolean(settingKeys.isAppStreamEnabled); } /** @@ -210,40 +214,54 @@ class ALBService extends Service { const listenerArn = albRecord.listenerArn; const priority = await this.calculateRulePriority(requestContext, resolvedVars, albRecord.listenerArn); const subdomain = this.getHostname(prefix, resolvedVars.envId); - const params = { - ListenerArn: listenerArn, - Priority: priority, - Actions: [ - { - TargetGroupArn: targetGroupArn, - Type: 'forward', - }, - ], - Conditions: isAppStreamEnabled - ? [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, + let params; + if (isAppStreamEnabled) { + params = { + ListenerArn: listenerArn, + Priority: priority, + Actions: [ + { + TargetGroupArn: targetGroupArn, + Type: 'forward', + }, + ], + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], }, - ] - : [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, + }, + ], + Tags: resolvedVars.tags, + }; + } else { + params = { + ListenerArn: listenerArn, + Priority: priority, + Actions: [ + { + TargetGroupArn: targetGroupArn, + Type: 'forward', + }, + ], + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], }, - { - Field: 'source-ip', - SourceIpConfig: { - Values: [resolvedVars.cidr], - }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: [resolvedVars.cidr], }, - ], - Tags: resolvedVars.tags, - }; + }, + ], + Tags: resolvedVars.tags, + }; + } const albClient = await this.getAlbSdk(requestContext, resolvedVars); let response = null; try { @@ -433,32 +451,38 @@ class ALBService extends Service { const subdomain = this.getHostname(resolvedVars.prefix, resolvedVars.envId); const isAppStreamEnabled = this.checkIfAppStreamEnabled(); try { - const params = { - Conditions: isAppStreamEnabled - ? [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, + let params; + if (isAppStreamEnabled) { + params = { + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], }, - ] - : [ - { - Field: 'host-header', - HostHeaderConfig: { - Values: [subdomain], - }, + }, + ], + RuleArn: resolvedVars.ruleARN, + }; + } else { + params = { + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], }, - { - Field: 'source-ip', - SourceIpConfig: { - Values: [resolvedVars.cidr], - }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: resolvedVars.cidr, }, - ], - RuleArn: resolvedVars.ruleARN, - }; + }, + ], + RuleArn: resolvedVars.ruleARN, + }; + } const { externalId } = await this.findAwsAccountDetails(requestContext, resolvedVars.projectId); resolvedVars.externalId = externalId; const albClient = await this.getAlbSdk(requestContext, resolvedVars); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/__tests__/aws-cfn-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/__tests__/aws-cfn-service.test.js index 44252065c6..203d27217f 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/__tests__/aws-cfn-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/__tests__/aws-cfn-service.test.js @@ -100,6 +100,11 @@ describe('AwsAccountService', () => { OutputValue: 'subnet-placeholder', Description: 'A reference to the public subnet in the 1st Availability Zone', }, + { + OutputKey: 'PublicRouteTableId', + OutputValue: 'rtd-samplePublicRouteTableId', + Description: 'The public route table assigned to the workspace VPC', + }, { OutputKey: 'CrossAccountEnvMgmtRoleArn', OutputValue: 'arn:aws:iam::placeholder', @@ -481,6 +486,7 @@ describe('AwsAccountService', () => { encryptionKeyArn: 'arn:aws:kms:placeholder', permissionStatus: 'CURRENT', rev: completedAccountMock.rev, + publicRouteTableId: 'rtd-samplePublicRouteTableId', }; awsAccountsService.list.mockImplementationOnce(() => [completedAccountMock]); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-cfn-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-cfn-service.js index 5c9796b279..6529e31f01 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-cfn-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-cfn-service.js @@ -476,8 +476,8 @@ class AwsCfnService extends Service { } } else { fieldsToUpdate.subnetId = findOutputValue('VpcPublicSubnet1'); + fieldsToUpdate.publicRouteTableId = findOutputValue('PublicRouteTableId'); } - await awsAccountsService.update(requestContext, fieldsToUpdate); // TODO Start AppStream fleet diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/schema/create-aws-accounts.json b/addons/addon-base-raas/packages/base-raas-services/lib/schema/create-aws-accounts.json index d434d80756..90cc53227c 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/schema/create-aws-accounts.json +++ b/addons/addon-base-raas/packages/base-raas-services/lib/schema/create-aws-accounts.json @@ -46,6 +46,9 @@ "type": "string", "pattern": "^subnet-[a-f0-9]{8,17}$" }, + "publicRouteTableId": { + "type": "string" + }, "encryptionKeyArn": { "type": "string", "pattern": "^arn:aws:kms:.*$" diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/schema/update-aws-accounts.json b/addons/addon-base-raas/packages/base-raas-services/lib/schema/update-aws-accounts.json index 3877001494..13bff13f3f 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/schema/update-aws-accounts.json +++ b/addons/addon-base-raas/packages/base-raas-services/lib/schema/update-aws-accounts.json @@ -67,6 +67,9 @@ "permissionStatus": { "type": "string" }, + "publicRouteTableId": { + "type": "string" + }, "appStreamStackName": { "type": "string" }, diff --git a/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js b/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js index db326c3693..a3e2dd20ef 100644 --- a/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js +++ b/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js @@ -289,6 +289,7 @@ class ProvisionAccount extends StepBase { vpcId: cfnOutputs.VPC, encryptionKeyArn: cfnOutputs.EncryptionKeyArn, onboardStatusRoleArn: cfnOutputs.OnboardStatusRoleArn, + publicRouteTableId: cfnOutputs.PublicRouteTableId, cfnStackName: stackInfo.StackName, cfnStackId: stackInfo.StackId, permissionStatus: 'CURRENT', diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js index b2c74e70ac..9fdefa7d7d 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/check-launch-dependency/check-launch-dependency.js @@ -225,50 +225,12 @@ class CheckLaunchDependency extends StepBase { throw new Error(`Error provisioning environment. Reason: ALB lock does not exist or expired`); } await albService.saveAlbDetails(awsAccountId, albDetails); - if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { - // Allow ALB and AppStream security groups to interact with each other - await this.authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); - } this.print({ msg: `Dependency Details Updated Successfully`, }); } - /** - * Method to allow egress access from AppStream to ALB - * - * @param requestContext - * @param resolvedVars - * @param albDetails - */ - async authorizeAppStreamAlbEgress(requestContext, resolvedVars, albDetails) { - try { - // Assign ALB security group to AppStream security group egress (allow ALB egress from appstream) - const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); - const params = { - GroupId: appStreamSecurityGroupId, - IpPermissions: [ - { - IpProtocol: '-1', - UserIdGroupPairs: [ - { - GroupId: albDetails.albSecurityGroup, - }, - ], - }, - ], - }; - const [albService] = await this.mustFindServices(['albService']); - const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.authorizeSecurityGroupEgress(params).promise(); - } catch (e) { - throw new Error( - `Assigning ALB security group to AppStream security group egress failed with error - ${e.message}`, - ); - } - } - /** * Method to get CFT template output. The method gets CFT URL, read the CFT from S3 * and parses the tmplate using js-yaml library diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index 0fa0c6c080..bf3868217c 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -105,7 +105,7 @@ class TerminateLaunchDependency extends StepBase { if (albExists) { try { - const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled); + const isAppStreamEnabled = this.settings.getBoolean(settingKeys.isAppStreamEnabled); if (isAppStreamEnabled) { const memberAccount = await environmentScService.getMemberAccount(requestContext, environment); const albHostedZoneId = await albService.getAlbHostedZoneID( @@ -238,16 +238,6 @@ class TerminateLaunchDependency extends StepBase { * @returns {Promise<>} */ async terminateStack(requestContext, projectId, externalId, albDetails) { - // Before we perform ALB stack deletion, we need to remove SG association with AppStream if it exists - if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) { - // creating resolvedvars object with the necessary Metadata - const resolvedVars = { - projectId, - externalId, - }; - await this.revokeAppStreamAlbEgress(requestContext, resolvedVars, albDetails); - } - const stackName = albDetails.albStackName; const cfn = await this.getCloudFormationService(requestContext, projectId, externalId); const params = { @@ -268,39 +258,6 @@ class TerminateLaunchDependency extends StepBase { ); } - /** - * Method to revoke egress access from AppStream to ALB - * - * @param requestContext - * @param resolvedVars - * @param albDetails - */ - async revokeAppStreamAlbEgress(requestContext, resolvedVars, albDetails) { - try { - const appStreamSecurityGroupId = await this.getAppStreamSecurityGroupId(requestContext, resolvedVars); - const params = { - GroupId: appStreamSecurityGroupId, - IpPermissions: [ - { - IpProtocol: '-1', - UserIdGroupPairs: [ - { - GroupId: albDetails.albSecurityGroup, - }, - ], - }, - ], - }; - const [albService] = await this.mustFindServices(['albService']); - const ec2Client = await albService.getEc2Sdk(requestContext, resolvedVars); - await ec2Client.revokeSecurityGroupEgress(params).promise(); - } catch (e) { - throw new Error( - `Revoking ALB security group from AppStream security group egress failed with error - ${e.message}`, - ); - } - } - /** * Method to get the cloud formation service client for the target aws account * From 92e30ccbf391a136ba5a10efa6c49ed2e9c3735a Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Tue, 9 Nov 2021 14:54:59 -0500 Subject: [PATCH 11/13] fix: add perms to cross acct role --- .../src/templates/onboard-account.cfn.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml index 3bf34f0e51..ec57f5b9e8 100644 --- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml +++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml @@ -415,6 +415,7 @@ Resources: - ec2:DescribeInstances - ec2:DescribeRouteTables - ec2:AssociateRouteTable + - ec2:DisassociateRouteTable Resource: '*' # For the actions listed above IAM does not support resource-level permissions and requires all resources to be chosen - Effect: Allow Action: From a31317644d98c7b692693afa4f3486a45e315e49 Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Thu, 11 Nov 2021 16:42:43 -0500 Subject: [PATCH 12/13] chore: update unit tests --- .../lib/alb/__tests__/alb-service.test.js | 232 +++++++++++++++++- .../__test__/check-launch-dependency.test.js | 30 +++ .../terminate-launch-dependency.test.js | 53 +++- .../terminate-launch-dependency.js | 7 +- 4 files changed, 302 insertions(+), 20 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index 0ca2015d83..62dd373e7d 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -256,16 +256,75 @@ describe('ALBService', () => { }); describe('getStackCreationInput', () => { - const resolvedInputParams = [ - { Key: 'ACMSSLCertARN', Value: 'Value' }, - { Key: 'IsAppStreamEnabled', Value: 'false' }, - ]; const resolvedVars = { namespace: 'namespace' }; + + it('should pass and return the stack creation input with success for AppStream', async () => { + service.checkIfAppStreamEnabled = jest.fn(() => { + return true; + }); + const resolvedInputParams = [ + { Key: 'ACMSSLCertARN', Value: 'Value' }, + { Key: 'IsAppStreamEnabled', Value: 'true' }, + ]; + service.findAwsAccountDetails = jest.fn(() => { + return { + subnetId: 'subnet-0a661d9f417ecff3f', + vpcId: 'vpc-096b034133955abba', + appStreamSecurityGroupId: 'sampleAppStreamSgId', + }; + }); + cfnTemplateService.getTemplate.mockImplementationOnce(() => { + return ['template']; + }); + const apiResponse = { + StackName: resolvedVars.namespace, + Parameters: [ + { + ParameterKey: 'Namespace', + ParameterValue: 'namespace', + }, + { + ParameterKey: 'ACMSSLCertARN', + ParameterValue: 'Value', + }, + { + ParameterKey: 'VPC', + ParameterValue: 'vpc-096b034133955abba', + }, + { + ParameterKey: 'IsAppStreamEnabled', + ParameterValue: 'true', + }, + { + ParameterKey: 'AppStreamSG', + ParameterValue: 'sampleAppStreamSgId', + }, + { + ParameterKey: 'PublicRouteTableId', + ParameterValue: 'N/A', + }, + ], + TemplateBody: ['template'], + Tags: [ + { + Key: 'Description', + Value: 'Created by SWB for the AWS account', + }, + ], + }; + const response = await service.getStackCreationInput({}, resolvedVars, resolvedInputParams, 'project_id'); + expect(response).toEqual(apiResponse); + }); it('should pass and return the stack creation input with success', async () => { + const resolvedInputParams = [ + { Key: 'ACMSSLCertARN', Value: 'Value' }, + { Key: 'IsAppStreamEnabled', Value: 'false' }, + ]; service.findAwsAccountDetails = jest.fn(() => { return { subnetId: 'subnet-0a661d9f417ecff3f', vpcId: 'vpc-096b034133955abba', + publicRouteTableId: 'rtb-sampleRouteTableId', }; }); cfnTemplateService.getTemplate.mockImplementationOnce(() => { @@ -296,7 +355,7 @@ describe('ALBService', () => { }, { ParameterKey: 'PublicRouteTableId', - ParameterValue: undefined, + ParameterValue: 'rtb-sampleRouteTableId', }, ], TemplateBody: ['template'], @@ -312,6 +371,10 @@ describe('ALBService', () => { }); it('should fail because project id is not valid', async () => { + const resolvedInputParams = [ + { Key: 'ACMSSLCertARN', Value: 'Value' }, + { Key: 'IsAppStreamEnabled', Value: 'false' }, + ]; projectService.mustFind.mockImplementationOnce(() => { throw service.boom.notFound(`project with id "test-id" does not exist`, true); }); @@ -351,10 +414,34 @@ describe('ALBService', () => { const targetGroupArn = 'rn:aws:elasticloadbalancing:us-east-2:977461429431:targetgroup/devrgsaas-sg/f4c2a2df084e5df4'; - it('should pass if system is trying to create listener rule', async () => { + it('should pass if system is trying to create listener rule for AppStream', async () => { + service.checkIfAppStreamEnabled = jest.fn(() => { + return true; + }); service.findAwsAccountId = jest.fn(() => { return 'sampleAwsAccountId'; }); + const subdomain = 'rtsudio-test.example.com'; + const priority = 1; + const params = { + ListenerArn: JSON.parse(albDetails.value).listenerArn, + Priority: priority, + Actions: [ + { + TargetGroupArn: targetGroupArn, + Type: 'forward', + }, + ], + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + ], + Tags: resolvedVars.tags, + }; service.updateAlbDependentWorkspaceCount = jest.fn(); albClient.describeRules = jest.fn().mockImplementation(() => { return { @@ -375,12 +462,74 @@ describe('ALBService', () => { return albDetails; }); service.getHostname = jest.fn(() => { - return 'rtsudio-test.example.com'; + return subdomain; }); jest.spyOn(service, 'calculateRulePriority').mockImplementationOnce(() => { - return 1; + return priority; }); await service.createListenerRule(prefix, requestContext, resolvedVars, targetGroupArn); + expect(albClient.createRule).toHaveBeenCalledWith(params); + expect(albClient.createRule).toHaveBeenCalled(); + }); + + it('should pass if system is trying to create listener rulefor non-AppStream', async () => { + service.findAwsAccountId = jest.fn(() => { + return 'sampleAwsAccountId'; + }); + service.updateAlbDependentWorkspaceCount = jest.fn(); + const subdomain = 'rtsudio-test.example.com'; + const priority = 1; + const params = { + ListenerArn: JSON.parse(albDetails.value).listenerArn, + Priority: priority, + Actions: [ + { + TargetGroupArn: targetGroupArn, + Type: 'forward', + }, + ], + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: [resolvedVars.cidr], + }, + }, + ], + Tags: resolvedVars.tags, + }; + albClient.describeRules = jest.fn().mockImplementation(() => { + return { + promise: () => { + return describeAPIResponse; + }, + }; + }); + albClient.createRule = jest.fn().mockImplementation(() => { + return { + promise: () => { + return createAPIResponse; + }, + }; + }); + service.getAlbSdk = jest.fn().mockResolvedValue(albClient); + service.getAlbDetails = jest.fn(() => { + return albDetails; + }); + service.getHostname = jest.fn(() => { + return subdomain; + }); + jest.spyOn(service, 'calculateRulePriority').mockImplementationOnce(() => { + return priority; + }); + await service.createListenerRule(prefix, requestContext, resolvedVars, targetGroupArn); + expect(albClient.createRule).toHaveBeenCalledWith(params); expect(albClient.createRule).toHaveBeenCalled(); }); @@ -510,7 +659,51 @@ describe('ALBService', () => { }); describe('modifyRule', () => { - it('should pass and return empty object with success', async () => { + it('should pass for AppStream', async () => { + service.checkIfAppStreamEnabled = jest.fn(() => { + return true; + }); + const resolvedVars = { + projectId: 'bio-research-vir2', + envId: '018bb1e1-6bd3-49d9-b608-051cfb180882', + prefix: 'rstudio', + ruleARN: + 'arn:aws:elasticloadbalancing:us-west-2:123456789012:listener-rule/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2/9683b2d02a6cabee', + }; + const subdomain = 'rtsudio-test.example.com'; + const params = { + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + ], + RuleArn: resolvedVars.ruleARN, + }; + service.getHostname = jest.fn(() => { + return subdomain; + }); + service.findAwsAccountDetails = jest.fn(() => { + return { + externalId: 'subnet-0a661d9f417ecff3f', + }; + }); + albClient.modifyRule = jest.fn().mockImplementation(() => { + return { + promise: () => { + return {}; + }, + }; + }); + service.getAlbSdk = jest.fn().mockResolvedValue(albClient); + const response = await service.modifyRule({}, resolvedVars); + expect(albClient.modifyRule).toHaveBeenCalledWith(params); + expect(response).toEqual({}); + }); + + it('should pass for non-AppStream', async () => { const resolvedVars = { projectId: 'bio-research-vir2', envId: '018bb1e1-6bd3-49d9-b608-051cfb180882', @@ -519,8 +712,26 @@ describe('ALBService', () => { ruleARN: 'arn:aws:elasticloadbalancing:us-west-2:123456789012:listener-rule/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2/9683b2d02a6cabee', }; + const subdomain = 'rtsudio-test.example.com'; + const params = { + Conditions: [ + { + Field: 'host-header', + HostHeaderConfig: { + Values: [subdomain], + }, + }, + { + Field: 'source-ip', + SourceIpConfig: { + Values: resolvedVars.cidr, + }, + }, + ], + RuleArn: resolvedVars.ruleARN, + }; service.getHostname = jest.fn(() => { - return 'rtsudio-test.example.com'; + return subdomain; }); service.findAwsAccountDetails = jest.fn(() => { return { @@ -536,6 +747,7 @@ describe('ALBService', () => { }); service.getAlbSdk = jest.fn().mockResolvedValue(albClient); const response = await service.modifyRule({}, resolvedVars); + expect(albClient.modifyRule).toHaveBeenCalledWith(params); expect(response).toEqual({}); }); diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/check-launch-dependency.test.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/check-launch-dependency.test.js index 58b4df0407..b6b549083c 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/check-launch-dependency.test.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/check-launch-dependency.test.js @@ -337,6 +337,7 @@ describe('CheckLaunchDependencyStep', () => { albArn: null, listenerArn: null, albDnsName: null, + albHostedZoneId: null, albSecurityGroup: null, albDependentWorkspacesCount: 0, }; @@ -344,6 +345,34 @@ describe('CheckLaunchDependencyStep', () => { expect(albService.saveAlbDetails).toHaveBeenCalledWith('test-account-id', albDetails); }); + it('should update alb details with output values for AppStream', async () => { + albService.findAwsAccountId.mockImplementationOnce(() => { + return 'test-account-id'; + }); + albService.getAlbHostedZoneId = jest.fn(() => { + return 'albHostedZoneId'; + }); + jest.spyOn(albService, 'saveAlbDetails').mockImplementationOnce(() => {}); + const output = { + LoadBalancerArn: 'test-alb-arn', + ListenerArn: 'test-listener-arn', + ALBDNSName: 'test-dns', + ALBSecurityGroupId: 'test-sg', + ALBHostedZoneId: 'albHostedZoneId', + }; + const albDetails = { + id: 'test-account-id', + albStackName: 'STACK_ID', + albArn: 'test-alb-arn', + listenerArn: 'test-listener-arn', + albDnsName: 'test-dns', + albHostedZoneId: 'albHostedZoneId', + albSecurityGroup: 'test-sg', + albDependentWorkspacesCount: 0, + }; + await step.handleStackCompletion(output); + expect(albService.saveAlbDetails).toHaveBeenCalledWith('test-account-id', albDetails); + }); it('should update alb details with output values', async () => { albService.findAwsAccountId.mockImplementationOnce(() => { return 'test-account-id'; @@ -361,6 +390,7 @@ describe('CheckLaunchDependencyStep', () => { albArn: 'test-alb-arn', listenerArn: 'test-listener-arn', albDnsName: 'test-dns', + albHostedZoneId: null, albSecurityGroup: 'test-sg', albDependentWorkspacesCount: 0, }; diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-launch-dependency.test.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-launch-dependency.test.js index ae28d8c574..30173657b1 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-launch-dependency.test.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-launch-dependency.test.js @@ -143,6 +143,9 @@ describe('TerminateLaunchDependencyStep', () => { deleteStack: jest.fn(), }; step.getCloudFormationService = jest.fn().mockResolvedValue(cfn); + step.checkIfAppStreamEnabled = jest.fn(() => { + return false; + }); albService.findAwsAccountId = jest.fn(() => { return 'test-account-id'; }); @@ -152,7 +155,7 @@ describe('TerminateLaunchDependencyStep', () => { lockService.releaseWriteLock = jest.fn(() => { return true; }); - // Mock locking so that the putBucketPolicy actually gets called + // Mock locking so that the fn() actually gets called lockService.tryWriteLockAndRun = jest.fn((params, callback) => callback()); step.cfnOutputsArrayToObject = jest.fn(() => { return { @@ -224,7 +227,40 @@ describe('TerminateLaunchDependencyStep', () => { expect(environmentScCidrService.revokeIngressRuleWithSecurityGroup).not.toHaveBeenCalled(); }); + it('should call delete private route53 record if type is RstudioV2 and alb exists for AppStream', async () => { + step.checkIfAppStreamEnabled = jest.fn(() => { + return true; + }); + const templateOutputs = { + NeedsALB: { Description: 'Needs ALB', Value: false }, + }; + step.cfnOutputsArrayToObject = jest.fn().mockImplementationOnce(() => { + return { + MetaConnection1Type: 'rstudiov2', + ListenerRuleARN: null, + }; + }); + environmentScService.getMemberAccount = jest.fn().mockImplementationOnce(() => { + return { + route53HostedZone: 'sampleRoute53HostedZone', + }; + }); + albService.checkAlbExists.mockImplementationOnce(() => { + return true; + }); + albService.getAlbHostedZoneID = jest.fn(); + jest.spyOn(step, 'getTemplateOutputs').mockImplementationOnce(() => { + return templateOutputs; + }); + environmentDnsService.deletePrivateRecordForDNS = jest.fn(); + albService.checkAndTerminateAlb = jest.fn(); + await step.start(); + expect(environmentDnsService.deleteRecord).not.toHaveBeenCalled(); + expect(environmentDnsService.deletePrivateRecordForDNS).toHaveBeenCalled(); + }); + it('should call delete route53 record if type is RstudioV2 and alb exists', async () => { + step.setting = { getBoolean: jest.fn(() => false) }; const templateOutputs = { NeedsALB: { Description: 'Needs ALB', Value: false }, }; @@ -443,7 +479,7 @@ describe('TerminateLaunchDependencyStep', () => { throw new Error('project with id "test-project" does not exist'); }); await expect( - step.terminateStack(requestContext, 'test-project-id', 'test-external-id', 'test-stack-name'), + step.terminateStack(requestContext, 'test-project-id', 'test-external-id', { albStackName: 'test-stack-id' }), ).rejects.toThrow('project with id "test-project" does not exist'); }); @@ -454,7 +490,9 @@ describe('TerminateLaunchDependencyStep', () => { }; }); step.getCloudFormationService = jest.fn().mockResolvedValue(cfn); - await step.terminateStack(requestContext, 'test-project-id', 'test-external-id', 'test-stack-id'); + await step.terminateStack(requestContext, 'test-project-id', 'test-external-id', { + albStackName: 'test-stack-id', + }); // CHECK expect(cfn.deleteStack).toHaveBeenCalled(); expect(step.state.setKey).toHaveBeenCalledWith('STACK_ID', 'test-stack-id'); @@ -468,12 +506,9 @@ describe('TerminateLaunchDependencyStep', () => { }); step.getCloudFormationService = jest.fn().mockResolvedValue(cfn); // OPERATE - const response = await step.terminateStack( - requestContext, - 'test-project-id', - 'test-external-id', - 'test-stack-id', - ); + const response = await step.terminateStack(requestContext, 'test-project-id', 'test-external-id', { + albStackName: 'test-stack-id', + }); // CHECK expect(response).toMatchObject({ waitDecision: { diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js index bf3868217c..a8386849b6 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-launch-dependency/terminate-launch-dependency.js @@ -105,7 +105,7 @@ class TerminateLaunchDependency extends StepBase { if (albExists) { try { - const isAppStreamEnabled = this.settings.getBoolean(settingKeys.isAppStreamEnabled); + const isAppStreamEnabled = this.checkIfAppStreamEnabled(); if (isAppStreamEnabled) { const memberAccount = await environmentScService.getMemberAccount(requestContext, environment); const albHostedZoneId = await albService.getAlbHostedZoneID( @@ -124,6 +124,7 @@ class TerminateLaunchDependency extends StepBase { } else { await environmentDnsService.deleteRecord('rstudio', envId, dnsName); } + this.print({ msg: 'Route53 record deleted successfully', }); @@ -199,6 +200,10 @@ class TerminateLaunchDependency extends StepBase { return await this.checkAndTerminateAlb(requestContext, projectId, externalId); } + checkIfAppStreamEnabled() { + return this.settings.getBoolean(settingKeys.isAppStreamEnabled); + } + /** * Method to check and terminate ALB if the environment is the last ALB dependent environment * From 575364df85aafa3657e37caede441ad1a283210a Mon Sep 17 00:00:00 2001 From: SanketD92 Date: Fri, 12 Nov 2021 18:51:28 -0500 Subject: [PATCH 13/13] fix: added rstudiov2 unit tests --- .../lib/alb/__tests__/alb-service.test.js | 18 +- .../__tests__/environment-dns-service.test.js | 142 ++++++++++++++- .../__tests__/env-provisioning-plugin.test.js | 168 ++++++++++++++++++ .../steps/__test__/provision-account.test.js | 2 + 4 files changed, 321 insertions(+), 9 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js index 62dd373e7d..c7677cf1b2 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/alb/__tests__/alb-service.test.js @@ -801,7 +801,6 @@ describe('ALBService', () => { albClient.describeRules = jest.fn().mockImplementation(() => { throw new Error(`Error calculating rule priority. Rule describe failed with message - Rule not found`); }); - // service.getAlbSdk = jest.fn().mockResolvedValue(albClient); try { await service.calculateRulePriority({}, {}, ''); } catch (err) { @@ -819,7 +818,6 @@ describe('ALBService', () => { }, }; }); - // service.getAlbSdk = jest.fn().mockResolvedValue(albClient); const response = await service.calculateRulePriority({}, {}, ''); expect(response).toEqual(1); }); @@ -834,9 +832,23 @@ describe('ALBService', () => { }, }; }); - // service.getAlbSdk = jest.fn().mockResolvedValue(albClient); const response = await service.calculateRulePriority({}, {}, ''); expect(response).toEqual(3); }); }); + + describe('getAlbHostedZoneID', () => { + it('should provide ALB hosted zone ID', async () => { + const albHostedZoneId = 'sampleAlbHostedZoneId'; + albClient.describeLoadBalancers = jest.fn().mockImplementation(() => { + return { + promise: () => { + return { LoadBalancers: [{ CanonicalHostedZoneId: albHostedZoneId }] }; + }, + }; + }); + const response = await service.getAlbHostedZoneID({}, {}, ''); + expect(response).toEqual(albHostedZoneId); + }); + }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/__tests__/environment-dns-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/__tests__/environment-dns-service.test.js index 118cb5b07e..d1084c7d06 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/__tests__/environment-dns-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/__tests__/environment-dns-service.test.js @@ -43,6 +43,12 @@ describe('EnvironmentDnsService', () => { service = await container.find('environmentDnsService'); environmentScService = await container.find('environmentScService'); settings = await container.find('settings'); + settings.get = jest.fn(key => { + if (key === 'domainName') { + return 'test.aws'; + } + throw Error(`${key} not found`); + }); }); describe('Test changePrivateRecordSet', () => { @@ -53,12 +59,6 @@ describe('EnvironmentDnsService', () => { environmentScService.getClientSdkWithEnvMgmtRole = jest.fn(() => { return route53Client; }); - settings.get = jest.fn(key => { - if (key === 'domainName') { - return 'test.aws'; - } - throw Error(`${key} not found`); - }); await service.changePrivateRecordSet(requestContext, 'CREATE', 'rstudio', 'test-id', '10.1.1.1', 'HOSTEDZONE123'); expect(service.changeResourceRecordSets).toHaveBeenCalledWith( route53Client, @@ -127,6 +127,56 @@ describe('EnvironmentDnsService', () => { }); }); + describe('Test createPrivateRecordForDNS', () => { + it('should call changePrivateRecordSetALB', async () => { + const requestContext = { principalIdentifier: { uid: 'u-testuser' } }; + service.changePrivateRecordSetALB = jest.fn(); + await service.createPrivateRecordForDNS( + requestContext, + 'rstudio', + 'test-id', + 'sampleAlbHostedZoneId', + 'sampleAlbDnsName', + 'HOSTEDZONE123', + ); + expect(service.changePrivateRecordSetALB).toHaveBeenCalledTimes(1); + expect(service.changePrivateRecordSetALB).toHaveBeenCalledWith( + requestContext, + 'CREATE', + 'rstudio', + 'test-id', + 'HOSTEDZONE123', + 'sampleAlbHostedZoneId', + 'sampleAlbDnsName', + ); + }); + }); + + describe('Test deletePrivateRecordForDNS', () => { + it('should call changePrivateRecordSetALB', async () => { + const requestContext = { principalIdentifier: { uid: 'u-testuser' } }; + service.changePrivateRecordSetALB = jest.fn(); + await service.deletePrivateRecordForDNS( + requestContext, + 'rstudio', + 'test-id', + 'sampleAlbHostedZoneId', + 'sampleAlbDnsName', + 'HOSTEDZONE123', + ); + expect(service.changePrivateRecordSetALB).toHaveBeenCalledTimes(1); + expect(service.changePrivateRecordSetALB).toHaveBeenCalledWith( + requestContext, + 'DELETE', + 'rstudio', + 'test-id', + 'HOSTEDZONE123', + 'sampleAlbHostedZoneId', + 'sampleAlbDnsName', + ); + }); + }); + describe('Test deletePrivateRecord', () => { it('should call changePrivateRecordSet', async () => { const requestContext = { principalIdentifier: { uid: 'u-testuser' } }; @@ -143,4 +193,84 @@ describe('EnvironmentDnsService', () => { ); }); }); + + describe('Test changePrivateRecordSetALB', () => { + it('should call changeResourceRecordSetsPrivateALB', async () => { + // BUILD + const requestContext = { principalIdentifier: { uid: 'u-testuser' } }; + environmentScService.getClientSdkWithEnvMgmtRole = jest.fn(() => { + return {}; + }); + + service.changeResourceRecordSetsPrivateALB = jest.fn(); + + // OPERATE + await service.changePrivateRecordSetALB( + requestContext, + 'ACTION', + 'rstudio', + 'test-id', + 'sampleHostedZoneId', + 'samplealbHostedZoneId', + 'sampleRecordValue', + ); + + // CHECK + expect(service.changeResourceRecordSetsPrivateALB).toHaveBeenCalledTimes(1); + expect(service.changeResourceRecordSetsPrivateALB).toHaveBeenCalledWith( + {}, + 'sampleHostedZoneId', + 'ACTION', + `rstudio-test-id.test.aws`, + 'samplealbHostedZoneId', + 'sampleRecordValue', + ); + }); + }); + + describe('Test changeResourceRecordSetsPrivateALB', () => { + it('should call changeResourceRecordSets', async () => { + // BUILD + const route53Client = jest.fn(); + route53Client.changeResourceRecordSets = jest.fn(() => { + return { + promise: jest.fn(), + }; + }); + + const params = { + HostedZoneId: 'sampleHostedZoneId', + ChangeBatch: { + Changes: [ + { + Action: 'ACTION', + ResourceRecordSet: { + Name: 'rstudio-test-id.test.aws', + Type: 'A', + AliasTarget: { + HostedZoneId: 'samplealbHostedZoneId', + DNSName: 'dualstack.sampleRecordValue', + EvaluateTargetHealth: false, + }, + }, + }, + ], + }, + }; + + // OPERATE + await service.changeResourceRecordSetsPrivateALB( + route53Client, + 'sampleHostedZoneId', + 'ACTION', + 'rstudio-test-id.test.aws', + 'samplealbHostedZoneId', + 'sampleRecordValue', + ); + + // CHECK + expect(route53Client.changeResourceRecordSets).toHaveBeenCalledTimes(1); + expect(route53Client.changeResourceRecordSets).toHaveBeenCalledWith(params); + }); + }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/__tests__/env-provisioning-plugin.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/__tests__/env-provisioning-plugin.test.js index 5675fb1136..4981324200 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/plugins/__tests__/env-provisioning-plugin.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/plugins/__tests__/env-provisioning-plugin.test.js @@ -22,13 +22,22 @@ const Logger = require('@aws-ee/base-services/lib/logger/logger-service'); jest.mock('@aws-ee/base-services/lib/plugin-registry/plugin-registry-service'); const PluginRegistryService = require('@aws-ee/base-services/lib/plugin-registry/plugin-registry-service'); +jest.mock('@aws-ee/base-services/lib/lock/lock-service'); +const LockService = require('@aws-ee/base-services/lib/lock/lock-service'); + jest.mock('../../environment/service-catalog/environment-sc-service'); const SettingsServiceMock = require('@aws-ee/base-services/lib/settings/env-settings-service'); const EnvironmentScService = require('../../environment/service-catalog/environment-sc-service'); +jest.mock('../../environment/service-catalog/environment-sc-cidr-service'); +const EnvironmentScCidrService = require('../../environment/service-catalog/environment-sc-service'); + jest.mock('../../environment/environment-dns-service'); const EnvironmentDNSService = require('../../environment/environment-dns-service'); +jest.mock('../../alb/alb-service'); +const AlbService = require('../../alb/alb-service'); + jest.mock('../../environment/service-catalog/environment-sc-keypair-service'); const EnvironmentSCKeyPairService = require('../../environment/service-catalog/environment-sc-keypair-service'); @@ -41,15 +50,21 @@ describe('envProvisioningPlugin', () => { let container; const requestContext = { principal: { isAdmin: true, status: 'active' } }; let environmentScService; + let environmentScCidrService; let pluginRegistryService; let environmentDnsService; + let albService; + let lockService; let settings; beforeEach(async () => { // Initialize services container and register dependencies container = new ServicesContainer(); container.register('environmentScService', new EnvironmentScService()); + container.register('environmentScCidrService', new EnvironmentScCidrService()); container.register('pluginRegistryService', new PluginRegistryService()); container.register('environmentDnsService', new EnvironmentDNSService()); + container.register('albService', new AlbService()); + container.register('lockService', new LockService()); container.register('environmentScKeypairService', new EnvironmentSCKeyPairService()); container.register('log', new Logger()); container.register('settings', new SettingsServiceMock()); @@ -59,6 +74,10 @@ describe('envProvisioningPlugin', () => { environmentScService = await container.find('environmentScService'); pluginRegistryService = await container.find('pluginRegistryService'); environmentDnsService = await container.find('environmentDnsService'); + environmentScCidrService = await container.find('environmentScCidrService'); + albService = await container.find('albService'); + lockService = await container.find('lockService'); + lockService.tryWriteLockAndRun = jest.fn((params, callback) => callback()); }); describe('preProvisioning', () => { @@ -303,6 +322,155 @@ describe('envProvisioningPlugin', () => { expect(environmentDnsService.createRecord).toHaveBeenCalledWith('rstudio', 'env-id', 'some-dns-name'); }); + it('should create environmentDNS record if it is RStudioV2 environment', async () => { + // BUILD + environmentScService.mustFind = jest.fn().mockResolvedValueOnce({ id: 'env-id' }); + settings.getBoolean = jest.fn(key => { + if (key === 'isAppStreamEnabled') { + return false; + } + throw Error(`${key} not found`); + }); + const albDetails = { + createdAt: '2021-05-21T13:06:58.216Z', + id: 'test-id', + type: 'account-workspace-details', + updatedAt: '2021-05-31T13:32:15.503Z', + value: + '{"id":"test-id","albStackName":null,"albArn":"arn:test-arn","listenerArn":"alb-listener-arn","albDnsName":"albDNSName","albDependentWorkspacesCount":1}', + }; + albService.getAlbDetails = jest.fn(() => { + return albDetails; + }); + albService.createListenerRule = jest.fn(() => { + return 'alb-listener-rule-arn'; + }); + environmentScCidrService.authorizeIngressRuleWithSecurityGroup = jest.fn(); + // OPERATE + await plugin.onEnvProvisioningSuccess({ + requestContext, + container, + resolvedVars: { envId: 'env-id' }, + status: 'SUCCEED', + outputs: [ + { OutputKey: 'MetaConnection1Type', OutputValue: 'RStudioV2' }, + { OutputKey: 'Ec2WorkspaceDnsName', OutputValue: 'some-dns-name' }, + { OutputKey: 'TargetGroupARN', OutputValue: 'some-target-group-arn' }, + { OutputKey: 'InstanceSecurityGroupId', OutputValue: 'some-security-group-id' }, + ], + provisionedProductId: 'provisioned-product-id', + }); + // CHECK + expect(environmentScService.update).toHaveBeenCalledWith( + { + principal: { + isAdmin: true, + status: 'active', + }, + }, + { + id: 'env-id', + rev: 0, + status: 'SUCCEED', + inWorkflow: 'false', + outputs: [ + { OutputKey: 'MetaConnection1Type', OutputValue: 'RStudioV2' }, + { OutputKey: 'Ec2WorkspaceDnsName', OutputValue: 'some-dns-name' }, + { OutputKey: 'TargetGroupARN', OutputValue: 'some-target-group-arn' }, + { OutputKey: 'InstanceSecurityGroupId', OutputValue: 'some-security-group-id' }, + { + OutputKey: 'ListenerRuleARN', + Description: 'ARN of the listener rule created by code', + OutputValue: 'alb-listener-rule-arn', + }, + ], + provisionedProductId: 'provisioned-product-id', + }, + ); + expect(environmentDnsService.createRecord).toHaveBeenCalledWith('rstudio', 'env-id', 'albDNSName'); + }); + + it('should create environmentDNS record if it is RStudioV2 AppStream environment', async () => { + // BUILD + environmentScService.getMemberAccount = jest + .fn() + .mockResolvedValueOnce({ route53HostedZone: 'route53HostedZone' }); + environmentScService.mustFind = jest.fn().mockResolvedValueOnce({ id: 'env-id' }); + settings.getBoolean = jest.fn(key => { + if (key === 'isAppStreamEnabled') { + return true; + } + throw Error(`${key} not found`); + }); + const albDetails = { + createdAt: '2021-05-21T13:06:58.216Z', + id: 'test-id', + type: 'account-workspace-details', + updatedAt: '2021-05-31T13:32:15.503Z', + value: + '{"id":"test-id","albStackName":null,"albArn":"arn:test-arn","listenerArn":"alb-listener-arn","albDnsName":"albDNSName","albDependentWorkspacesCount":1}', + }; + albService.getAlbHostedZoneID = jest.fn(() => { + return 'albHostedZoneId'; + }); + albService.getAlbDetails = jest.fn(() => { + return albDetails; + }); + albService.createListenerRule = jest.fn(() => { + return 'alb-listener-rule-arn'; + }); + environmentScCidrService.authorizeIngressRuleWithSecurityGroup = jest.fn(); + // OPERATE + await plugin.onEnvProvisioningSuccess({ + requestContext, + container, + resolvedVars: { envId: 'env-id' }, + status: 'SUCCEED', + outputs: [ + { OutputKey: 'MetaConnection1Type', OutputValue: 'RStudioV2' }, + { OutputKey: 'Ec2WorkspaceDnsName', OutputValue: 'some-dns-name' }, + { OutputKey: 'TargetGroupARN', OutputValue: 'some-target-group-arn' }, + { OutputKey: 'InstanceSecurityGroupId', OutputValue: 'some-security-group-id' }, + ], + provisionedProductId: 'provisioned-product-id', + }); + // CHECK + expect(environmentScService.update).toHaveBeenCalledWith( + { + principal: { + isAdmin: true, + status: 'active', + }, + }, + { + id: 'env-id', + rev: 0, + status: 'SUCCEED', + inWorkflow: 'false', + outputs: [ + { OutputKey: 'MetaConnection1Type', OutputValue: 'RStudioV2' }, + { OutputKey: 'Ec2WorkspaceDnsName', OutputValue: 'some-dns-name' }, + { OutputKey: 'TargetGroupARN', OutputValue: 'some-target-group-arn' }, + { OutputKey: 'InstanceSecurityGroupId', OutputValue: 'some-security-group-id' }, + { + OutputKey: 'ListenerRuleARN', + Description: 'ARN of the listener rule created by code', + OutputValue: 'alb-listener-rule-arn', + }, + ], + provisionedProductId: 'provisioned-product-id', + }, + ); + expect(environmentDnsService.createPrivateRecordForDNS).toHaveBeenCalledWith( + requestContext, + 'rstudio', + 'env-id', + 'albHostedZoneId', + 'albDNSName', + 'route53HostedZone', + ); + }); + it('should create private DNS record if it is RStudio environment when AppStream enabled', async () => { // BUILD environmentScService.mustFind = jest.fn().mockResolvedValueOnce({ id: 'env-id' }); diff --git a/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js b/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js index 373b5f73d0..e1b93f7a4e 100644 --- a/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js +++ b/addons/addon-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js @@ -307,6 +307,7 @@ describe('ProvisionAccount', () => { { OutputKey: 'CrossAccountEnvMgmtRoleArn', OutputValue: 'env-mgmt-role-arn-1' }, { OutputKey: 'EncryptionKeyArn', OutputValue: 'encryption-key-arn-1' }, { OutputKey: 'OnboardStatusRoleArn', OutputValue: 'arn-onboard-1234' }, + { OutputKey: 'PublicRouteTableId', OutputValue: 'rtb-12345' }, ], }, ], @@ -356,6 +357,7 @@ describe('ProvisionAccount', () => { vpcId: 'vpc-123', encryptionKeyArn: 'encryption-key-arn-1', subnetId: 'public-subnet-1', + publicRouteTableId: 'rtb-12345', }, ); });