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
This commit is contained in:
spencergibb
2021-04-08 17:39:25 -04:00
parent e07a7e8fad
commit 3056d5f842
9 changed files with 142 additions and 18 deletions

View File

@@ -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]));
});
}
}
}

View File

@@ -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

View File

@@ -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;
}

View File

@@ -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");
}
}

View File

@@ -0,0 +1 @@
my.prop: my value from config server dev profile

View File

@@ -0,0 +1 @@
my.prop: my value from config server default profile

View File

@@ -0,0 +1 @@
my.prop: my value from local dev profile

View File

@@ -0,0 +1 @@
spring.config.import: "configserver:"

View File

@@ -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;