From 7f897d59ee0f8aee720b73f86c6f9366b881c3ab Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 27 Jan 2016 16:33:33 +0000 Subject: [PATCH] Fix so local and remote bootstrap properties can be decrypted The order of initializers is important (decrypt has to come after property sources), as is the order of property sources (the decrypted ones have to be high enough priority to override local and remote bootstrap). Fixes gh-325 --- .../PropertySourceBootstrapConfiguration.java | 168 +++++++++--------- ...ironmentDecryptApplicationInitializer.java | 158 ++++++++-------- ...gCustomPropertySourceIntegrationTests.java | 65 +++++++ .../src/test/resources/custom.properties | 1 + 4 files changed, 229 insertions(+), 163 deletions(-) create mode 100644 spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomPropertySourceIntegrationTests.java create mode 100644 spring-cloud-context/src/test/resources/custom.properties diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java index b85dad2d..45fc054f 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java @@ -26,7 +26,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.bind.PropertySourcesPropertyValues; import org.springframework.boot.bind.RelaxedDataBinder; -import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.logging.LogFile; import org.springframework.boot.logging.LoggingSystem; @@ -36,6 +35,7 @@ import org.springframework.cloud.logging.LoggingRebinder; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; @@ -52,117 +52,117 @@ import org.springframework.util.StringUtils; @Configuration @EnableConfigurationProperties(PropertySourceBootstrapProperties.class) public class PropertySourceBootstrapConfiguration implements - ApplicationContextInitializer { + ApplicationContextInitializer, Ordered { private static final String BOOTSTRAP_PROPERTY_SOURCE_NAME = BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME; private static Log logger = LogFactory - .getLog(PropertySourceBootstrapConfiguration.class); + .getLog(PropertySourceBootstrapConfiguration.class); + + private int order = Ordered.HIGHEST_PRECEDENCE + 10; @Autowired(required = false) private List propertySourceLocators = new ArrayList<>(); - @Autowired - private PropertySourceBootstrapProperties properties; - - @Autowired - private ConfigurationPropertiesBindingPostProcessor binder; - public void setPropertySourceLocators( - Collection propertySourceLocators) { - this.propertySourceLocators = new ArrayList<>(propertySourceLocators); + Collection propertySourceLocators) { + this.propertySourceLocators = new ArrayList<>(propertySourceLocators); + } + + @Override + public int getOrder() { + return this.order; } @Override public void initialize(ConfigurableApplicationContext applicationContext) { - CompositePropertySource composite = new CompositePropertySource( - BOOTSTRAP_PROPERTY_SOURCE_NAME); - AnnotationAwareOrderComparator.sort(this.propertySourceLocators); - boolean empty = true; - ConfigurableEnvironment environment = applicationContext.getEnvironment(); - for (PropertySourceLocator locator : this.propertySourceLocators) { - PropertySource source = null; - source = locator.locate(environment); - if (source == null) { - continue; - } - logger.info("Located property source: " + source); - composite.addPropertySource(source); - empty = false; + CompositePropertySource composite = new CompositePropertySource( + BOOTSTRAP_PROPERTY_SOURCE_NAME); + AnnotationAwareOrderComparator.sort(this.propertySourceLocators); + boolean empty = true; + ConfigurableEnvironment environment = applicationContext.getEnvironment(); + for (PropertySourceLocator locator : this.propertySourceLocators) { + PropertySource source = null; + source = locator.locate(environment); + if (source == null) { + continue; } - if (!empty) { - MutablePropertySources propertySources = environment.getPropertySources(); - String logConfig = environment.resolvePlaceholders("${logging.config:}"); - if (propertySources.contains(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { - propertySources.remove(BOOTSTRAP_PROPERTY_SOURCE_NAME); - } - insertPropertySources(propertySources, composite); - reinitializeLoggingSystem(environment, logConfig); - setLogLevels(environment); + logger.info("Located property source: " + source); + composite.addPropertySource(source); + empty = false; + } + if (!empty) { + MutablePropertySources propertySources = environment.getPropertySources(); + String logConfig = environment.resolvePlaceholders("${logging.config:}"); + if (propertySources.contains(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { + propertySources.remove(BOOTSTRAP_PROPERTY_SOURCE_NAME); } + insertPropertySources(propertySources, composite); + reinitializeLoggingSystem(environment, logConfig); + setLogLevels(environment); + } } private void reinitializeLoggingSystem(ConfigurableEnvironment environment, - String oldLogConfig) { - String logConfig = environment.resolvePlaceholders("${logging.config:}"); - if (StringUtils.hasText(logConfig) && !logConfig.equals(oldLogConfig)) { - LoggingSystem system = LoggingSystem - .get(LoggingSystem.class.getClassLoader()); - try { - ResourceUtils.getURL(logConfig).openStream().close(); - LogFile logFile = LogFile.get(environment); - system.initialize(logConfig, logFile); - } - catch (Exception ex) { - PropertySourceBootstrapConfiguration.logger - .warn("Logging config file location '" + logConfig - + "' cannot be opened and will be ignored"); - } + String oldLogConfig) { + String logConfig = environment.resolvePlaceholders("${logging.config:}"); + if (StringUtils.hasText(logConfig) && !logConfig.equals(oldLogConfig)) { + LoggingSystem system = LoggingSystem + .get(LoggingSystem.class.getClassLoader()); + try { + ResourceUtils.getURL(logConfig).openStream().close(); + LogFile logFile = LogFile.get(environment); + system.initialize(logConfig, logFile); + } + catch (Exception ex) { + PropertySourceBootstrapConfiguration.logger + .warn("Logging config file location '" + logConfig + + "' cannot be opened and will be ignored"); } } + } private void setLogLevels(ConfigurableEnvironment environment) { - LoggingRebinder rebinder = new LoggingRebinder(); - rebinder.setEnvironment(environment); - // We can't fire the event in the ApplicationContext here (too early), but we can - // create our own listener and poke it (it doesn't need the key changes) - rebinder.onApplicationEvent(new EnvironmentChangeEvent(Collections - . emptySet())); + LoggingRebinder rebinder = new LoggingRebinder(); + rebinder.setEnvironment(environment); + // We can't fire the event in the ApplicationContext here (too early), but we can + // create our own listener and poke it (it doesn't need the key changes) + rebinder.onApplicationEvent( + new EnvironmentChangeEvent(Collections. emptySet())); } private void insertPropertySources(MutablePropertySources propertySources, - CompositePropertySource composite) { - MutablePropertySources incoming = new MutablePropertySources(); - incoming.addFirst(composite); - PropertySourceBootstrapProperties remoteProperties = new PropertySourceBootstrapProperties(); - new RelaxedDataBinder(remoteProperties, "spring.cloud.config") - .bind(new PropertySourcesPropertyValues(incoming)); - if (!remoteProperties.isAllowOverride() - || (!remoteProperties.isOverrideNone() && remoteProperties - .isOverrideSystemProperties())) { - propertySources.addFirst(composite); - return; - } - if (remoteProperties.isOverrideNone()) { - propertySources.addLast(composite); - return; - } - if (propertySources - .contains(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { - if (!remoteProperties.isOverrideSystemProperties()) { - propertySources.addAfter( - StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, - composite); - } - else { - propertySources.addBefore( - StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, - composite); - } + CompositePropertySource composite) { + MutablePropertySources incoming = new MutablePropertySources(); + incoming.addFirst(composite); + PropertySourceBootstrapProperties remoteProperties = new PropertySourceBootstrapProperties(); + new RelaxedDataBinder(remoteProperties, "spring.cloud.config") + .bind(new PropertySourcesPropertyValues(incoming)); + if (!remoteProperties.isAllowOverride() || (!remoteProperties.isOverrideNone() + && remoteProperties.isOverrideSystemProperties())) { + propertySources.addFirst(composite); + return; + } + if (remoteProperties.isOverrideNone()) { + propertySources.addLast(composite); + return; + } + if (propertySources + .contains(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { + if (!remoteProperties.isOverrideSystemProperties()) { + propertySources.addAfter( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, + composite); } else { - propertySources.addLast(composite); + propertySources.addBefore( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, + composite); } } + else { + propertySources.addLast(composite); + } + } } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java index 7691c51d..f0b2f440 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java @@ -45,14 +45,14 @@ import org.springframework.security.crypto.encrypt.TextEncryptor; * */ public class EnvironmentDecryptApplicationInitializer implements - ApplicationContextInitializer, Ordered { + ApplicationContextInitializer, Ordered { public static final String DECRYPTED_PROPERTY_SOURCE_NAME = "decrypted"; private int order = Ordered.HIGHEST_PRECEDENCE + 15; private static Log logger = LogFactory - .getLog(EnvironmentDecryptApplicationInitializer.class); + .getLog(EnvironmentDecryptApplicationInitializer.class); private TextEncryptor encryptor; @@ -64,111 +64,111 @@ public class EnvironmentDecryptApplicationInitializer implements * @param failOnError the flag value (default true) */ public void setFailOnError(boolean failOnError) { - this.failOnError = failOnError; + this.failOnError = failOnError; } public EnvironmentDecryptApplicationInitializer(TextEncryptor encryptor) { - this.encryptor = encryptor; + this.encryptor = encryptor; } @Override public int getOrder() { - return this.order; + return this.order; } @Override public void initialize(ConfigurableApplicationContext applicationContext) { - ConfigurableEnvironment environment = applicationContext.getEnvironment(); - MapPropertySource decrypted = new MapPropertySource( - DECRYPTED_PROPERTY_SOURCE_NAME, - decrypt(environment.getPropertySources())); - if (!decrypted.getSource().isEmpty()) { - // We have some decrypted properties - insert(environment.getPropertySources(), decrypted); - ApplicationContext parent = applicationContext.getParent(); - if (parent != null - && (parent.getEnvironment() instanceof ConfigurableEnvironment)) { - ConfigurableEnvironment mutable = (ConfigurableEnvironment) parent - .getEnvironment(); - // The parent is actually the bootstrap context, and it is fully - // initialized, so we can fire an EnvironmentChangeEvent there to rebind - // @ConfigurationProperties, in case they were encrypted. - insert(mutable.getPropertySources(), decrypted); - parent.publishEvent( - new EnvironmentChangeEvent(decrypted.getSource().keySet())); - } + ConfigurableEnvironment environment = applicationContext.getEnvironment(); + MapPropertySource decrypted = new MapPropertySource( + DECRYPTED_PROPERTY_SOURCE_NAME, + decrypt(environment.getPropertySources())); + if (!decrypted.getSource().isEmpty()) { + // We have some decrypted properties + insert(environment.getPropertySources(), decrypted); + ApplicationContext parent = applicationContext.getParent(); + if (parent != null + && (parent.getEnvironment() instanceof ConfigurableEnvironment)) { + ConfigurableEnvironment mutable = (ConfigurableEnvironment) parent + .getEnvironment(); + // The parent is actually the bootstrap context, and it is fully + // initialized, so we can fire an EnvironmentChangeEvent there to rebind + // @ConfigurationProperties, in case they were encrypted. + insert(mutable.getPropertySources(), decrypted); + parent.publishEvent( + new EnvironmentChangeEvent(decrypted.getSource().keySet())); } } + } private void insert(MutablePropertySources propertySources, - MapPropertySource propertySource) { - if (propertySources - .contains(BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME)) { - propertySources.addAfter( - BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME, - propertySource); - } - else { - propertySources.addFirst(propertySource); - } + MapPropertySource propertySource) { + if (propertySources + .contains(BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME)) { + propertySources.addBefore( + BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME, + propertySource); + } + else { + propertySources.addFirst(propertySource); + } } public Map decrypt(PropertySources propertySources) { - Map overrides = new LinkedHashMap(); - List> sources = new ArrayList>(); - for (PropertySource source : propertySources) { - sources.add(0, source); - } - for (PropertySource source : sources) { - decrypt(source, overrides); - } - return overrides; + Map overrides = new LinkedHashMap(); + List> sources = new ArrayList>(); + for (PropertySource source : propertySources) { + sources.add(0, source); + } + for (PropertySource source : sources) { + decrypt(source, overrides); + } + return overrides; } private void decrypt(PropertySource source, Map overrides) { - if (source instanceof EnumerablePropertySource) { - - EnumerablePropertySource enumerable = (EnumerablePropertySource) source; - for (String key : enumerable.getPropertyNames()) { - String value = source.getProperty(key).toString(); - if (value.startsWith("{cipher}")) { - value = value.substring("{cipher}".length()); - try { - value = this.encryptor.decrypt(value); - if (logger.isDebugEnabled()) { - logger.debug("Decrypted: key=" + key); - } - } - catch (Exception e) { - String message = "Cannot decrypt: key=" + key; - if (this.failOnError) { - throw new IllegalStateException(message, e); - } - if (logger.isDebugEnabled()) { - logger.warn(message, e); - } - else { - logger.warn(message); - } - // Set value to empty to avoid making a password out of the - // cipher text - value = ""; - } - overrides.put(key, value); - } + if (source instanceof EnumerablePropertySource) { + + EnumerablePropertySource enumerable = (EnumerablePropertySource) source; + for (String key : enumerable.getPropertyNames()) { + String value = source.getProperty(key).toString(); + if (value.startsWith("{cipher}")) { + value = value.substring("{cipher}".length()); + try { + value = this.encryptor.decrypt(value); + if (logger.isDebugEnabled()) { + logger.debug("Decrypted: key=" + key); } - + } + catch (Exception e) { + String message = "Cannot decrypt: key=" + key; + if (this.failOnError) { + throw new IllegalStateException(message, e); + } + if (logger.isDebugEnabled()) { + logger.warn(message, e); + } + else { + logger.warn(message); + } + // Set value to empty to avoid making a password out of the + // cipher text + value = ""; + } + overrides.put(key, value); + } } - else if (source instanceof CompositePropertySource) { - for (PropertySource nested : ((CompositePropertySource) source) - .getPropertySources()) { - decrypt(nested, overrides); - } + } + else if (source instanceof CompositePropertySource) { + for (PropertySource nested : ((CompositePropertySource) source) + .getPropertySources()) { + decrypt(nested, overrides); } } + } + } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomPropertySourceIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomPropertySourceIntegrationTests.java new file mode 100644 index 00000000..9bc8e28e --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomPropertySourceIntegrationTests.java @@ -0,0 +1,65 @@ +package org.springframework.cloud.bootstrap; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.IntegrationTest; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.bootstrap.BootstrapOrderingCustomPropertySourceIntegrationTests.Application; +import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.PropertySource; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = Application.class) +@IntegrationTest({"encrypt.key:deadbeef", "spring.cloud.bootstrap.name:custom"}) +@ActiveProfiles("encrypt") +public class BootstrapOrderingCustomPropertySourceIntegrationTests { + + @Autowired + private ConfigurableEnvironment environment; + + @Test + public void bootstrapPropertiesExist() { + assertTrue(this.environment.getPropertySources().contains("bootstrap")); + } + + @Test + public void customPropertiesDecrypted() { + assertEquals("bar", this.environment.resolvePlaceholders("${custom.foo}")); + } + + @EnableAutoConfiguration + @Configuration + protected static class Application { + + } + + @Configuration + // This is added to bootstrap context as a source in bootstrap.properties + protected static class PropertySourceConfiguration implements PropertySourceLocator { + + public static Map MAP = new HashMap( + Collections. singletonMap("custom.foo", "{cipher}6154ca04d4bb6144d672c4e3d750b5147116dd381946d51fa44f8bc25dc256f4")); + + @Override + public PropertySource locate(Environment environment) { + return new MapPropertySource("testBootstrap", MAP); + } + + } + +} diff --git a/spring-cloud-context/src/test/resources/custom.properties b/spring-cloud-context/src/test/resources/custom.properties new file mode 100644 index 00000000..1ee3320d --- /dev/null +++ b/spring-cloud-context/src/test/resources/custom.properties @@ -0,0 +1 @@ +spring.main.sources: org.springframework.cloud.bootstrap.BootstrapOrderingCustomPropertySourceIntegrationTests.PropertySourceConfiguration