Updated base on PR comments

awslabs · Aug 10, 2021 · 25c0159 · 25c0159
1 parent 3f0649c
commit 25c0159
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 21 deletions.
diff --git a/...ronment-type-mgmt-services/lib/environment-type/__tests__/env-type-config-service.test.js b/...ronment-type-mgmt-services/lib/environment-type/__tests__/env-type-config-service.test.js
@@ -86,7 +86,7 @@ describe('EnvTypeService', () => {
       if (settingKey === 'isAppStreamEnabled') {
         return true;
       }
-      return undefined;
+      throw Error(`${settingKey} not found`);
     });
   });
 

diff --git a/...i/packages/environment-type-mgmt-services/lib/environment-type/env-type-config-service.js b/...i/packages/environment-type-mgmt-services/lib/environment-type/env-type-config-service.js
@@ -13,6 +13,7 @@
  *  permissions and limitations under the License.
  */
 
+/* eslint-disable no-template-curly-in-string */
 const _ = require('lodash');
 const Service = require('@aws-ee/base-services-container/lib/service');
 const { isAllow, allowIfActive, allowIfAdmin } = require('@aws-ee/base-services/lib/authorization/authorization-utils');
@@ -249,10 +250,16 @@ class EnvTypeConfigService extends Service {
     });
     let params = [...updatedConfig.params];
     if (isAppStreamEnabled) {
-      params.push({
-        key: 'AccessFromCIDRBlock',
-        value: '',
-      });
+      params = [
+        ...params,
+        {
+          key: 'AccessFromCIDRBlock',
+          value: '',
+        },
+        // Let's automatically fill in these values for the customer
+        { key: 'EgressStoreIamPolicyDocument', value: '${egressStoreIamPolicyDocument}' },
+        { key: 'SolutionNamespace', value: '${solutionNamespace}' },
+      ];
     } else {
       params = [
         ...params,

diff --git a/...don-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js b/...don-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js
@@ -34,31 +34,25 @@ import { createForm } from '@aws-ee/base-ui/dist/helpers/form';
  */
 function getCfnParamsForm(cfnParams, existingParamValues) {
   const isAppStreamEnabled = process.env.REACT_APP_IS_APP_STREAM_ENABLED === 'true';
+  const keysToFilterOut = ['IsAppStreamEnabled', 'EgressStoreIamPolicyDocument', 'SolutionNamespace'];
+  if (isAppStreamEnabled) {
+    keysToFilterOut.push('AccessFromCIDRBlock');
+  }
   const filteredCfnParams = cfnParams.filter(cfnParam => {
     const { ParameterKey } = cfnParam;
-    // We don't need to provide `IsAppStreamEnabled` as a config option to user because we can pull this value from settings on the backend
-    const keysToFilterOut = ['IsAppStreamEnabled'];
-    if (isAppStreamEnabled) {
-      keysToFilterOut.push('AccessFromCIDRBlock');
-    } else {
-      keysToFilterOut.push('EgressStoreIamPolicyDocument');
-      keysToFilterOut.push('SolutionNamespace');
-    }
     // Include keys that are not in keysToFilterOut array
     return !keysToFilterOut.includes(ParameterKey);
   });
 
   const fields = {};
   filteredCfnParams.forEach(({ ParameterKey, Description, DefaultValue }) => {
     const existingValue = _.get(_.find(existingParamValues, { key: ParameterKey }), 'value') || DefaultValue;
-    if (!isAppStreamEnabled || (isAppStreamEnabled && !(ParameterKey === 'AccessFromCIDRBlock'))) {
-      fields[ParameterKey] = {
-        label: ParameterKey,
-        extra: { explain: Description },
-        value: existingValue,
-        rules: 'required',
-      };
-    }
+    fields[ParameterKey] = {
+      label: ParameterKey,
+      extra: { explain: Description },
+      value: existingValue,
+      rules: 'required',
+    };
   });
   return createForm(fields);
 }

diff --git a/...-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js b/...-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js
@@ -131,4 +131,49 @@ describe('CfnParamsForm', () => {
     expect(formMock.createForm).toHaveBeenCalledTimes(1);
     expect(formMock.createForm).toHaveBeenCalledWith(fieldsWithCidr);
   });
+
+  describe('should not return these fields if present in the CFN template: IsAppStreamEnabled, EgressStoreIamPolicyDocument, SolutionNamespace', () => {
+    const isAppStreamEnabledParam = {
+      ParameterKey: 'IsAppStreamEnabled',
+      Description: 'Enable AppStream',
+      DefaultValue: 'false',
+    };
+
+    const egressStoreIamPolicyDocumentParam = {
+      ParameterKey: 'EgressStoreIamPolicyDocument',
+      Description: 'Policy for egress store',
+    };
+
+    const solutionNamespaceParam = {
+      ParameterKey: 'SolutionNamespace',
+      Description: 'Namespace provided when onboarding your account',
+    };
+    it('AppStream Enabled', () => {
+      process.env.REACT_APP_IS_APP_STREAM_ENABLED = true;
+      runTest();
+    });
+
+    it('AppStream Disabled', () => {
+      process.env.REACT_APP_IS_APP_STREAM_ENABLED = false;
+      runTest();
+    });
+
+    function runTest() {
+      // BUILD
+      const params = [
+        ...defaultParams,
+        isAppStreamEnabledParam,
+        egressStoreIamPolicyDocumentParam,
+        solutionNamespaceParam,
+      ];
+      const fields = JSON.parse(JSON.stringify(defaultFields));
+      // OPERATE
+      const returnedForm = getCfnParamsForm(params, []);
+
+      // CHECK
+      expect(returnedForm).toBe(expectedForm);
+      expect(formMock.createForm).toHaveBeenCalledTimes(1);
+      expect(formMock.createForm).toHaveBeenCalledWith(fields);
+    }
+  });
 });
diff --git a/scripts/app-stream/SETUP.md b/scripts/app-stream/SETUP.md
@@ -25,6 +25,8 @@ Note: Please set up your [AWS Profile](https://docs.aws.amazon.com/cli/latest/us
 2. This will open a new tab in your browser. When the prompt ask for which user you would like to log in as, choose Administrator. This will take you to a Windows Desktop that you can interact with to create your AppStream image. 
 3. On the Windows desktop, click the `Start` button and type in `Windows Powershell`. Right click the application in the search result, and choose `Run as administrator`
 4. Run the following commands
+
+TODO: Change `feat-secure-workspace-egress` to `mainline` for the url when merging feat branch back to mainline 
 ```
 cd ~\Documents
 

diff --git a/scripts/app-stream/buildImage.ps1 b/scripts/app-stream/buildImage.ps1
@@ -3,6 +3,7 @@ Set-ExecutionPolicy Bypass -Scope Process -Force; [System.Net.ServicePointManage
 
 choco install -y putty.install
 
+# TODO: Change `feat-secure-workspace-egress` to `mainline` for the url when merging feat branch back to mainline
 Invoke-WebRequest -Uri https://raw.githubusercontent.com/awslabs/service-workbench-on-aws/feat-secure-workspace-egress/scripts/app-stream/ec2linux.ps1 -OutFile 'C:\Users\Public\Documents\EC2Linux.ps1'
 
 # Prepare remote desktop script with arguments