refactor(*): Remove credentialAccount from all AWS-derived cloud prov…

…iders (#4017)
spinnaker · Sep 10, 2019 · 7c861e3 · 7c861e3
1 parent 1c1c03b
commit 7c861e3
Show file tree

Hide file tree

Showing 16 changed files with 114 additions and 45 deletions.
diff --git a/.../spinnaker/clouddriver/aws/deploy/description/AbstractAmazonCredentialsDescription.groovy b/.../spinnaker/clouddriver/aws/deploy/description/AbstractAmazonCredentialsDescription.groovy
@@ -17,16 +17,10 @@
 package com.netflix.spinnaker.clouddriver.aws.deploy.description
 
 import com.fasterxml.jackson.annotation.JsonIgnore
-import com.fasterxml.jackson.annotation.JsonProperty
 import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials
 import com.netflix.spinnaker.clouddriver.security.resources.CredentialsNameable
 
 abstract class AbstractAmazonCredentialsDescription implements CredentialsNameable {
   @JsonIgnore
   NetflixAmazonCredentials credentials
-
-  @JsonProperty("credentials")
-  String getCredentialAccount() {
-    this.credentials?.name
-  }
 }
diff --git a/...ovy/com/netflix/spinnaker/clouddriver/aws/deploy/handlers/BasicAmazonDeployHandler.groovy b/...ovy/com/netflix/spinnaker/clouddriver/aws/deploy/handlers/BasicAmazonDeployHandler.groovy
@@ -324,7 +324,7 @@ class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescrip
       deploymentResult.deployments.add(
           new DeploymentResult.Deployment(
               cloudProvider: "aws",
-              account: description.getCredentialAccount(),
+              account: description.getAccount(),
               location: region,
               serverGroupName: asgName,
               capacity: capacity

diff --git a/...er/clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerAtomicOperation.groovy b/...er/clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerAtomicOperation.groovy
@@ -146,7 +146,7 @@ class UpsertAmazonLoadBalancerAtomicOperation implements AtomicOperation<UpsertA
             IngressLoadBalancerBuilder.IngressLoadBalancerGroupResult ingressLoadBalancerResult = ingressLoadBalancerBuilder.ingressApplicationLoadBalancerGroup(
               application,
               region,
-              description.credentialAccount,
+              description.account,
               description.credentials,
               description.vpcId,
               ports,

diff --git a/.../clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerV2AtomicOperation.groovy b/.../clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerV2AtomicOperation.groovy
@@ -164,7 +164,7 @@ class UpsertAmazonLoadBalancerV2AtomicOperation implements AtomicOperation<Upser
         IngressLoadBalancerBuilder.IngressLoadBalancerGroupResult ingressLoadBalancerResult = ingressLoadBalancerBuilder.ingressApplicationLoadBalancerGroup(
           application,
           region,
-          description.credentialAccount,
+          description.account,
           description.credentials,
           description.vpcId,
           ports,

diff --git a/...x/spinnaker/clouddriver/aws/deploy/ops/securitygroup/SecurityGroupIngressConverter.groovy b/...x/spinnaker/clouddriver/aws/deploy/ops/securitygroup/SecurityGroupIngressConverter.groovy
@@ -68,7 +68,7 @@ class SecurityGroupIngressConverter {
       permission
     }
     description.securityGroupIngress.each { ingress ->
-      final accountName = ingress.accountName ?: description.credentialAccount
+      final accountName = ingress.accountName ?: description.account
       final accountId = ingress.accountId ?: securityGroupLookup.getAccountIdForName(accountName)
       final vpcId = ingress.vpcId ?: description.vpcId
       def newUserIdGroupPair = null
@@ -96,7 +96,7 @@ class SecurityGroupIngressConverter {
     }
     new ConvertedIngress(ipPermissions, new MissingSecurityGroups(
       all: missing,
-      selfReferencing: missing.findAll { it.name == description.name && it.accountName == description.credentialAccount }
+      selfReferencing: missing.findAll { it.name == description.name && it.accountName == description.account }
     ))
   }
 

diff --git a/...flix/spinnaker/clouddriver/aws/deploy/ops/securitygroup/SecurityGroupLookupFactory.groovy b/...flix/spinnaker/clouddriver/aws/deploy/ops/securitygroup/SecurityGroupLookupFactory.groovy
@@ -112,7 +112,7 @@ class SecurityGroupLookupFactory {
     }
 
     SecurityGroupUpdater createSecurityGroup(UpsertSecurityGroupDescription description) {
-      final credentials = getCredentialsForName(description.credentialAccount)
+      final credentials = getCredentialsForName(description.account)
       final request = new CreateSecurityGroupRequest(description.name, description.description)
       if (description.vpcId) {
         request.withVpcId(description.vpcId)
@@ -152,7 +152,7 @@ class SecurityGroupLookupFactory {
       }
 
       if (!skipEdda) {
-        getEddaSecurityGroups(amazonEC2, description.credentialAccount, region).add(newSecurityGroup)
+        getEddaSecurityGroups(amazonEC2, description.account, region).add(newSecurityGroup)
       }
       new SecurityGroupUpdater(newSecurityGroup, amazonEC2)
     }

diff --git a/...nnaker/clouddriver/aws/deploy/ops/securitygroup/UpsertSecurityGroupAtomicOperation.groovy b/...nnaker/clouddriver/aws/deploy/ops/securitygroup/UpsertSecurityGroupAtomicOperation.groovy
@@ -55,7 +55,7 @@ class UpsertSecurityGroupAtomicOperation implements AtomicOperation<Void> {
     ConvertedIngress ipPermissionsFromDescription = convertDescriptionToIngress(securityGroupLookup, description, true)
 
     def securityGroupUpdater = securityGroupLookup.getSecurityGroupByName(
-      description.credentialAccount,
+      description.account,
       description.name,
       description.vpcId
     )
@@ -74,14 +74,14 @@ class UpsertSecurityGroupAtomicOperation implements AtomicOperation<Void> {
       } catch (AmazonServiceException e) {
         if (e.errorCode == "InvalidGroup.Duplicate") {
           securityGroupUpdater = securityGroupLookup.getSecurityGroupByName(
-            description.credentialAccount,
+            description.account,
             description.name,
             description.vpcId
           ).get()
           existingIpPermissions = SecurityGroupIngressConverter.
             flattenPermissions(securityGroupUpdater.securityGroup)
         } else {
-          task.updateStatus BASE_PHASE, "Failed to create security group '${description.name}' in ${description.credentialAccount}: ${e.errorMessage}"
+          task.updateStatus BASE_PHASE, "Failed to create security group '${description.name}' in ${description.account}: ${e.errorMessage}"
           throw e
         }
       }
@@ -124,7 +124,7 @@ class UpsertSecurityGroupAtomicOperation implements AtomicOperation<Void> {
 
     if (ipPermissionsFromDescription.missingSecurityGroups.anyMissing(ignoreSelfReferencingRules)) {
       def missingSecurityGroupDescriptions = ipPermissionsFromDescription.missingSecurityGroups.all.collect {
-        "'${it.name ?: it.id}' in '${it.accountName ?: description.credentialAccount}' ${it.vpcId ?: description.vpcId ?: 'EC2-classic'}"
+        "'${it.name ?: it.id}' in '${it.accountName ?: description.account}' ${it.vpcId ?: description.vpcId ?: 'EC2-classic'}"
       }
       def securityGroupsDoNotExistErrorMessage = "The following security groups do not exist: ${missingSecurityGroupDescriptions.join(", ")}"
       task.updateStatus BASE_PHASE, securityGroupsDoNotExistErrorMessage

diff --git a/...ker/clouddriver/aws/deploy/preprocessors/CredentialsAccountNormalizationPreProcessor.java b/...ker/clouddriver/aws/deploy/preprocessors/CredentialsAccountNormalizationPreProcessor.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2019 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.netflix.spinnaker.clouddriver.aws.deploy.preprocessors;
+
+import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperationDescriptionPreProcessor;
+import java.util.Map;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.stereotype.Component;
+
+/** Normalizes the use of `account` vs `credentials`, ensuring that both are always set. */
+@Slf4j
+@Component
+public class CredentialsAccountNormalizationPreProcessor
+    implements AtomicOperationDescriptionPreProcessor {
+  @Override
+  public boolean supports(Class descriptionClass) {
+    return true;
+  }
+
+  @Override
+  public Map process(Map description) {
+    final String account = (String) description.get("account");
+    final String credentials = (String) description.get("credentials");
+
+    if (account != null && credentials != null && !account.equals(credentials)) {
+      log.warn(
+          "Passed 'account' ({}) and 'credentials' ({}), but values are not equal",
+          account,
+          credentials);
+    }
+
+    if (credentials == null && account != null) {
+      description.put("credentials", account);
+    }
+    if (account == null && credentials != null) {
+      description.put("account", credentials);
+    }
+
+    return description;
+  }
+}
diff --git a/...ain/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/AbstractEcsAtomicOperation.java b/...ain/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/AbstractEcsAtomicOperation.java
@@ -16,7 +16,6 @@
 
 package com.netflix.spinnaker.clouddriver.ecs.deploy.ops;
 
-import com.amazonaws.auth.AWSCredentialsProvider;
 import com.amazonaws.services.applicationautoscaling.AWSApplicationAutoScaling;
 import com.amazonaws.services.ecs.AmazonECS;
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider;
@@ -53,15 +52,13 @@ String getCluster(String service, String account) {
   }
 
   AmazonECS getAmazonEcsClient() {
-    AWSCredentialsProvider credentialsProvider = getCredentials().getCredentialsProvider();
     String region = getRegion();
     NetflixAmazonCredentials credentialAccount = description.getCredentials();
 
     return amazonClientProvider.getAmazonEcs(credentialAccount, region, false);
   }
 
   AWSApplicationAutoScaling getAmazonApplicationAutoScalingClient() {
-    AWSCredentialsProvider credentialsProvider = getCredentials().getCredentialsProvider();
     String region = getRegion();
     NetflixAmazonCredentials credentialAccount = description.getCredentials();
 
@@ -73,8 +70,7 @@ protected String getRegion() {
   }
 
   AmazonCredentials getCredentials() {
-    return (AmazonCredentials)
-        accountCredentialsProvider.getCredentials(description.getCredentialAccount());
+    return (AmazonCredentials) accountCredentialsProvider.getCredentials(description.getAccount());
   }
 
   void updateTaskStatus(String status) {

diff --git a/...va/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java b/...va/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java
@@ -16,11 +16,32 @@
 
 package com.netflix.spinnaker.clouddriver.ecs.deploy.ops;
 
-import com.amazonaws.auth.AWSCredentialsProvider;
 import com.amazonaws.services.applicationautoscaling.AWSApplicationAutoScaling;
-import com.amazonaws.services.applicationautoscaling.model.*;
+import com.amazonaws.services.applicationautoscaling.model.DescribeScalableTargetsRequest;
+import com.amazonaws.services.applicationautoscaling.model.DescribeScalableTargetsResult;
+import com.amazonaws.services.applicationautoscaling.model.RegisterScalableTargetRequest;
+import com.amazonaws.services.applicationautoscaling.model.ScalableDimension;
+import com.amazonaws.services.applicationautoscaling.model.ScalableTarget;
+import com.amazonaws.services.applicationautoscaling.model.ServiceNamespace;
 import com.amazonaws.services.ecs.AmazonECS;
-import com.amazonaws.services.ecs.model.*;
+import com.amazonaws.services.ecs.model.AwsVpcConfiguration;
+import com.amazonaws.services.ecs.model.ContainerDefinition;
+import com.amazonaws.services.ecs.model.CreateServiceRequest;
+import com.amazonaws.services.ecs.model.DeploymentConfiguration;
+import com.amazonaws.services.ecs.model.DescribeServicesRequest;
+import com.amazonaws.services.ecs.model.DescribeServicesResult;
+import com.amazonaws.services.ecs.model.KeyValuePair;
+import com.amazonaws.services.ecs.model.LoadBalancer;
+import com.amazonaws.services.ecs.model.LogConfiguration;
+import com.amazonaws.services.ecs.model.NetworkConfiguration;
+import com.amazonaws.services.ecs.model.PortMapping;
+import com.amazonaws.services.ecs.model.RegisterTaskDefinitionRequest;
+import com.amazonaws.services.ecs.model.RegisterTaskDefinitionResult;
+import com.amazonaws.services.ecs.model.RepositoryCredentials;
+import com.amazonaws.services.ecs.model.Service;
+import com.amazonaws.services.ecs.model.ServiceRegistry;
+import com.amazonaws.services.ecs.model.Tag;
+import com.amazonaws.services.ecs.model.TaskDefinition;
 import com.amazonaws.services.elasticloadbalancingv2.AmazonElasticLoadBalancing;
 import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetGroupsRequest;
 import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetGroupsResult;
@@ -45,8 +66,19 @@
 import com.netflix.spinnaker.clouddriver.ecs.services.SubnetSelector;
 import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
-import java.io.*;
-import java.util.*;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
 import java.util.stream.Collectors;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -115,7 +147,7 @@ public DeploymentResult operate(List priorOutputs) {
     if (description.isCopySourceScalingPoliciesAndActions() && sourceTarget != null) {
       updateTaskStatus("Copying scaling policies...");
       ecsCloudMetricService.copyScalingPolicies(
-          description.getCredentialAccount(),
+          description.getAccount(),
           getRegion(),
           service.getServiceName(),
           resourceId,
@@ -388,20 +420,14 @@ private Service createService(
     updateTaskStatus(
         String.format(
             "Creating %s of %s with %s for %s.",
-            desiredCount,
-            newServerGroupName,
-            taskDefinitionArn,
-            description.getCredentialAccount()));
+            desiredCount, newServerGroupName, taskDefinitionArn, description.getAccount()));
 
     Service service = ecs.createService(request).getService();
 
     updateTaskStatus(
         String.format(
             "Done creating %s of %s with %s for %s.",
-            desiredCount,
-            newServerGroupName,
-            taskDefinitionArn,
-            description.getCredentialAccount()));
+            desiredCount, newServerGroupName, taskDefinitionArn, description.getAccount()));
 
     return service;
   }
@@ -703,15 +729,13 @@ private AmazonECS getSourceAmazonEcsClient() {
   }
 
   private AmazonElasticLoadBalancing getAmazonElasticLoadBalancingClient() {
-    AWSCredentialsProvider credentialsProvider = getCredentials().getCredentialsProvider();
     NetflixAmazonCredentials credentialAccount = description.getCredentials();
 
     return amazonClientProvider.getAmazonElasticLoadBalancingV2(
         credentialAccount, getRegion(), false);
   }
 
   private AmazonIdentityManagement getAmazonIdentityManagementClient() {
-    AWSCredentialsProvider credentialsProvider = getCredentials().getCredentialsProvider();
     NetflixAmazonCredentials credentialAccount = description.getCredentials();
 
     return amazonClientProvider.getAmazonIdentityManagement(credentialAccount, getRegion(), false);

diff --git a/.../java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DisableServiceAtomicOperation.java b/.../java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DisableServiceAtomicOperation.java
@@ -45,7 +45,7 @@ private void disableService() {
     AWSApplicationAutoScaling autoScalingClient = getAmazonApplicationAutoScalingClient();
 
     String service = description.getServerGroupName();
-    String account = description.getCredentialAccount();
+    String account = description.getAccount();
     String cluster = getCluster(service, account);
 
     updateTaskStatus(

diff --git a/...n/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/EnableServiceAtomicOperation.java b/...n/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/EnableServiceAtomicOperation.java
@@ -43,7 +43,7 @@ private void enableService() {
     AWSApplicationAutoScaling autoScalingClient = getAmazonApplicationAutoScalingClient();
 
     String service = description.getServerGroupName();
-    String account = description.getCredentialAccount();
+    String account = description.getAccount();
     String cluster = getCluster(service, account);
 
     UpdateServiceRequest request =

diff --git a/...a/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/TerminateInstancesAtomicOperation.java b/...a/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/TerminateInstancesAtomicOperation.java
@@ -37,7 +37,7 @@ public Void operate(List priorOutputs) {
       updateTaskStatus("Terminating instance: " + taskId);
       String clusterArn =
           containerInformationService.getClusterArn(
-              description.getCredentialAccount(), description.getRegion(), taskId);
+              description.getAccount(), description.getRegion(), taskId);
       StopTaskRequest request = new StopTaskRequest().withTask(taskId).withCluster(clusterArn);
       ecs.stopTask(request);
     }

diff --git a/...n/groovy/com/netflix/spinnaker/clouddriver/eureka/deploy/ops/AbstractEurekaSupport.groovy b/...n/groovy/com/netflix/spinnaker/clouddriver/eureka/deploy/ops/AbstractEurekaSupport.groovy
@@ -113,15 +113,15 @@ abstract class AbstractEurekaSupport {
         if (index % eurekaSupportConfigurationProperties.attemptShortCircuitEveryNInstances == 0) {
           try {
             def hasUpInstances = doesCachedClusterContainDiscoveryStatus(
-              clusterProviders, description.credentialAccount, description.region, description.asgName, "UP"
+              clusterProviders, description.account, description.region, description.asgName, "UP"
             )
             if (hasUpInstances.present && !hasUpInstances.get()) {
               // there are no UP instances, we can return early
               task.updateStatus phaseName, "ASG and all instances are '${discoveryStatus.value}', short circuiting."
               break
             }
           } catch (Exception e) {
-            def account = description.credentialAccount
+            def account = description.account
             def region = description.region
             def asgName = description.asgName
             AbstractEurekaSupport.log.error("[$phaseName] - Unable to verify cached discovery status (account: ${account}, region: ${region}, asgName: ${asgName}", e)

diff --git a/...va/com/netflix/spinnaker/clouddriver/lambda/deploy/ops/AbstractLambdaAtomicOperation.java b/...va/com/netflix/spinnaker/clouddriver/lambda/deploy/ops/AbstractLambdaAtomicOperation.java
@@ -61,8 +61,7 @@ protected String getRegion() {
   }
 
   protected AmazonCredentials getCredentials() {
-    return (AmazonCredentials)
-        accountCredentialsProvider.getCredentials(description.getCredentialAccount());
+    return (AmazonCredentials) accountCredentialsProvider.getCredentials(description.getAccount());
   }
 
   void updateTaskStatus(String status) {

diff --git a/...main/groovy/com/netflix/spinnaker/clouddriver/security/resources/CredentialsNameable.java b/...main/groovy/com/netflix/spinnaker/clouddriver/security/resources/CredentialsNameable.java
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.clouddriver.security.resources;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentials;
 
 /**
@@ -25,6 +26,7 @@
 public interface CredentialsNameable extends AccountNameable {
   AccountCredentials getCredentials();
 
+  @JsonProperty("credentials")
   @Override
   default String getAccount() {
     return getCredentials().getName();