From 3056d5f842aa51fded6b8d379486295b855cd830 Mon Sep 17 00:00:00 2001
From: spencergibb <sgibb@pivotal.io>
Date: Thu, 8 Apr 2021 17:39:25 -0400
Subject: [PATCH] Fixes profile ordering of remote vs local.

Previously a local profiles specific file would have more priority than a remote profile specific file. Using a new ConfigData Option, this is now fixed.

Fixes gh-1795
---
 .../client/ConfigServerConfigDataLoader.java  | 42 ++++++---
 .../ConfigServerConfigDataResource.java       |  7 +-
 ...nfigDataCustomizationIntegrationTests.java | 13 ++-
 .../ConfigDataOrderingIntegrationTests.java   | 89 +++++++++++++++++++
 .../resources/config/profilesample-dev.yml    |  1 +
 .../test/resources/config/profilesample.yml   |  1 +
 .../src/test/resources/orderingtest-dev.yml   |  1 +
 .../src/test/resources/orderingtest.yml       |  1 +
 .../NativeEnvironmentRepository.java          |  5 +-
 9 files changed, 142 insertions(+), 18 deletions(-)
 create mode 100644 spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingIntegrationTests.java
 create mode 100644 spring-cloud-config-sample/src/test/resources/config/profilesample-dev.yml
 create mode 100644 spring-cloud-config-sample/src/test/resources/config/profilesample.yml
 create mode 100644 spring-cloud-config-sample/src/test/resources/orderingtest-dev.yml
 create mode 100644 spring-cloud-config-sample/src/test/resources/orderingtest.yml

diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java
index 5136ab08..3d69ab78 100644
--- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java
+++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java
@@ -19,6 +19,7 @@ package org.springframework.cloud.config.client;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -28,6 +29,7 @@ import org.apache.commons.logging.Log;
 
 import org.springframework.boot.context.config.ConfigData;
 import org.springframework.boot.context.config.ConfigData.Option;
+import org.springframework.boot.context.config.ConfigData.Options;
 import org.springframework.boot.context.config.ConfigDataLoader;
 import org.springframework.boot.context.config.ConfigDataLoaderContext;
 import org.springframework.boot.context.properties.bind.Binder;
@@ -62,6 +64,8 @@ public class ConfigServerConfigDataLoader implements ConfigDataLoader<ConfigServ
 	 */
 	public static final String CONFIG_CLIENT_PROPERTYSOURCE_NAME = "configClient";
 
