fix(ecs): Correctly scope TargetHealthCachingAgent describe calls
MODIFY
clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TargetHealthCachingAgent.java
clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TargetHealthCachingAgent.java
@@ -72,6 +72,7 @@ public class TargetHealthCachingAgent extends AbstractEcsAwsAwareCachingAgent<Ec
EcsTargetGroupCacheClient targetGroupCacheClient =
new EcsTargetGroupCacheClient(awsProviderCache, objectMapper);
Set<String> targetGroups = fetchTargetGroups(targetGroupCacheClient);
+ log.debug("Fetched {} target groups for which to get target healths", targetGroups.size());
AmazonElasticLoadBalancing amazonLoadBalancing =
amazonClientProvider.getAmazonElasticLoadBalancingV2(account, region, false);
@@ -114,7 +115,15 @@ public class TargetHealthCachingAgent extends AbstractEcsAwsAwareCachingAgent<Ec
Collection<String> targetGroupKeys =
awsProviderCache.filterIdentifiers(TARGET_GROUPS.getNs(), searchKey);
- List<EcsTargetGroup> tgList = cacheClient.find(targetGroupKeys);
+ if (targetGroupKeys.size() > 0) {
+ log.debug("Found " + targetGroupKeys.size() + " target group keys in " + getAgentType());
+ }
+
+ // TODO: refine search key instead of filtering these cache results
+ List<EcsTargetGroup> tgList =
+ cacheClient.find(targetGroupKeys).stream()
+ .filter(t -> t.getTargetGroupArn().contains(account.getAccountId()))
+ .collect(Collectors.toList());
return tgList.stream().map(EcsTargetGroup::getTargetGroupArn).collect(Collectors.toSet());
}

MODIFY
clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TargetHealthCachingAgentSpec.groovy
clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TargetHealthCachingAgentSpec.groovy
@@ -41,7 +41,7 @@ class TargetHealthCachingAgentSpec extends Specification {
def awsProviderCache = Mock(ProviderCache)
def credentialsProvider = Mock(AWSCredentialsProvider)
def amazonloadBalancing = Mock(AmazonElasticLoadBalancing)
- def targetGroupArn = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':123456789012:targetgroup/test-tg/9e8997b7cff00c62'
+ def targetGroupArn = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':' + CommonCachingAgent.ACCOUNT_ID + ':targetgroup/test-tg/9e8997b7cff00c62'
ObjectMapper mapper = new ObjectMapper()
@Subject
@@ -140,4 +140,61 @@ class TargetHealthCachingAgentSpec extends Specification {
targetHealthList.size() == 0
}
+
+ def 'should describe and return target groups for matching account only'() {
+ given:
+ // set up correct account cache data
+ def targetGroupAttributes = [
+ loadBalancerNames: ['loadBalancerName'],
+ targetGroupArn: targetGroupArn,
+ targetGroupName: 'test-tg',
+ vpcId: 'vpc-id',
+ ]
+ def targetGroupKey =
+ Keys.getTargetGroupKey('test-tg', CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, 'ip', 'vpc-id')
+ def loadbalancerKey =
+ Keys.getLoadBalancerKey('loadBalancerName', CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, 'vpc-id', 'ip')
+ def relations = [loadBalancers: [loadbalancerKey]]
+ def targetGroupCacheData =
+ new DefaultCacheData(targetGroupKey, targetGroupAttributes, relations)
+
+ // set up other account cache data
+ def targetGroupArn2 = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':210987654321:targetgroup/other-tg/7uf491b7cff00c62'
+ def otherAccountName = "other-account"
+ def targetGroupAttributes2 = [
+ loadBalancerNames: ['loadBalancerName'],
+ targetGroupArn: targetGroupArn2,
+ targetGroupName: 'other-tg',
+ vpcId: 'vpc-id',
+ ]
+ def targetGroupKey2 =
+ Keys.getTargetGroupKey('test-tg', otherAccountName, CommonCachingAgent.REGION, 'ip', 'vpc-id')
+ def loadbalancerKey2 =
+ Keys.getLoadBalancerKey('loadBalancerName', otherAccountName, CommonCachingAgent.REGION, 'vpc-id', 'ip')
+ def relations2 = [loadBalancers: [loadbalancerKey2]]
+ def targetGroupCacheData2 =
+ new DefaultCacheData(targetGroupKey2, targetGroupAttributes2, relations2)
+
+ TargetHealthDescription targetHealth =
+ new TargetHealthDescription().withTarget(
+ new TargetDescription().withId('10.0.0.3').withPort(80))
+ .withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
+
+ // return cache data for both target groups
+ awsProviderCache.getAll(TARGET_GROUPS.getNs(), _, _) >> [targetGroupCacheData, targetGroupCacheData2]
+
+ when:
+ agent.setAwsCache(awsProviderCache)
+ def targetHealthList = agent.getItems(ecs, Mock(ProviderCache))
+
+ then:
+ // expect one describe call, with correct target group
+ 1 * amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
+ request.targetGroupArn == targetGroupArn
+ }) >> new DescribeTargetHealthResult().withTargetHealthDescriptions(targetHealth)
+
+ targetHealthList.size() == 1
+ EcsTargetHealth targetHealthDescription = targetHealthList.get(0)
+ targetHealthDescription.getTargetGroupArn() == targetGroupArn
+ }
}

MODIFY
clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskHealthCachingAgentSpec.groovy
clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskHealthCachingAgentSpec.groovy
@@ -53,7 +53,7 @@ class TaskHealthCachingAgentSpec extends Specification {
def clientProvider = Mock(AmazonClientProvider)
def providerCache = Mock(ProviderCache)
def credentialsProvider = Mock(AWSCredentialsProvider)
- def targetGroupArn = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':769716316905:targetgroup/test-target-group/9e8997b7cff00c62'
+ def targetGroupArn = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':' + CommonCachingAgent.ACCOUNT_ID + ':targetgroup/test-target-group/9e8997b7cff00c62'
ObjectMapper mapper = new ObjectMapper()

MODIFY
clouddriver-ecs/src/test/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/CommonCachingAgent.java
clouddriver-ecs/src/test/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/CommonCachingAgent.java
@@ -32,7 +32,8 @@ import org.junit.BeforeClass;
public class CommonCachingAgent {
static final String REGION = "us-west-2";
- private static final String ECS_SERIVCE = "arn:aws:ecs:" + REGION + ":012345678910:";
+ static final String ACCOUNT_ID = "012345678910";
+ private static final String ECS_SERIVCE = "arn:aws:ecs:" + REGION + ":" + ACCOUNT_ID + ":";
static final String ACCOUNT = "test-account";
static final String APP_NAME = "testapp";
static final String ROLE_ARN = ECS_SERIVCE + "service/test-role";
@@ -77,6 +78,7 @@ public class CommonCachingAgent {
static {
netflixAmazonCredentials = mock(NetflixAmazonCredentials.class);
when(netflixAmazonCredentials.getName()).thenReturn(ACCOUNT);
+ when(netflixAmazonCredentials.getAccountId()).thenReturn(ACCOUNT_ID);
}
@BeforeClass

fix(ecs): Correctly scope TargetHealthCachingAgent describe calls (#4429)