fix(ecs): set IAMRoleCachingAgent AWS region (#4674)

* fix(ecs): set IAMRoleCachingAgent AWS region Previously, DEFAULT_REGION was always used, breaking for users in China or GovCloud. This change uses a configured region, if present, and logs the choice. * use default unless sample shows otherwise Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Jun 15, 2020 · e585daa · e585daa
1 parent e9f7c3e
commit e585daa
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 2 deletions.
diff --git a/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgent.java b/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgent.java
@@ -46,6 +46,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -83,8 +84,7 @@ public static Map<String, Object> convertIamRoleToAttributes(IamRole iamRole) {
 
   @Override
   public CacheResult loadData(ProviderCache providerCache) {
-    AmazonIdentityManagement iam =
-        amazonClientProvider.getIam(account, Regions.DEFAULT_REGION.getName(), false);
+    AmazonIdentityManagement iam = amazonClientProvider.getIam(account, getIamRegion(), false);
 
     Set<IamRole> cacheableRoles = fetchIamRoles(iam, accountName);
     Map<String, Collection<CacheData>> newDataMap = generateFreshData(cacheableRoles);
@@ -134,6 +134,23 @@ private Map<String, Collection<String>> computeEvictableData(
     return evictionsByKey;
   }
 
+  protected String getIamRegion() {
+    // sample a region from the account in case the default won't work
+    String testRegion =
+        !account.getRegions().isEmpty() && account.getRegions().get(0) != null
+            ? account.getRegions().get(0).getName()
+            : "";
+
+    if (StringUtils.isNotBlank(testRegion)
+        && (testRegion.startsWith("cn-") || testRegion.startsWith("us-gov-"))) {
+      log.debug("retrieving IAM Roles from given region: {}", testRegion);
+      return testRegion;
+    }
+
+    log.debug("retrieving IAM Roles from default region: {}", Regions.DEFAULT_REGION.getName());
+    return Regions.DEFAULT_REGION.getName();
+  }
+
   Map<String, Collection<CacheData>> generateFreshData(Set<IamRole> cacheableRoles) {
     Collection<CacheData> dataPoints = new HashSet<>();
     Map<String, Collection<CacheData>> newDataMap = new HashMap<>();

diff --git a/...st/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgentTest.java b/...st/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgentTest.java
@@ -23,11 +23,13 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import com.amazonaws.regions.Regions;
 import com.amazonaws.services.identitymanagement.AmazonIdentityManagement;
 import com.amazonaws.services.identitymanagement.model.ListRolesRequest;
 import com.amazonaws.services.identitymanagement.model.ListRolesResult;
 import com.amazonaws.services.identitymanagement.model.Role;
 import com.netflix.spinnaker.cats.cache.CacheData;
+import com.netflix.spinnaker.clouddriver.aws.security.AmazonCredentials;
 import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials;
 import com.netflix.spinnaker.clouddriver.ecs.cache.Keys;
 import com.netflix.spinnaker.clouddriver.ecs.cache.model.IamRole;
@@ -152,4 +154,39 @@ public void shouldGenerateFreshData() {
           keys.contains(cacheData.getId()));
     }
   }
+
+  @Test
+  public void shouldGetDefaultRegion() {
+    // given
+    String defaultRegionName = Regions.DEFAULT_REGION.getName();
+    when(netflixAmazonCredentials.getRegions())
+        .thenReturn(Collections.singletonList(new AmazonCredentials.AWSRegion("us-east-1", null)));
+
+    // when
+    String actualRegionName = agent.getIamRegion();
+
+    // then
+    assertEquals(
+        "Expected region to equal " + defaultRegionName + ", but got " + actualRegionName,
+        defaultRegionName,
+        actualRegionName);
+  }
+
+  @Test
+  public void shouldGetConfiguredIamRegion() {
+    // given
+    String expectedRegionName = "cn-north-1";
+    when(netflixAmazonCredentials.getRegions())
+        .thenReturn(
+            Collections.singletonList(new AmazonCredentials.AWSRegion(expectedRegionName, null)));
+
+    // when
+    String actualRegionName = agent.getIamRegion();
+
+    // then
+    assertEquals(
+        "Expected region to equal " + expectedRegionName + ", but got " + actualRegionName,
+        expectedRegionName,
+        actualRegionName);
+  }
 }