Improve handling of environment variables in failure analysis

Prior to this change, the failure analysis for an invalid
configuration property value filtered out the configuration property
sources property source. This property source contains a "duplicate"
of all of the environment's other property sources but with
configuration property support (such as relaxed/fuzzy matching of
environment variables). This was done to prevent the reporting of
duplicates when a property was found in both the configuration
property sources property source and the "normal" property sources.
An unwanted side-effect of this was that fuzzy matching of
environment variables was lost so the origin of
com.example.some-property would be found in the environment variable
was COM_EXAMPLE_SOME_PROPERTY but would not be found if it was
COM_EXAMPLE_SOMEPROPERTY.

This commit addresses this side-effect by no longer filtering out
the configuration property sources property source. To then
prevent duplicates from being reported in the analysis, it instead
deduplicates things based on the origin of each property that's
found in the environment's property sources.

Fixes gh-43380
This commit is contained in:
Andy Wilkinson
2024-12-04 13:21:29 +00:00
parent 3f346d46a2
commit 7c7bb531b4
2 changed files with 87 additions and 17 deletions

View File

@@ -16,16 +16,18 @@
package org.springframework.boot.diagnostics.analyzer;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException;
import org.springframework.boot.diagnostics.AbstractFailureAnalyzer;
import org.springframework.boot.diagnostics.FailureAnalysis;
import org.springframework.boot.diagnostics.FailureAnalyzer;
import org.springframework.boot.origin.Origin;
import org.springframework.boot.origin.OriginLookup;
import org.springframework.boot.origin.PropertySourceOrigin;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.Environment;
import org.springframework.core.env.PropertySource;
@@ -61,18 +63,23 @@ class InvalidConfigurationPropertyValueFailureAnalyzer
}
private List<Descriptor> getDescriptors(String propertyName) {
Set<Origin> seen = new HashSet<>();
return getPropertySources().filter((source) -> source.containsProperty(propertyName))
.map((source) -> Descriptor.get(source, propertyName))
.filter((descriptor) -> seen.add(getOrigin(descriptor)))
.toList();
}
private Stream<PropertySource<?>> getPropertySources() {
if (this.environment == null) {
return Stream.empty();
private Origin getOrigin(Descriptor descriptor) {
Origin origin = descriptor.origin;
if (origin instanceof PropertySourceOrigin propertySourceOrigin) {
origin = propertySourceOrigin.getOrigin();
}
return this.environment.getPropertySources()
.stream()
.filter((source) -> !ConfigurationPropertySources.isAttachedConfigurationPropertySource(source));
return origin;
}
private Stream<PropertySource<?>> getPropertySources() {
return (this.environment != null) ? this.environment.getPropertySources().stream() : Stream.empty();
}
private void appendDetails(StringBuilder message, InvalidConfigurationPropertyValueException cause,

View File

@@ -20,13 +20,16 @@ import java.util.Collections;
import org.junit.jupiter.api.Test;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException;
import org.springframework.boot.diagnostics.FailureAnalysis;
import org.springframework.boot.origin.Origin;
import org.springframework.boot.origin.OriginLookup;
import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.SystemEnvironmentPropertySource;
import org.springframework.mock.env.MockEnvironment;
import org.springframework.util.ObjectUtils;
import static org.assertj.core.api.Assertions.assertThat;
@@ -63,6 +66,26 @@ class InvalidConfigurationPropertyValueFailureAnalyzerTests {
.doesNotContain("Additionally, this property is also set");
}
@Test
void analysisWithKnownPropertyFromSystemEnvironment() {
MapPropertySource source = new SystemEnvironmentPropertySource("systemEnvironment",
Collections.singletonMap("COM_EXAMPLE_TESTPROPERTY", "invalid"));
this.environment.getPropertySources().addFirst(source);
ConfigurationPropertySources.attach(this.environment);
assertThat(this.environment.getProperty("com.example.test-property")).isEqualTo("invalid");
InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException(
"com.example.test-property", "invalid", "This is not valid.");
FailureAnalysis analysis = performAnalysis(failure);
assertThat(analysis.getDescription()).contains("com.example.test-property")
.contains("invalid")
.contains("property source \"systemEnvironment\"");
assertThat(analysis.getCause()).isSameAs(failure);
assertThat(analysis.getAction()).contains("Review the value of the property with the provided reason.");
assertThat(analysis.getDescription()).contains("Validation failed for the following reason")
.contains("This is not valid.")
.doesNotContain("Additionally, this property is also set");
}
@Test
void analysisWithKnownPropertyAndNoReason() {
MapPropertySource source = new MapPropertySource("test", Collections.singletonMap("test.property", "invalid"));
@@ -84,6 +107,8 @@ class InvalidConfigurationPropertyValueFailureAnalyzerTests {
this.environment.getPropertySources().addFirst(OriginCapablePropertySource.get(source));
this.environment.getPropertySources().addLast(additional);
this.environment.getPropertySources().addLast(OriginCapablePropertySource.get(another));
this.environment.getPropertySources().addLast(OriginCapablePropertySource.get("another-again", another));
ConfigurationPropertySources.attach(this.environment);
InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException(
"test.property", "invalid", "This is not valid.");
FailureAnalysis analysis = performAnalysis(failure);
@@ -92,7 +117,8 @@ class InvalidConfigurationPropertyValueFailureAnalyzerTests {
assertThat(analysis.getDescription())
.contains("Additionally, this property is also set in the following property sources:")
.contains("In 'additional' with the value 'valid'")
.contains("In 'another' with the value 'test' (originating from 'TestOrigin test.property')");
.contains("In 'another' with the value 'test' (originating from 'TestOrigin test.property')")
.doesNotContain("another-again");
}
@Test
@@ -122,7 +148,11 @@ class InvalidConfigurationPropertyValueFailureAnalyzerTests {
private final EnumerablePropertySource<T> propertySource;
OriginCapablePropertySource(EnumerablePropertySource<T> propertySource) {
super(propertySource.getName(), propertySource.getSource());
this(propertySource.getName(), propertySource);
}
OriginCapablePropertySource(String name, EnumerablePropertySource<T> propertySource) {
super(name, propertySource.getSource());
this.propertySource = propertySource;
}
@@ -138,20 +168,53 @@ class InvalidConfigurationPropertyValueFailureAnalyzerTests {
@Override
public Origin getOrigin(String name) {
return new Origin() {
@Override
public String toString() {
return "TestOrigin " + name;
}
};
return new TestOrigin(name, this.propertySource.getName());
}
static <T> OriginCapablePropertySource<T> get(EnumerablePropertySource<T> propertySource) {
return new OriginCapablePropertySource<>(propertySource);
}
static <T> OriginCapablePropertySource<T> get(String name, EnumerablePropertySource<T> propertySource) {
return new OriginCapablePropertySource<>(name, propertySource);
}
static final class TestOrigin implements Origin {
private final String name;
private final String sourceName;
private TestOrigin(String name, String sourceName) {
this.name = name;
this.sourceName = sourceName;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
TestOrigin other = (TestOrigin) obj;
return ObjectUtils.nullSafeEquals(this.name, other.name)
&& ObjectUtils.nullSafeEquals(this.sourceName, other.sourceName);
}
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.name);
}
@Override
public String toString() {
return "TestOrigin " + this.name;
}
}
}
}