fix: Allow Sagemaker Appstream workspaces to autostop (#689)

* fix:Add AppStream param to envPollHandler error and allow Sagemaker instance access to Sagemaker API Endpoint * Remove commented out code Co-authored-by: Tim Nguyen <thingut@amazon.com>
awslabs · Sep 2, 2021 · f871a3e · f871a3e
1 parent baf724f
commit f871a3e
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 8 deletions.
diff --git a/...ns/addon-base-raas-ui/packages/base-raas-ui/src/parts/accounts/AwsAccountUpdateContent.js b/...ns/addon-base-raas-ui/packages/base-raas-ui/src/parts/accounts/AwsAccountUpdateContent.js
@@ -124,10 +124,16 @@ class AwsAccountUpdateContent extends React.Component {
   };
 
   render() {
+    const { isUpdateStep } = this.getStep();
     let shouldDisableDoneButton = this.shouldShowWarning && !this.warningAcknowledged;
     if (isAppStreamEnabled) {
-      shouldDisableDoneButton =
-        shouldDisableDoneButton || !this.accessAppStreamAcknowledged || !this.startedAppStreamFleetAcknowledged;
+      // No acknowledgements is necessary if we're just updating the preexisting AppStream account
+      if (isUpdateStep) {
+        shouldDisableDoneButton = false;
+      } else {
+        shouldDisableDoneButton =
+          shouldDisableDoneButton || !this.accessAppStreamAcknowledged || !this.startedAppStreamFleetAcknowledged;
+      }
     }
     return (
       <Container className="mt3 animated fadeIn">
@@ -202,15 +208,21 @@ class AwsAccountUpdateContent extends React.Component {
     );
   }
 
-  renderSteps() {
+  getStep() {
     // We need to determine if this is for creating the stack or updating the stack
     const form = this.form;
     const stackInfo = this.stackInfo;
     const { hasUpdateStackUrl } = stackInfo;
     const field = form.$('createOrUpdate');
-    const update = field.value === 'update';
+    const isUpdateStep = field.value === 'update';
     const hasAdminAccess = form.$('managed').value === 'admin';
 
+    return { isUpdateStep, hasAdminAccess, hasUpdateStackUrl, field };
+  }
+
+  renderSteps() {
+    const { isUpdateStep, hasAdminAccess, hasUpdateStackUrl, field } = this.getStep();
+
     return (
       <>
         <div className="flex justify-between pt3 pb0 pr3 pl1">
@@ -219,9 +231,9 @@ class AwsAccountUpdateContent extends React.Component {
           </Header>
           {hasUpdateStackUrl && <YesNo field={field} className="mb0 mt0" />}
         </div>
-        {!update && hasAdminAccess && this.renderCreateSteps()}
-        {update && hasAdminAccess && this.renderUpdateSteps()}
-        {!hasAdminAccess && this.renderEmailTemplate(update)}
+        {!isUpdateStep && hasAdminAccess && this.renderCreateSteps()}
+        {isUpdateStep && hasAdminAccess && this.renderUpdateSteps()}
+        {!hasAdminAccess && this.renderEmailTemplate(isUpdateStep)}
       </>
     );
   }

diff --git a/...ns/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml b/...ns/addon-base-raas/packages/base-raas-cfn-templates/src/templates/onboard-account.cfn.yml
@@ -737,6 +737,30 @@ Resources:
         - !Ref SageMakerSecurityGroup
       VpcId: !Ref VPC
 
+  SagemakerApiEndpoint:
+    Type: 'AWS::EC2::VPCEndpoint'
+    Condition: isAppStream
+    Properties:
+      SubnetIds:
+        - !Ref PrivateWorkspaceSubnet
+      VpcEndpointType: Interface
+      PrivateDnsEnabled: true
+      ServiceName: !Sub "com.amazonaws.${AWS::Region}.sagemaker.api"
+      VpcId: !Ref VPC
+      SecurityGroupIds:
+        - !Ref SagemakerApiEndpointSecurityGroup
+
+  SagemakerApiEndpointSecurityGroup:
+    Type: AWS::EC2::SecurityGroup
+    Condition: isAppStream
+    Properties:
+      GroupDescription: 'Sagemaker Api Endpoint Security Group for interface endpoint'
+      GroupName: 'Sagemaker-API-SG'
+      VpcId: !Ref VPC
+      SecurityGroupIngress:
+        - SourceSecurityGroupId: !GetAtt VPC.DefaultSecurityGroup
+          IpProtocol: '-1'
+
   SageMakerSecurityGroup:
     Type: AWS::EC2::SecurityGroup
     Condition: isAppStream
@@ -915,6 +939,13 @@ Outputs:
     Export:
       Name: !Join ['', [Ref: Namespace, '-SageMakerVPCE']]
 
+  SageMakerApiSG:
+    Description: SageMaker API SG
+    Condition: isAppStream
+    Value: !Ref SagemakerApiEndpointSecurityGroup
+    Export:
+      Name: !Join ['', [Ref: Namespace, '-SageMakerApiSecurityGroup']]
+
   Route53HostedZone:
     Description: Route53 hosted zone
     Condition: isAppStreamAndCustomDomain

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
@@ -68,7 +68,13 @@ Resources:
             FromPort: 443
             ToPort: 443
             CidrIp: !Ref AccessFromCIDRBlock
-
+      SecurityGroupEgress:
+        - !If
+          - AppStreamEnabled
+          - DestinationSecurityGroupId:
+              Fn::ImportValue: !Sub "${SolutionNamespace}-SageMakerApiSecurityGroup"
+            IpProtocol: '-1'
+          - !Ref "AWS::NoValue"
   PreSignedURLBoundary:
     Type: AWS::IAM::ManagedPolicy
     Condition: AppStreamEnabled

diff --git a/main/solution/backend/config/infra/functions.yml b/main/solution/backend/config/infra/functions.yml
@@ -38,6 +38,7 @@ envStatusPollHandler:
         description: 'Invokes the lambda function that polls and synchronize environment status.'
   environment:
     APP_CUSTOM_USER_AGENT: ${self:custom.settings.customUserAgent}
+    APP_IS_APP_STREAM_ENABLED: ${self:custom.settings.isAppStreamEnabled}
 
 dataSourceReachabilityHandler:
   handler: src/lambdas/data-source-reachability/handler.handler

diff --git a/main/solution/post-deployment/config/infra/cloudformation.yml b/main/solution/post-deployment/config/infra/cloudformation.yml
@@ -335,6 +335,7 @@ Resources:
               - Effect: Allow
                 Action:
                   - ec2:AuthorizeSecurityGroupIngress
+                  - ec2:AuthorizeSecurityGroupEgress
                   - ec2:RevokeSecurityGroupEgress
                   - ec2:CreateSecurityGroup
                   - ec2:DeleteSecurityGroup