+	private static final EnumSet<Option> ALL_OPTIONS = EnumSet.allOf(Option.class);
+
 	protected final Log logger;
 
 	public ConfigServerConfigDataLoader(Log logger) {
@@ -91,7 +95,7 @@ public class ConfigServerConfigDataLoader implements ConfigDataLoader<ConfigServ
 
 	public ConfigData doLoad(ConfigDataLoaderContext context, ConfigServerConfigDataResource resource) {
 		ConfigClientProperties properties = resource.getProperties();
-		List<PropertySource<?>> composite = new ArrayList<>();
+		List<PropertySource<?>> propertySources = new ArrayList<>();
 		Exception error = null;
 		String errorBody = null;
 		try {
@@ -113,7 +117,7 @@ public class ConfigServerConfigDataLoader implements ConfigDataLoader<ConfigServ
 							@SuppressWarnings("unchecked")
 							Map<String, Object> map = translateOrigins(source.getName(),
 									(Map<String, Object>) source.getSource());
-							composite.add(0,
+							propertySources.add(0,
 									new OriginTrackedMapPropertySource("configserver:" + source.getName(), map));
 						}
 					}
@@ -127,17 +131,31 @@ public class ConfigServerConfigDataLoader implements ConfigDataLoader<ConfigServ
 					}
 					// the existence of this property source confirms a successful
 					// response from config server
-					composite.add(0, new MapPropertySource(CONFIG_CLIENT_PROPERTYSOURCE_NAME, map));
-					try {
-						return new ConfigData(composite, Option.IGNORE_IMPORTS, Option.IGNORE_PROFILES);
+					propertySources.add(0, new MapPropertySource(CONFIG_CLIENT_PROPERTYSOURCE_NAME, map));
+					if (ALL_OPTIONS.size() == 1) {
+						// boot 2.4.2 and prior
+						return new ConfigData(propertySources);
 					}
-					catch (NoSuchFieldError e) {
-						// IGNORE_PROFILES was added in boot 2.4.3, for backwards
-						// compatibility
-						// IGNORE_IMPORTS alone causes NPE prior to 2.4.3
-						// this will still throw an error if spring.profiles.include in
-						// remote config
-						return new ConfigData(composite);
+					else if (ALL_OPTIONS.size() == 2) {
+						// boot 2.4.3 and 2.4.4
+						return new ConfigData(propertySources, Option.IGNORE_IMPORTS, Option.IGNORE_PROFILES);
+					}
+					else if (ALL_OPTIONS.size() > 2) {
+						// boot 2.4.5+
+						return new ConfigData(propertySources, propertySource -> {
+							String propertySourceName = propertySource.getName();
+							List<Option> options = new ArrayList<>();
+							options.add(Option.IGNORE_IMPORTS);
+							options.add(Option.IGNORE_PROFILES);
+							for (String profile : resource.getAcceptedProfiles()) {
+								// TODO: switch to match
+								if (propertySourceName.contains("-" + profile + ".")) {
+									// TODO: switch to Options.with() when implemented
+									options.add(Option.PROFILE_SPECIFIC);
+								}
+							}
+							return Options.of(options.toArray(new Option[0]));
+						});
 					}
 				}
 			}
diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataResource.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataResource.java
index 95a1b9b3..23027197 100644
--- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataResource.java
+++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataResource.java
@@ -47,12 +47,15 @@ public class ConfigServerConfigDataResource extends ConfigDataResource {
 	}
 
 	public String getProfiles() {
-		List<String> accepted = profiles.getAccepted();
 		if (StringUtils.hasText(properties.getProfile())
 				&& !properties.getProfile().equals(ConfigClientProperties.DEFAULT_PROFILE)) {
 			return properties.getProfile();
 		}
-		return StringUtils.collectionToCommaDelimitedString(accepted);
+		return StringUtils.collectionToCommaDelimitedString(getAcceptedProfiles());
+	}
+
+	List<String> getAcceptedProfiles() {
+		return this.profiles.getAccepted();
 	}
 
 	@Override
diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataCustomizationIntegrationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataCustomizationIntegrationTests.java
index 76f03db5..b3904fd2 100644
--- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataCustomizationIntegrationTests.java
+++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataCustomizationIntegrationTests.java
@@ -29,6 +29,7 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.context.config.ConfigData;
 import org.springframework.boot.context.config.ConfigData.Option;
+import org.springframework.boot.context.config.ConfigData.Options;
 import org.springframework.boot.context.properties.bind.BindContext;
 import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.Bindable;
@@ -37,6 +38,7 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyN
 import org.springframework.cloud.config.client.ConfigServerBootstrapper.LoaderInterceptor;
 import org.springframework.cloud.config.environment.Environment;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.core.env.PropertySource;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.ResponseEntity;
@@ -110,9 +112,14 @@ public class ConfigServerConfigDataCustomizationIntegrationTests {
 			hasBinder = context.getBinder() != null;
 			ConfigData configData = context.getInvocation().apply(context.getLoaderContext(), context.getResource());
 			assertThat(configData).as("ConfigData was null for location %s", context.getResource()).isNotNull();
-			assertThat(configData.getOptions()).as("ConfigData.options was null for location %s", context.getResource())
-					.isNotNull();
-			assertThat(configData.getOptions()).contains(Option.IGNORE_IMPORTS, Option.IGNORE_PROFILES);
+			assertThat(configData.getPropertySources()).hasSize(1);
+			PropertySource<?> propertySource = configData.getPropertySources().iterator().next();
+			Options options = configData.getOptions(propertySource);
+			assertThat(options).as("ConfigData.options was null for location %s property source %s",
+					context.getResource(), propertySource.getName()).isNotNull();
+			assertThat(options.contains(Option.IGNORE_IMPORTS)).isTrue();
+			assertThat(options.contains(Option.IGNORE_PROFILES)).isTrue();
+			assertThat(options.contains(Option.PROFILE_SPECIFIC)).isFalse();
 			return configData;
 		}
 
diff --git a/spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingIntegrationTests.java b/spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingIntegrationTests.java
new file mode 100644
index 00000000..4d861988
--- /dev/null
+++ b/spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingIntegrationTests.java
@@ -0,0 +1,89 @@
+/*
+ * Copyright 2018-2019 the original author or authors.
+ *
+ * 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
+ *
+ *      https://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 sample;
+
+import java.util.Map;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointProperties;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.boot.web.server.LocalServerPort;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.util.SocketUtils;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
+
+@RunWith(SpringRunner.class)
+@SpringBootTest(classes = Application.class,
+		// Normally spring.cloud.config.enabled:true is the default but since we have the
+		// config server on the classpath we need to set it explicitly
+		// spring.config.import needs to come from orderingtest.yml to test this issue
+		// hence no spring.config.import here and config name change
+		properties = { "spring.application.name=profilesample", "spring.cloud.config.enabled=true",
+				"spring.config.name=orderingtest", "management.security.enabled=false", "spring.profiles.active=dev",
+				"management.endpoints.web.exposure.include=*" },
+		webEnvironment = RANDOM_PORT)
+public class ConfigDataOrderingIntegrationTests {
+
+	private static final String BASE_PATH = new WebEndpointProperties().getBasePath();
+
+	private static final int configPort = SocketUtils.findAvailableTcpPort();
+
+	private static ConfigurableApplicationContext server;
+
+	@LocalServerPort
+	private int port;
+
+	@BeforeClass
+	public static void startConfigServer() {
+		server = SpringApplication.run(org.springframework.cloud.config.server.ConfigServerApplication.class,
+				"--spring.profiles.active=native", "--server.port=" + configPort, "--spring.config.name=server");
+
+		System.setProperty("spring.cloud.config.uri", "http://localhost:" + configPort);
+	}
+
+	@AfterClass
+	public static void close() {
+		System.clearProperty("spring.cloud.config.uri");
+		if (server != null) {
+			server.close();
+		}
+	}
+
+	@Test
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	public void contextLoads() {
+		ResponseEntity<Map> response = new TestRestTemplate()
+				.getForEntity("http://localhost:" + this.port + BASE_PATH + "/env/my.prop", Map.class);
+		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
+		Map res = response.getBody();
+		assertThat(res).containsKey("propertySources");
+		Map<String, Object> property = (Map<String, Object>) res.get("property");
+		assertThat(property).containsEntry("value", "my value from config server dev profile");
+	}
+
+}
diff --git a/spring-cloud-config-sample/src/test/resources/config/profilesample-dev.yml b/spring-cloud-config-sample/src/test/resources/config/profilesample-dev.yml
new file mode 100644
index 00000000..fec7be31
--- /dev/null
+++ b/spring-cloud-config-sample/src/test/resources/config/profilesample-dev.yml
@@ -0,0 +1 @@
+my.prop: my value from config server dev profile
\ No newline at end of file
diff --git a/spring-cloud-config-sample/src/test/resources/config/profilesample.yml b/spring-cloud-config-sample/src/test/resources/config/profilesample.yml
new file mode 100644
index 00000000..1ca8c881
--- /dev/null
+++ b/spring-cloud-config-sample/src/test/resources/config/profilesample.yml
@@ -0,0 +1 @@
+my.prop: my value from config server default profile
\ No newline at end of file
diff --git a/spring-cloud-config-sample/src/test/resources/orderingtest-dev.yml b/spring-cloud-config-sample/src/test/resources/orderingtest-dev.yml
new file mode 100644
index 00000000..45acf0a8
--- /dev/null
+++ b/spring-cloud-config-sample/src/test/resources/orderingtest-dev.yml
@@ -0,0 +1 @@
+my.prop: my value from local dev profile
\ No newline at end of file
diff --git a/spring-cloud-config-sample/src/test/resources/orderingtest.yml b/spring-cloud-config-sample/src/test/resources/orderingtest.yml
new file mode 100644
index 00000000..54f06dd7
--- /dev/null
+++ b/spring-cloud-config-sample/src/test/resources/orderingtest.yml
@@ -0,0 +1 @@
+spring.config.import: "configserver:"
diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NativeEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NativeEnvironmentRepository.java
index b1255806..2ae00c7b 100644
--- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NativeEnvironmentRepository.java
+++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NativeEnvironmentRepository.java
@@ -235,7 +235,10 @@ public class NativeEnvironmentRepository implements EnvironmentRepository, Searc
 			// TODO: needed anymore?
 			name = name.replace("applicationConfig: [", "");
 			name = name.replace("file [", "file:");
-			name = name.replace("]", "");
+			if (name.indexOf('[') < 0) {
+				// only remove if there isn't a matching left bracket
+				name = name.replace("]", "");
+			}
 			if (this.searchLocations != null) {
 				boolean matches = false;
 				String normal = name;