ryanjbaxter

* Handles multiple application names in CredhubEnvironmentRepository Signed-off-by: kvmw <mshamsi@broadcom.com> * Always add the main credhub PropertySource, even if it is empty Signed-off-by: kvmw <mshamsi@broadcom.com> --------- Signed-off-by: kvmw <mshamsi@broadcom.com>
2024-08-13 18:01:51 +02:00
parent b880e0009e
commit eaa06aa84e
2 changed files with 179 additions and 91 deletions
--- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CredhubEnvironmentRepository.java
+++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CredhubEnvironmentRepository.java
@@ -18,6 +18,7 @@ package org.springframework.cloud.config.server.environment;

 import java.util.Arrays;
 import java.util.Map;
+import java.util.stream.Collectors;

 import org.springframework.cloud.config.environment.Environment;
 import org.springframework.cloud.config.environment.PropertySource;
@@ -37,8 +38,6 @@ import static java.util.stream.Collectors.toMap;
 */
 public class CredhubEnvironmentRepository implements EnvironmentRepository, Ordered {

-	private CredHubOperations credHubOperations;
-
 	private static final String DEFAULT_PROFILE = "default";

 	private static final String DEFAULT_LABEL = "master";
@@ -47,44 +46,52 @@ public class CredhubEnvironmentRepository implements EnvironmentRepository, Orde

 	private int order = Ordered.LOWEST_PRECEDENCE;

+	private final CredHubOperations credHubOperations;
+
 	public CredhubEnvironmentRepository(CredHubOperations credHubOperations) {
 		this.credHubOperations = credHubOperations;
 	}

 	@Override
-	public Environment findOne(String application, String profilesList, String label) {
-		if (ObjectUtils.isEmpty(profilesList)) {
-			profilesList = DEFAULT_PROFILE;
+	public Environment findOne(String application, String profile, String label) {
+		if (ObjectUtils.isEmpty(profile)) {
+			profile = DEFAULT_PROFILE;
 		}
 		if (ObjectUtils.isEmpty(label)) {
 			label = DEFAULT_LABEL;
 		}

-		String[] profiles = StringUtils.commaDelimitedListToStringArray(profilesList);
+		String[] applications = deDuplicateAndAddDefault(application, DEFAULT_APPLICATION);
+		String[] profiles = deDuplicateAndAddDefault(profile, DEFAULT_PROFILE);

 		Environment environment = new Environment(application, profiles, label, null, null);
-		for (String profile : profiles) {
-			environment.add(new PropertySource("credhub-" + application + "-" + profile + "-" + label,
-					findProperties(application, profile, label)));
-			if (!DEFAULT_APPLICATION.equals(application)) {
-				addDefaultPropertySource(environment, DEFAULT_APPLICATION, profile, label);
+		for (String prof : profiles) {
+			for (String app : applications) {
+				addPropertySource(environment, app, prof, label);
 			}
 		}

-		if (!Arrays.asList(profiles).contains(DEFAULT_PROFILE)) {
-			addDefaultPropertySource(environment, application, DEFAULT_PROFILE, label);
-		}
-
-		if (!Arrays.asList(profiles).contains(DEFAULT_PROFILE) && !DEFAULT_APPLICATION.equals(application)) {
-			addDefaultPropertySource(environment, DEFAULT_APPLICATION, DEFAULT_PROFILE, label);
-		}
-
 		return environment;
 	}

-	private void addDefaultPropertySource(Environment environment, String application, String profile, String label) {
+	/**
+	 * Converts the comma delimited items to a List and then: - Removes duplicates and
+	 * keeps unique items only. - Moves or Adds the given default item to the end of List.
+	 */
+	private String[] deDuplicateAndAddDefault(String commaDelimitedItems, String defaultItem) {
+		var items = Arrays.stream(StringUtils.commaDelimitedListToStringArray(commaDelimitedItems))
+			.distinct()
+			.filter(item -> !defaultItem.equals(item))
+			.collect(Collectors.toList());
+
+		items.add(defaultItem);
+		return items.toArray(new String[0]);
+	}
+
+	private void addPropertySource(Environment environment, String application, String profile, String label) {
 		Map<Object, Object> properties = findProperties(application, profile, label);
-		if (!properties.isEmpty()) {
+		// The main PropertySource (the first one) should be always there, even if it is empty.
+		if (!properties.isEmpty() || environment.getPropertySources().isEmpty()) {
 			PropertySource propertySource = new PropertySource("credhub-" + application + "-" + profile + "-" + label,
 					properties);
 			environment.add(propertySource);
--- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CredhubEnvironmentRepositoryTests.java
+++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CredhubEnvironmentRepositoryTests.java
@@ -58,77 +58,94 @@ public class CredhubEnvironmentRepositoryTests {

 	@Test
 	public void shouldDisplayEmptyPropertiesWhenNoPathFound() {
-		when(this.credhubCredentialOperations.findByPath("/my-application/production/mylabel")).thenReturn(emptyList());
+		when(this.credhubCredentialOperations.findByPath("/myApp/prod/myLabel")).thenReturn(emptyList());

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production", "mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", "prod", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).containsExactly("production");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("myApp");
+		assertThat(environment.getProfiles()).containsExactly("prod", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

 		assertThat(environment.getPropertySources()).hasSize(1);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-prod-myLabel");
 		assertThat(environment.getPropertySources().get(0).getSource()).isEmpty();
 	}

 	@Test
 	public void shouldRetrieveDefaultsWhenNoLabelNorProfileProvided() {
-		stubCredentials("/my-application/default/master", "toggles", "key1", "value1");
+		stubCredentials("/myApp/default/master", "toggles", "key1", "value1");

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", null, null);
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", null, null);

-		assertThat(environment.getName()).isEqualTo("my-application");
+		assertThat(environment.getName()).isEqualTo("myApp");
 		assertThat(environment.getProfiles()).containsExactly("default");
 		assertThat(environment.getLabel()).isEqualTo("master");

 		assertThat(environment.getPropertySources()).hasSize(1);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-default-master");
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-default-master");
 		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
 	}

 	@Test
 	public void shouldRetrieveGivenProfileAndLabel() {
-		stubCredentials("/my-application/production/mylabel", "toggles", "key1", "value1");
+		stubCredentials("/myApp/prod/myLabel", "toggles", "key1", "value1");

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production", "mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", "prod", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).containsExactly("production");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("myApp");
+		assertThat(environment.getProfiles()).containsExactly("prod", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

 		assertThat(environment.getPropertySources()).hasSize(1);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-prod-myLabel");
 		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
 	}

 	@Test
 	public void shouldRetrieveGivenMultipleProfiles() {
-		stubCredentials("/my-application/production/mylabel", "toggles", "key1", "value1");
-		stubCredentials("/my-application/cloud/mylabel", "abs", "key2", "value2");
+		stubCredentials("/myApp/prod/myLabel", "toggles", "key1", "value1");
+		stubCredentials("/myApp/cloud/myLabel", "abs", "key2", "value2");

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production,cloud",
-				"mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", "prod,cloud", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).containsExactly("production", "cloud");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("myApp");
+		assertThat(environment.getProfiles()).containsExactly("prod", "cloud", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

 		assertThat(environment.getPropertySources()).hasSize(2);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
-		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
-		assertThat(environment.getPropertySources().get(1).getName())

-			.isEqualTo("credhub-my-application-cloud-mylabel");
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-prod-myLabel");
+		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
+
+		assertThat(environment.getPropertySources().get(1).getName()).isEqualTo("credhub-myApp-cloud-myLabel");
+		assertThat(environment.getPropertySources().get(1).getSource()).isEqualTo(singletonMap("key2", "value2"));
+	}
+
+	@Test
+	public void shouldRetrieveGivenMultipleApplicationNames() {
+		stubCredentials("/app1/default/myLabel", "toggles", "key1", "value1");
+		stubCredentials("/app2/default/myLabel", "abs", "key2", "value2");
+
+		Environment environment = this.credhubEnvironmentRepository.findOne("app1,app2", null, "myLabel");
+
+		assertThat(environment.getName()).isEqualTo("app1,app2");
+		assertThat(environment.getProfiles()).containsExactly("default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");
+
+		assertThat(environment.getPropertySources()).hasSize(2);
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-app1-default-myLabel");
+		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
+
+		assertThat(environment.getPropertySources().get(1).getName()).isEqualTo("credhub-app2-default-myLabel");
 		assertThat(environment.getPropertySources().get(1).getSource()).isEqualTo(singletonMap("key2", "value2"));
 	}

 	@Test
 	public void shouldMergeWhenMoreThanOneCredentialsFound() {
-		String expectedPath = "/my-application/production/mylabel";
+		String expectedPath = "/myApp/prod/myLabel";

 		SimpleCredentialName togglesCredentialName = new SimpleCredentialName(expectedPath + "/toggles");
 		SimpleCredentialName absCredentialName = new SimpleCredentialName(expectedPath + "/abs");
@@ -144,15 +161,14 @@ public class CredhubEnvironmentRepositoryTests {
 		when(this.credhubCredentialOperations.getByName(absCredentialName, JsonCredential.class))
 			.thenReturn(new CredentialDetails<>("id2", absCredentialName, CredentialType.JSON, otherCredentials));

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production", "mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", "prod", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).containsExactly("production");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("myApp");
+		assertThat(environment.getProfiles()).containsExactly("prod", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

 		assertThat(environment.getPropertySources()).hasSize(1);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-prod-myLabel");
 		HashMap<Object, Object> expectedValues = new HashMap<>();
 		expectedValues.put("key1", "value1");
 		expectedValues.put("key2", "value2");
@@ -161,49 +177,114 @@ public class CredhubEnvironmentRepositoryTests {

 	@Test
 	public void shouldIncludeDefaultApplicationWhenOtherProvided() {
-		stubCredentials("/my-application/production/mylabel", "toggles", "key1", "value1");
-		stubCredentials("/application/production/mylabel", "abs", "key2", "value2");
+		stubCredentials("/app1/prod/myLabel", "toggles", "app1-prod", "value1");
+		stubCredentials("/app2/prod/myLabel", "toggles", "app2-prod", "value2");
+		stubCredentials("/application/prod/myLabel", "abs", "application-prod", "value3");
+		stubCredentials("/app1/default/myLabel", "toggles", "app1-default", "value4");
+		stubCredentials("/app2/default/myLabel", "toggles", "app2-default", "value5");
+		stubCredentials("/application/default/myLabel", "abs", "application-default", "value6");

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production", "mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("app1,app2", "prod", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).containsExactly("production");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("app1,app2");
+		assertThat(environment.getProfiles()).containsExactly("prod", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

-		assertThat(environment.getPropertySources()).hasSize(2);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
-		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
-		assertThat(environment.getPropertySources().get(1).getName())
-			.isEqualTo("credhub-application-production-mylabel");
-		assertThat(environment.getPropertySources().get(1).getSource()).isEqualTo(singletonMap("key2", "value2"));
+		assertThat(environment.getPropertySources()).hasSize(6);
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-app1-prod-myLabel");
+		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("app1-prod", "value1"));
+
+		assertThat(environment.getPropertySources().get(1).getName()).isEqualTo("credhub-app2-prod-myLabel");
+		assertThat(environment.getPropertySources().get(1).getSource()).isEqualTo(singletonMap("app2-prod", "value2"));
+
+		assertThat(environment.getPropertySources().get(2).getName()).isEqualTo("credhub-application-prod-myLabel");
+		assertThat(environment.getPropertySources().get(2).getSource())
+			.isEqualTo(singletonMap("application-prod", "value3"));
+
+		assertThat(environment.getPropertySources().get(3).getName()).isEqualTo("credhub-app1-default-myLabel");
+		assertThat(environment.getPropertySources().get(3).getSource())
+			.isEqualTo(singletonMap("app1-default", "value4"));
+
+		assertThat(environment.getPropertySources().get(4).getName()).isEqualTo("credhub-app2-default-myLabel");
+		assertThat(environment.getPropertySources().get(4).getSource())
+			.isEqualTo(singletonMap("app2-default", "value5"));
+
+		assertThat(environment.getPropertySources().get(5).getName()).isEqualTo("credhub-application-default-myLabel");
+		assertThat(environment.getPropertySources().get(5).getSource())
+			.isEqualTo(singletonMap("application-default", "value6"));
 	}

 	@Test
 	public void shouldIncludeDefaultProfileWhenOtherProvided() {
-		stubCredentials("/my-application/production/mylabel", "toggles", "key1", "value1");
-		stubCredentials("/application/production/mylabel", "abs", "key2", "value2");
-		stubCredentials("/my-application/default/mylabel", "abs", "key3", "value3");
-		stubCredentials("/application/default/mylabel", "abs", "key4", "value4");
+		stubCredentials("/myApp/dev/myLabel", "toggles", "myApp-dev", "value1");
+		stubCredentials("/application/dev/myLabel", "abs", "application-dev", "value2");
+		stubCredentials("/myApp/prod/myLabel", "toggles", "myApp-prod", "value3");
+		stubCredentials("/application/prod/myLabel", "abs", "application-prod", "value4");
+		stubCredentials("/myApp/default/myLabel", "abs", "myApp-default", "value5");
+		stubCredentials("/application/default/myLabel", "abs", "application-default", "value6");

-		Environment environment = this.credhubEnvironmentRepository.findOne("my-application", "production", "mylabel");
+		Environment environment = this.credhubEnvironmentRepository.findOne("myApp", "dev,prod", "myLabel");

-		assertThat(environment.getName()).isEqualTo("my-application");
-		assertThat(environment.getProfiles()).contains("production");
-		assertThat(environment.getLabel()).isEqualTo("mylabel");
+		assertThat(environment.getName()).isEqualTo("myApp");
+		assertThat(environment.getProfiles()).containsExactly("dev", "prod", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");
+
+		assertThat(environment.getPropertySources()).hasSize(6);
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-dev-myLabel");
+		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("myApp-dev", "value1"));
+
+		assertThat(environment.getPropertySources().get(1).getName()).isEqualTo("credhub-application-dev-myLabel");
+		assertThat(environment.getPropertySources().get(1).getSource())
+			.isEqualTo(singletonMap("application-dev", "value2"));
+
+		assertThat(environment.getPropertySources().get(2).getName()).isEqualTo("credhub-myApp-prod-myLabel");
+		assertThat(environment.getPropertySources().get(2).getSource()).isEqualTo(singletonMap("myApp-prod", "value3"));
+
+		assertThat(environment.getPropertySources().get(3).getName()).isEqualTo("credhub-application-prod-myLabel");
+		assertThat(environment.getPropertySources().get(3).getSource())
+			.isEqualTo(singletonMap("application-prod", "value4"));
+
+		assertThat(environment.getPropertySources().get(4).getName()).isEqualTo("credhub-myApp-default-myLabel");
+		assertThat(environment.getPropertySources().get(4).getSource())
+			.isEqualTo(singletonMap("myApp-default", "value5"));
+
+		assertThat(environment.getPropertySources().get(5).getName()).isEqualTo("credhub-application-default-myLabel");
+		assertThat(environment.getPropertySources().get(5).getSource())
+			.isEqualTo(singletonMap("application-default", "value6"));
+	}
+
+	@Test
+	public void shouldIncludeDefaultProfileAndApplicationNameAtTheEnd() {
+		stubCredentials("/myApp/dev/myLabel", "toggles", "myApp-dev", "value1");
+		stubCredentials("/application/dev/myLabel", "abs", "application-dev", "value2");
+		stubCredentials("/myApp/default/myLabel", "abs", "myApp-default", "value3");
+		stubCredentials("/application/default/myLabel", "abs", "application-default", "value4");
+
+		Environment environment = this.credhubEnvironmentRepository.findOne("application,myApp", "default,dev",
+				"myLabel");
+
+		assertThat(environment.getName()).isEqualTo("application,myApp");
+		assertThat(environment.getProfiles()).containsExactly("dev", "default");
+		assertThat(environment.getLabel()).isEqualTo("myLabel");

 		assertThat(environment.getPropertySources()).hasSize(4);
-		assertThat(environment.getPropertySources().get(0).getName())
-			.isEqualTo("credhub-my-application-production-mylabel");
-		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("key1", "value1"));
-		assertThat(environment.getPropertySources().get(1).getName())
-			.isEqualTo("credhub-application-production-mylabel");
-		assertThat(environment.getPropertySources().get(1).getSource()).isEqualTo(singletonMap("key2", "value2"));
-		assertThat(environment.getPropertySources().get(2).getName())
-			.isEqualTo("credhub-my-application-default-mylabel");
-		assertThat(environment.getPropertySources().get(2).getSource()).isEqualTo(singletonMap("key3", "value3"));
-		assertThat(environment.getPropertySources().get(3).getName()).isEqualTo("credhub-application-default-mylabel");
-		assertThat(environment.getPropertySources().get(3).getSource()).isEqualTo(singletonMap("key4", "value4"));
+
+		assertThat(environment.getPropertySources().get(0).getName()).isEqualTo("credhub-myApp-dev-myLabel");
+		assertThat(environment.getPropertySources().get(0).getSource()).isEqualTo(singletonMap("myApp-dev", "value1"));
+
+		assertThat(environment.getPropertySources().get(1).getName()).isEqualTo("credhub-application-dev-myLabel");
+		assertThat(environment.getPropertySources().get(1).getSource())
+			.isEqualTo(singletonMap("application-dev", "value2"));
+
+		assertThat(environment.getPropertySources().get(2).getName()).isEqualTo("credhub-myApp-default-myLabel");
+		assertThat(environment.getPropertySources().get(2).getSource())
+			.isEqualTo(singletonMap("myApp-default", "value3"));
+
+		assertThat(environment.getPropertySources().get(3).getName()).isEqualTo("credhub-application-default-myLabel");
+		assertThat(environment.getPropertySources().get(3).getSource())
+			.isEqualTo(singletonMap("application-default", "value4"));
 	}

 	private void stubCredentials(String expectedPath, String name, String key, String value) {