fix: Update CIDR fetching based on updated CFN template

awslabs · Aug 5, 2021 · 4767ff3 · 4767ff3
1 parent b0fa089
commit 4767ff3
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 13 deletions.
diff --git a/...base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml b/...base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
@@ -61,13 +61,13 @@ Resources:
       VpcId:
         Ref: VPC
       SecurityGroupIngress:
-        !If
-        - AppStreamEnabled
-        - !Ref 'AWS::NoValue'
-        - IpProtocol: tcp
-          FromPort: 443
-          ToPort: 443
-          CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
+          - !Ref "AWS::NoValue"
+          - IpProtocol: tcp
+            FromPort: 443
+            ToPort: 443
+            CidrIp: !Ref AccessFromCIDRBlock
 
   PreSignedURLBoundary:
     Type: AWS::IAM::ManagedPolicy

diff --git a/...se-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/...se-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
@@ -1438,6 +1438,87 @@ describe('EnvironmentSCService', () => {
   });
 
   describe('getSecurityGroupDetails function', () => {
+    it('should send filtered security group rules as expected for AppStream template', async () => {
+      // BUILD
+      const requestContext = {};
+      const stackArn = 'sampleCloudFormationStackArn';
+      const environment = {
+        outputs: [{ OutputKey: 'CloudformationStackARN', OutputValue: `<AwsAccountRoot>/${stackArn}` }],
+        status: 'COMPLETED',
+      };
+      const origSecurityGroupId = 'sampleSecurityGroupId';
+      const stackResources = {
+        StackResourceSummaries: [{ LogicalResourceId: 'SecurityGroup', PhysicalResourceId: origSecurityGroupId }],
+      };
+      const templateDetails = {
+        TemplateBody: YAML.dump({
+          Resources: {
+            SecurityGroup: {
+              Properties: {
+                SecurityGroupIngress: [
+                  {
+                    'Fn::If': [
+                      'AppStreamEnabled',
+                      {
+                        SourceSecurityGroupId: {},
+                        IpProtocol: '-1',
+                      },
+                      {
+                        IpProtocol: 'tcp',
+                        FromPort: 123,
+                        ToPort: 123,
+                        CidrIp: {
+                          Ref: 'AccessFromCIDRBlock',
+                        },
+                      },
+                    ],
+                  },
+                ],
+              },
+            },
+          },
+        }),
+      };
+      const workspaceIngressRules = [
+        {
+          IpProtocol: 'tcp',
+          FromPort: 123,
+          ToPort: 123,
+          IpRanges: [{ CidrIp: '123.123.123.123/32' }],
+        },
+        {
+          IpProtocol: 'tcp',
+          FromPort: 1,
+          ToPort: 1,
+          IpRanges: [{ CidrIp: '123.123.123.123/32' }],
+        },
+      ];
+      service.getCfnDetails = jest.fn(() => {
+        return { stackResources, templateDetails };
+      });
+      service.getWorkspaceSecurityGroup = jest.fn(() => {
+        return { securityGroupResponse: { SecurityGroups: [{ IpPermissions: workspaceIngressRules }] } };
+      });
+      const expectedOutcome = [
+        {
+          protocol: 'tcp',
+          fromPort: 123,
+          toPort: 123,
+          cidrBlocks: ['123.123.123.123/32'],
+        },
+      ];
+
+      // OPERATE
+      const { currentIngressRules, securityGroupId } = await service.getSecurityGroupDetails(
+        requestContext,
+        environment,
+      );
+
+      // CHECK
+      expect(currentIngressRules).toMatchObject(expectedOutcome);
+      expect(securityGroupId).toEqual(origSecurityGroupId);
+    });
+
     it('should send filtered security group rules as expected', async () => {
       // BUILD
       const requestContext = {};

diff --git a/...aas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/...aas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
@@ -1020,19 +1020,23 @@ class EnvironmentScService extends Service {
 
     // Only send back details of groups configured by the SC CFN stack
     const returnVal = _.map(cfnTemplateIngressRules, cfnRule => {
+      let ruleToUse = cfnRule;
+      if ('Fn::If' in cfnRule && cfnRule['Fn::If'][0] === 'AppStreamEnabled') {
+        ruleToUse = cfnRule['Fn::If'][2];
+      }
       const matchingRule = _.find(
         workspaceIngressRules,
         workspaceRule =>
-          cfnRule.FromPort === workspaceRule.FromPort &&
-          cfnRule.ToPort === workspaceRule.ToPort &&
-          cfnRule.IpProtocol === workspaceRule.IpProtocol,
+          ruleToUse.FromPort === workspaceRule.FromPort &&
+          ruleToUse.ToPort === workspaceRule.ToPort &&
+          ruleToUse.IpProtocol === workspaceRule.IpProtocol,
       );
       const currentCidrRanges = matchingRule ? _.map(matchingRule.IpRanges, ipRange => ipRange.CidrIp) : [];
 
       return {
-        fromPort: cfnRule.FromPort,
-        toPort: cfnRule.ToPort,
-        protocol: cfnRule.IpProtocol,
+        fromPort: ruleToUse.FromPort,
+        toPort: ruleToUse.ToPort,
+        protocol: ruleToUse.IpProtocol,
         cidrBlocks: currentCidrRanges,
       };
     });