From c1b8bbea7d834c48813e26a45f0f39f1c1b20240 Mon Sep 17 00:00:00 2001 From: Zach Smith <33258732+zachsmith1@users.noreply.github.com> Date: Mon, 16 Nov 2020 16:04:09 -0800 Subject: [PATCH] fix(cf): changed locationFilter to spaceFilter and added spacesLive to jsonIgnored (#5103) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../CloudFoundryConfigurationProperties.java | 2 +- .../config/CloudFoundryProviderConfig.java | 2 +- .../security/CloudFoundryCredentials.java | 27 +++++++++---------- .../security/CloudFoundryCredentialsTest.java | 22 +++++++-------- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/config/CloudFoundryConfigurationProperties.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/config/CloudFoundryConfigurationProperties.java index 60d538a8061..b2ccb1fd1de 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/config/CloudFoundryConfigurationProperties.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/config/CloudFoundryConfigurationProperties.java @@ -70,6 +70,6 @@ public static class ManagedAccount implements CredentialsDefinition { maxCapiConnectionsForCache; // Deprecated in favor of cloudfoundry.apiRequestParallelism private Permissions.Builder permissions = new Permissions.Builder(); - private Map> locationFilter = Collections.emptyMap(); + private Map> spaceFilter = Collections.emptyMap(); } } diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/config/CloudFoundryProviderConfig.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/config/CloudFoundryProviderConfig.java index 9d4ce5f5cc3..37b29dec56a 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/config/CloudFoundryProviderConfig.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/config/CloudFoundryProviderConfig.java @@ -80,7 +80,7 @@ public AbstractCredentialsLoader cloudFoundryCredential cacheRepository, a.getPermissions().build(), cloudFoundryThreadPool, - a.getLocationFilter()), + a.getSpaceFilter()), cloudFoundryCredentialsRepository); } diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java index 0b98725d36e..abdc88282be 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java @@ -48,7 +48,8 @@ "spaceSupplier", "cacheRepository", "forkJoinPool", - "filteredSpaces" + "filteredSpaces", + "spacesLive" }) public class CloudFoundryCredentials extends AbstractAccountCredentials { private static final int SPACE_EXPIRY_SECONDS = 30; @@ -97,7 +98,7 @@ public CloudFoundryCredentials( CacheRepository cacheRepository, Permissions permissions, ForkJoinPool forkJoinPool, - Map> locationFilter) { + Map> spaceFilter) { this.name = name; this.appsManagerUri = appsManagerUri; this.metricsUri = metricsUri; @@ -110,7 +111,7 @@ public CloudFoundryCredentials( this.cacheRepository = cacheRepository; this.permissions = permissions == null ? Permissions.EMPTY : permissions; this.forkJoinPool = forkJoinPool; - this.filteredSpaces = createFilteredSpaces(locationFilter); + this.filteredSpaces = createFilteredSpaces(spaceFilter); } public CloudFoundryClient getCredentials() { @@ -157,7 +158,7 @@ protected List spaceSupplier() { return getSpacesLive(); } - public List getSpacesLive() { + private List getSpacesLive() { try { return getClient().getSpaces().all(); } catch (CloudFoundryApiException e) { @@ -220,23 +221,23 @@ public static Memoizer memoizeWithExpiration( } } - protected List createFilteredSpaces(Map> locationFilter) { + protected List createFilteredSpaces(Map> spaceFilter) { List spaces = new ArrayList<>(); - if (locationFilter.isEmpty() || locationFilter == null) { + if (spaceFilter.isEmpty() || spaceFilter == null) { return emptyList(); } Set filteredRegions = new HashSet<>(); // IF an Org is provided without spaces -> add all spaces for the ORG - for (String orgName : locationFilter.keySet()) { - if (locationFilter.get(orgName).isEmpty() || locationFilter.get(orgName) == null) { + for (String orgName : spaceFilter.keySet()) { + if (spaceFilter.get(orgName).isEmpty() || spaceFilter.get(orgName) == null) { List allSpacesByOrg = this.getCredentials() .getSpaces() .findAllBySpaceNamesAndOrgNames(null, singletonList(orgName)); spaces.addAll(allSpacesByOrg); } else { - for (String spaceName : locationFilter.get(orgName)) { + for (String spaceName : spaceFilter.get(orgName)) { filteredRegions.add(orgName + " > " + spaceName); } } @@ -246,17 +247,15 @@ protected List createFilteredSpaces(Map> this.getCredentials() .getSpaces() .findAllBySpaceNamesAndOrgNames( - locationFilter.values().stream() - .flatMap(l -> l.stream()) - .collect(Collectors.toList()), - List.copyOf(locationFilter.keySet())); + spaceFilter.values().stream().flatMap(l -> l.stream()).collect(Collectors.toList()), + List.copyOf(spaceFilter.keySet())); allSpaces.stream() .filter(s -> filteredRegions.contains(s.getRegion())) .forEach(s -> spaces.add(s)); if (spaces.isEmpty()) throw new IllegalArgumentException( - "The locationFilter had Orgs and/or Spaces but CloudFoundry returned no spaces as a result. Spaces must not be null or empty when a locationFilter is included."); + "The spaceFilter had Orgs and/or Spaces but CloudFoundry returned no spaces as a result. Spaces must not be null or empty when a spaceFilter is included."); return ImmutableList.copyOf(spaces); } diff --git a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsTest.java b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsTest.java index 5d9c68d3eb3..d0e99e3415d 100644 --- a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsTest.java +++ b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsTest.java @@ -42,7 +42,7 @@ public class CloudFoundryCredentialsTest { private final CloudFoundryClient cloudFoundryClient = new MockCloudFoundryClient(); @Test - void emptyLocationFilterShouldConvertToEmptyList() { + void emptySpaceFilterShouldConvertToEmptyList() { CloudFoundryCredentials credentials = new CloudFoundryCredentials( "test", @@ -63,7 +63,7 @@ void emptyLocationFilterShouldConvertToEmptyList() { } @Test - void singleOrgLocationFilterShouldConvert() { + void singleOrgSpaceFilterShouldConvert() { CloudFoundryCredentials credentials = new CloudFoundryCredentials( "test", @@ -88,7 +88,7 @@ public CloudFoundryClient getCredentials() { } }; - Map> locationFilter = ImmutableMap.of("org", emptySet()); + Map> spaceFilter = ImmutableMap.of("org", emptySet()); CloudFoundryOrganization organization = CloudFoundryOrganization.builder().id("org123").name("org").build(); @@ -107,12 +107,12 @@ public CloudFoundryClient getCredentials() { when(cloudFoundryClient.getSpaces().findAllBySpaceNamesAndOrgNames(isNull(), any())) .thenReturn(List.of(space1, space2)); - List result = credentials.createFilteredSpaces(locationFilter); + List result = credentials.createFilteredSpaces(spaceFilter); assertThat(result).isEqualTo(List.of(space1, space2)); } @Test - void singleOrgSingleSpaceLocationFilterShouldConvert() { + void singleOrgSingleSpaceSpaceFilterShouldConvert() { CloudFoundryCredentials credentials = new CloudFoundryCredentials( "test", @@ -137,7 +137,7 @@ public CloudFoundryClient getCredentials() { } }; - Map> locationFilter = ImmutableMap.of("org", Set.of("space1")); + Map> spaceFilter = ImmutableMap.of("org", Set.of("space1")); CloudFoundryOrganization organization = CloudFoundryOrganization.builder().id("org123").name("org").build(); @@ -156,12 +156,12 @@ public CloudFoundryClient getCredentials() { when(cloudFoundryClient.getSpaces().findAllBySpaceNamesAndOrgNames(any(), any())) .thenReturn(List.of(space1, space2)); - List result = credentials.createFilteredSpaces(locationFilter); + List result = credentials.createFilteredSpaces(spaceFilter); assertThat(result).isEqualTo(List.of(space1)); } @Test - void fakeOrgFakeSpaceLocationFilterShouldThrowError() { + void fakeOrgFakeSpaceSpaceFilterShouldThrowError() { CloudFoundryCredentials credentials = new CloudFoundryCredentials( "test", @@ -186,14 +186,14 @@ public CloudFoundryClient getCredentials() { } }; - Map> locationFilter = ImmutableMap.of("org", Set.of("space1")); + Map> spaceFilter = ImmutableMap.of("org", Set.of("space1")); when(cloudFoundryClient.getSpaces().findAllBySpaceNamesAndOrgNames(any(), any())) .thenReturn(emptyList()); Exception e = - assertThrows(Exception.class, () -> credentials.createFilteredSpaces(locationFilter)); + assertThrows(Exception.class, () -> credentials.createFilteredSpaces(spaceFilter)); assertThat(e) .hasMessageContaining( - "The locationFilter had Orgs and/or Spaces but CloudFoundry returned no spaces as a result. Spaces must not be null or empty when a locationFilter is included."); + "The spaceFilter had Orgs and/or Spaces but CloudFoundry returned no spaces as a result. Spaces must not be null or empty when a spaceFilter is included."); } }