fix(spel): Fix exceptions when account is an expression (#7168)

spinnaker · Jul 9, 2019 · a468de4 · a468de4
1 parent 768c210
commit a468de4
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 45 deletions.
diff --git a/app/scripts/modules/titus/src/serverGroup/configure/serverGroupConfiguration.service.ts b/app/scripts/modules/titus/src/serverGroup/configure/serverGroupConfiguration.service.ts
@@ -171,7 +171,12 @@ export class TitusServerGroupConfigurationService {
       .then((backingData: any) => {
         backingData.accounts = Object.keys(backingData.credentialsKeyedByAccount);
         backingData.filtered = {};
-        backingData.filtered.regions = backingData.credentialsKeyedByAccount[cmd.credentials].regions;
+        if (cmd.credentials.includes('${')) {
+          // If our dependency is an expression, the only thing we can really do is to just preserve current selections
+          backingData.filtered.regions = [{ name: cmd.region }];
+        } else {
+          backingData.filtered.regions = (backingData.credentialsKeyedByAccount[cmd.credentials] || []).regions || [];
+        }
         cmd.backingData = backingData;
         backingData.filtered.securityGroups = this.getRegionalSecurityGroups(cmd);
         let securityGroupRefresher = $q.when();
@@ -210,40 +215,45 @@ export class TitusServerGroupConfigurationService {
 
   private configureSecurityGroupOptions(command: ITitusServerGroupCommand): void {
     const currentOptions = command.backingData.filtered.securityGroups;
-    const newRegionalSecurityGroups = this.getRegionalSecurityGroups(command);
-    const isExpression =
-      typeof command.securityGroups === 'string' && (command.securityGroups as string).includes('${');
-    if (currentOptions && command.securityGroups && !isExpression) {
-      // not initializing - we are actually changing groups
-      const currentGroupNames: string[] = command.securityGroups.map((groupId: string) => {
-        const match = currentOptions.find(o => o.id === groupId);
-        return match ? match.name : groupId;
-      });
+    if (command.credentials.includes('${') || command.region.includes('${')) {
+      // If any of our dependencies are expressions, the only thing we can do is preserve current values
+      command.backingData.filtered.securityGroups = command.securityGroups.map(group => ({ name: group, id: group }));
+    } else {
+      const newRegionalSecurityGroups = this.getRegionalSecurityGroups(command);
+      const isExpression =
+        typeof command.securityGroups === 'string' && (command.securityGroups as string).includes('${');
+      if (currentOptions && command.securityGroups && !isExpression) {
+        // not initializing - we are actually changing groups
+        const currentGroupNames: string[] = command.securityGroups.map((groupId: string) => {
+          const match = currentOptions.find(o => o.id === groupId);
+          return match ? match.name : groupId;
+        });
 
-      const matchedGroups = command.securityGroups
-        .map((groupId: string) => {
-          const securityGroup: any = currentOptions.find(o => o.id === groupId || o.name === groupId);
-          return securityGroup ? securityGroup.name : null;
-        })
-        .map((groupName: string) => newRegionalSecurityGroups.find(g => g.name === groupName))
-        .filter((group: any) => group);
+        const matchedGroups = command.securityGroups
+          .map((groupId: string) => {
+            const securityGroup: any = currentOptions.find(o => o.id === groupId || o.name === groupId);
+            return securityGroup ? securityGroup.name : null;
+          })
+          .map((groupName: string) => newRegionalSecurityGroups.find(g => g.name === groupName))
+          .filter((group: any) => group);
 
-      const matchedGroupNames: string[] = matchedGroups.map(g => g.name);
-      const removed: string[] = xor(currentGroupNames, matchedGroupNames);
-      command.securityGroups = matchedGroups.map(g => g.id);
-      if (removed.length) {
-        command.viewState.dirty.securityGroups = removed;
+        const matchedGroupNames: string[] = matchedGroups.map(g => g.name);
+        const removed: string[] = xor(currentGroupNames, matchedGroupNames);
+        command.securityGroups = matchedGroups.map(g => g.id);
+        if (removed.length) {
+          command.viewState.dirty.securityGroups = removed;
+        }
       }
+      command.backingData.filtered.securityGroups = newRegionalSecurityGroups.sort((a, b) => {
+        if (command.securityGroups.includes(a.id)) {
+          return -1;
+        }
+        if (command.securityGroups.includes(b.id)) {
+          return 1;
+        }
+        return a.name.localeCompare(b.name);
+      });
     }
-    command.backingData.filtered.securityGroups = newRegionalSecurityGroups.sort((a, b) => {
-      if (command.securityGroups.includes(a.id)) {
-        return -1;
-      }
-      if (command.securityGroups.includes(b.id)) {
-        return 1;
-      }
-      return a.name.localeCompare(b.name);
-    });
   }
 
   public refreshSecurityGroups(command: ITitusServerGroupCommand, skipCommandReconfiguration: boolean): IPromise<void> {
@@ -294,19 +304,25 @@ export class TitusServerGroupConfigurationService {
 
   public configureLoadBalancerOptions(command: ITitusServerGroupCommand) {
     const currentTargetGroups = command.targetGroups || [];
-    const allTargetGroups = this.getTargetGroupNames(command);
+    if (command.credentials.includes('${') || command.region.includes('${')) {
+      // If any of our dependencies are expressions, the only thing we can do is preserve current values
+      command.targetGroups = currentTargetGroups;
+      (command.backingData.filtered as any).targetGroups = currentTargetGroups;
+    } else {
+      const allTargetGroups = this.getTargetGroupNames(command);
 
-    if (currentTargetGroups && command.targetGroups) {
-      const matched = intersection(allTargetGroups, currentTargetGroups);
-      const removedTargetGroups = xor(matched, currentTargetGroups);
-      command.targetGroups = intersection(allTargetGroups, matched);
-      if (removedTargetGroups && removedTargetGroups.length > 0) {
-        command.viewState.dirty.targetGroups = removedTargetGroups;
-      } else {
-        delete command.viewState.dirty.targetGroups;
+      if (currentTargetGroups && command.targetGroups) {
+        const matched = intersection(allTargetGroups, currentTargetGroups);
+        const removedTargetGroups = xor(matched, currentTargetGroups);
+        command.targetGroups = intersection(allTargetGroups, matched);
+        if (removedTargetGroups && removedTargetGroups.length > 0) {
+          command.viewState.dirty.targetGroups = removedTargetGroups;
+        } else {
+          delete command.viewState.dirty.targetGroups;
+        }
       }
+      (command.backingData.filtered as any).targetGroups = allTargetGroups;
     }
-    (command.backingData.filtered as any).targetGroups = allTargetGroups;
   }
 
   public refreshLoadBalancers(command: ITitusServerGroupCommand) {

diff --git a/app/scripts/modules/titus/src/serverGroup/configure/wizard/TitusCloneServerGroupModal.tsx b/app/scripts/modules/titus/src/serverGroup/configure/wizard/TitusCloneServerGroupModal.tsx
@@ -123,7 +123,10 @@ export class TitusCloneServerGroupModal extends React.Component<
   private configureCommand = () => {
     const { command } = this.props;
     TitusReactInjector.titusServerGroupConfigurationService.configureCommand(command).then(() => {
-      command.registry = (command.backingData.credentialsKeyedByAccount[command.credentials] as any).registry;
+      if (!command.credentials.includes('${')) {
+        // so as to not erase registry when account is a spel expression
+        command.registry = ((command.backingData.credentialsKeyedByAccount[command.credentials] as any) || {}).registry;
+      }
       this.setState({ loaded: true, requiresTemplateSelection: false });
     });
   };
@@ -168,7 +171,12 @@ export class TitusCloneServerGroupModal extends React.Component<
             command.credentials !== undefined && (
               <p>
                 Uses target groups from the Amazon account{' '}
-                <AccountTag account={command.backingData.credentialsKeyedByAccount[command.credentials].awsAccount} />
+                <AccountTag
+                  account={
+                    command.backingData.credentialsKeyedByAccount[command.credentials] &&
+                    command.backingData.credentialsKeyedByAccount[command.credentials].awsAccount
+                  }
+                />
               </p>
             )}
         </div>
@@ -180,6 +188,7 @@ export class TitusCloneServerGroupModal extends React.Component<
     const amazonAccount =
       command.backingData &&
       command.backingData.credentialsKeyedByAccount &&
+      command.backingData.credentialsKeyedByAccount[command.credentials] &&
       command.backingData.credentialsKeyedByAccount[command.credentials].awsAccount;
     if (!amazonAccount || command.credentials === undefined) {
       return null;

diff --git a/...scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx b/...scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx
@@ -185,7 +185,12 @@ export class ServerGroupBasicSettings
             {values.credentials !== undefined && (
               <div className="small">
                 Uses resources from the Amazon account{' '}
-                <AccountTag account={values.backingData.credentialsKeyedByAccount[values.credentials].awsAccount} />
+                <AccountTag
+                  account={
+                    values.backingData.credentialsKeyedByAccount[values.credentials] &&
+                    values.backingData.credentialsKeyedByAccount[values.credentials].awsAccount
+                  }
+                />
               </div>
             )}
           </div>

diff --git a/app/scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupParameters.tsx b/app/scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupParameters.tsx
@@ -81,7 +81,13 @@ export class ServerGroupParameters extends React.Component<IServerGroupParameter
             <Field type="text" className="form-control input-sm no-spel" name="iamProfile" />
           </div>
           <div className="col-md-1 small" style={{ whiteSpace: 'nowrap', paddingLeft: '0px', paddingTop: '7px' }}>
-            in <AccountTag account={values.backingData.credentialsKeyedByAccount[values.credentials].awsAccount} />
+            in{' '}
+            <AccountTag
+              account={
+                values.backingData.credentialsKeyedByAccount[values.credentials] &&
+                values.backingData.credentialsKeyedByAccount[values.credentials].awsAccount
+              }
+            />
           </div>
         </div>
         <div className="form-group">