fix: get appstream setting optional bool (#633)

* fix: get appstream setting optional bool * fix: changing optionalBoolean to getBoolean * fix: unit tests for getBoolean * undo potential merge conflict
awslabs · Aug 5, 2021 · 74c206a · 74c206a
1 parent 41527a2
commit 74c206a
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 37 deletions.
diff --git a/...addon-base-raas/packages/base-raas-services/lib/account/__tests__/account-service.test.js b/...addon-base-raas/packages/base-raas-services/lib/account/__tests__/account-service.test.js
@@ -181,9 +181,9 @@ describe('accountService', () => {
         delete inputMissingAppStreamData.appStreamImageName;
         delete inputMissingAppStreamData.appStreamInstanceType;
 
-        settingsService.get = jest.fn(key => {
+        settingsService.getBoolean = jest.fn(key => {
           if (key === 'isAppStreamEnabled') {
-            return 'true';
+            return true;
           }
           return undefined;
         });

diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/account/account-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/account/account-service.js
@@ -92,7 +92,7 @@ class AccountService extends Service {
     await validationService.ensureValid(rawData, createSchema);
 
     let appStreamConfig = {};
-    if (this.settings.get(settingKeys.isAppStreamEnabled) === 'true') {
+    if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) {
       const {
         appStreamFleetDesiredInstances,
         appStreamDisconnectTimeoutSeconds,

diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-accounts-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-accounts-service.js
@@ -170,7 +170,7 @@ class AwsAccountsService extends Service {
     });
 
     // Only try to shareAppStreamImage with member account if AppStream is enabled and appStreamImageName is provided
-    if (this.settings.get(settingKeys.isAppStreamEnabled) === 'true' && rawData.appStreamImageName !== undefined) {
+    if (this.settings.getBoolean(settingKeys.isAppStreamEnabled) && rawData.appStreamImageName !== undefined) {
       const appStreamImageName = rawData.appStreamImageName;
       const accountId = rawData.accountId;
       await this.shareAppStreamImageWithMemberAccount(requestContext, accountId, appStreamImageName);

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
@@ -447,7 +447,7 @@ class AwsCfnService extends Service {
     fieldsToUpdate.id = accountEntity.id;
     fieldsToUpdate.rev = accountEntity.rev;
 
-    if (this.settings.get(settingKeys.isAppStreamEnabled) === 'true') {
+    if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) {
       fieldsToUpdate.subnetId = findOutputValue('PrivateWorkspaceSubnet');
       fieldsToUpdate.appStreamStackName = findOutputValue('AppStreamStackName');
       fieldsToUpdate.appStreamFleetName = findOutputValue('AppStreamFleet');

diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/project/project-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/project/project-service.js
@@ -48,7 +48,7 @@ class ProjectService extends Service {
     const [dbService] = await this.service(['dbService']);
     const table = this.settings.get(settingKeys.tableName);
     this.userService = await this.service('userService');
-    this.isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled) === 'true';
+    this.isAppStreamEnabled = this.settings.getBoolean(settingKeys.isAppStreamEnabled);
 
     this._getter = () => dbService.helper.getter().table(table);
     this._updater = () => dbService.helper.updater().table(table);

diff --git a/...-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js b/...-base-raas/packages/base-raas-workflow-steps/lib/steps/__test__/provision-account.test.js
@@ -92,9 +92,9 @@ describe('ProvisionAccount', () => {
     it('AppStream: false', async () => {
       // BUILD
       step.settings = {
-        get: key => {
+        getBoolean: key => {
           if (key === 'isAppStreamEnabled') {
-            return 'false';
+            return false;
           }
           throw new Error('Unexpected key');
         },
@@ -116,9 +116,9 @@ describe('ProvisionAccount', () => {
     it('AppStream: true', async () => {
       // BUILD
       step.settings = {
-        get: key => {
+        getBoolean: key => {
           if (key === 'isAppStreamEnabled') {
-            return 'true';
+            return true;
           }
           throw new Error('Unexpected key');
         },
@@ -273,9 +273,9 @@ describe('ProvisionAccount', () => {
     it('AppStream: false', async () => {
       // BUILD
       step.settings = {
-        get: key => {
+        getBoolean: key => {
           if (key === 'isAppStreamEnabled') {
-            return 'false';
+            return false;
           }
           throw new Error('Unexpected key');
         },
@@ -351,9 +351,9 @@ describe('ProvisionAccount', () => {
     it('AppStream: true', async () => {
       // BUILD
       step.settings = {
-        get: key => {
+        getBoolean: key => {
           if (key === 'isAppStreamEnabled') {
-            return 'true';
+            return true;
           }
           throw new Error('Unexpected key');
         },

diff --git a/...e-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js b/...e-raas/packages/base-raas-workflow-steps/lib/steps/provision-account/provision-account.js
@@ -99,7 +99,7 @@ class ProvisionAccount extends StepBase {
     // THIS IS NEEDED, we should wait AWS to setup the account, even if we can fetch the account ID
     this.print('start to wait for 5 minutes for AWS getting the account ready.');
 
-    if (this.settings.get(settingKeys.isAppStreamEnabled) === 'true') {
+    if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) {
       return this.wait(60 * 5).thenCall('shareImageWithMemberAccount');
     }
     return this.wait(60 * 5).thenCall('deployStack');
@@ -292,7 +292,7 @@ class ProvisionAccount extends StepBase {
           permissionStatus: 'CURRENT',
         };
         let additionalAccountData = {};
-        if (this.settings.get(settingKeys.isAppStreamEnabled) === 'true') {
+        if (this.settings.getBoolean(settingKeys.isAppStreamEnabled)) {
           // Start AppStream Fleet and wait for AppStream fleet to transition to RUNNING state
           await this.startAppStreamFleet(cfnOutputs.AppStreamFleet);
           const isAppStreamFleetRunning = await this.checkAppStreamFleetIsRunning(cfnOutputs.AppStreamFleet);
@@ -321,10 +321,9 @@ class ProvisionAccount extends StepBase {
           cfnInfo: {
             stackId,
             vpcId: cfnOutputs.VPC,
-            subnetId:
-              this.settings.get(settingKeys.isAppStreamEnabled) === 'true'
-                ? cfnOutputs.PrivateWorkspaceSubnet
-                : cfnOutputs.VpcPublicSubnet1,
+            subnetId: this.settings.getBoolean(settingKeys.isAppStreamEnabled)
+              ? cfnOutputs.PrivateWorkspaceSubnet
+              : cfnOutputs.VpcPublicSubnet1,
             crossAccountExecutionRoleArn: cfnOutputs.CrossAccountExecutionRoleArn,
             crossAccountEnvMgmtRoleArn: cfnOutputs.CrossAccountEnvMgmtRoleArn,
             encryptionKeyArn: cfnOutputs.EncryptionKeyArn,

diff --git a/addons/addon-base/packages/services/lib/s3-service.js b/addons/addon-base/packages/services/lib/s3-service.js
@@ -244,7 +244,7 @@ class S3Service extends Service {
       await s3Client.putObject(params).promise();
     } catch (error) {
       throw this.boom.badRequest(
-        `S3Service error with putting object to bukcet: ${params.Bucket} with key: ${params.Key}`,
+        `S3Service error with putting object to bucket: ${params.Bucket} with key: ${params.Key}`,
         true,
       );
     }

diff --git a/...don-stack-policy/packages/stack-policy/lib/steps/__test__/update-cfn-stack-policy.test.js b/...don-stack-policy/packages/stack-policy/lib/steps/__test__/update-cfn-stack-policy.test.js
@@ -49,9 +49,6 @@ describe('UpdateCfnStackPolicy', () => {
     service = await container.find('UpdateCfnStackPolicy');
     settings = await container.find('settings');
     settings.get = jest.fn(key => {
-      if (key === 'isAppStreamEnabled') {
-        return true;
-      }
       if (key === 'enableEgressStore') {
         return 'true';
       }
@@ -60,6 +57,12 @@ describe('UpdateCfnStackPolicy', () => {
       }
       return undefined;
     });
+    settings.getBoolean = jest.fn(key => {
+      if (key === 'isAppStreamEnabled') {
+        return true;
+      }
+      return undefined;
+    });
   });
 
   describe('Run post deployment step', () => {
@@ -366,9 +369,6 @@ describe('UpdateCfnStackPolicy', () => {
         ],
       };
       settings.get = jest.fn(key => {
-        if (key === 'isAppStreamEnabled') {
-          return false;
-        }
         if (key === 'enableEgressStore') {
           return 'true';
         }
@@ -377,6 +377,12 @@ describe('UpdateCfnStackPolicy', () => {
         }
         return undefined;
       });
+      settings.getBoolean = jest.fn(key => {
+        if (key === 'isAppStreamEnabled') {
+          return false;
+        }
+        return undefined;
+      });
       service.cfn.getStackPolicy = jest.fn(() => ({
         promise: () =>
           Promise.resolve({
@@ -416,9 +422,6 @@ describe('UpdateCfnStackPolicy', () => {
         ],
       };
       settings.get = jest.fn(key => {
-        if (key === 'isAppStreamEnabled') {
-          return true;
-        }
         if (key === 'enableEgressStore') {
           return 'false';
         }
@@ -427,6 +430,12 @@ describe('UpdateCfnStackPolicy', () => {
         }
         return undefined;
       });
+      settings.getBoolean = jest.fn(key => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return undefined;
+      });
       service.cfn.getStackPolicy = jest.fn(() => ({
         promise: () =>
           Promise.resolve({
@@ -444,9 +453,6 @@ describe('UpdateCfnStackPolicy', () => {
 
     it('should not update policy when Egress was disabled, but it was previously enabled', async () => {
       settings.get = jest.fn(key => {
-        if (key === 'isAppStreamEnabled') {
-          return false;
-        }
         if (key === 'enableEgressStore') {
           return 'false';
         }
@@ -455,6 +461,12 @@ describe('UpdateCfnStackPolicy', () => {
         }
         return undefined;
       });
+      settings.getBoolean = jest.fn(key => {
+        if (key === 'isAppStreamEnabled') {
+          return false;
+        }
+        return undefined;
+      });
       service.cfn.getStackPolicy = jest.fn();
       service.cfn.setStackPolicy = jest.fn();
 
@@ -465,9 +477,6 @@ describe('UpdateCfnStackPolicy', () => {
 
     it('should not update policy when AppStream was disabled, but it was previously enabled', async () => {
       settings.get = jest.fn(key => {
-        if (key === 'isAppStreamEnabled') {
-          return false;
-        }
         if (key === 'enableEgressStore') {
           return 'false';
         }
@@ -476,6 +485,12 @@ describe('UpdateCfnStackPolicy', () => {
         }
         return undefined;
       });
+      settings.getBoolean = jest.fn(key => {
+        if (key === 'isAppStreamEnabled') {
+          return false;
+        }
+        return undefined;
+      });
       service.cfn.getStackPolicy = jest.fn();
       service.cfn.setStackPolicy = jest.fn();
 

diff --git a/addons/addon-stack-policy/packages/stack-policy/lib/steps/update-cfn-stack-policy.js b/addons/addon-stack-policy/packages/stack-policy/lib/steps/update-cfn-stack-policy.js
@@ -58,7 +58,7 @@ class UpdateCfnStackPolicy extends Service {
   async execute() {
     const enableEgressStore = this.settings.get(settingKeys.enableEgressStore);
     const isEgressStoreEnabled = enableEgressStore ? enableEgressStore.toUpperCase() === 'TRUE' : false;
-    const isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled);
+    const isAppStreamEnabled = this.settings.getBoolean(settingKeys.isAppStreamEnabled);
 
     if (!isEgressStoreEnabled && !isAppStreamEnabled) {
       this.log.info('AppStream and EgressStore disabled. CFN stack policy does not need updates');