Skip to content

Commit 51b8778

Browse files
ryanjbaxterryanjbaxter
authored
Allow file based repos to continue on error when fetching multiple labels (#3130)
Fixes #3126
1 parent 16da639 commit 51b8778

5 files changed

Lines changed: 121 additions & 4 deletions

File tree

‎docs/modules/ROOT/pages/server/environment-repository/version-control-backend-filesystem-use.adoc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,13 @@ Some operating systems https://serverfault.com/questions/377348/when-does-tmp-ge
99
This can lead to unexpected behavior, such as missing properties.
1010
To avoid this problem, change the directory that Config Server uses by setting `spring.cloud.config.server.git.basedir` or `spring.cloud.config.server.svn.basedir` to a directory that does not reside in the system temp structure.
1111

12+
[[errors-with-multiple-labels]]
13+
== Error Handling With Multiple Labels
14+
15+
If a request is made to the config server and the request contains multiple labels the config server
16+
will return property sources for each label. However if trying to fetch one of those labels results in
17+
an error, the config server will return an error without trying any remaining labels.
18+
19+
If you prefer to have the config server ignore any errors when a label is invalid and try all
20+
labels before returning an error you can set `spring.cloud.config.server.[git | svn].continue-on-multiple-label-failure=true`.
21+

‎spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/AbstractScmEnvironmentRepository.java‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,23 @@ public abstract class AbstractScmEnvironmentRepository extends AbstractScmAccess
3939

4040
private final EnvironmentCleaner cleaner = new EnvironmentCleaner();
4141

42+
private boolean isContinueOnMultipleLabelFailure;
43+
4244
private int order = Ordered.LOWEST_PRECEDENCE;
4345

4446
private final ObservationRegistry observationRegistry;
4547

4648
public AbstractScmEnvironmentRepository(ConfigurableEnvironment environment,
4749
ObservationRegistry observationRegistry) {
4850
super(environment);
51+
this.isContinueOnMultipleLabelFailure = false;
4952
this.observationRegistry = observationRegistry;
5053
}
5154

5255
public AbstractScmEnvironmentRepository(ConfigurableEnvironment environment,
5356
AbstractScmAccessorProperties properties, ObservationRegistry observationRegistry) {
5457
super(environment, properties);
58+
this.isContinueOnMultipleLabelFailure = properties.isContinueOnMultipleLabelFailure();
5559
this.order = properties.getOrder();
5660
this.observationRegistry = observationRegistry;
5761
}
@@ -66,10 +70,21 @@ public synchronized Environment findOne(String application, String profile, Stri
6670
var environment = new Environment(application, StringUtils.commaDelimitedListToStringArray(profile), label, "",
6771
"");
6872

69-
for (String l : splitAndReorder(label)) {
70-
var e = findOneInternal(application, profile, l, includeOrigin);
71-
environment.addAll(e.getPropertySources());
72-
environment.setVersion(concat(e.getVersion(), environment.getVersion()));
73+
List<String> labels = splitAndReorder(label);
74+
for (String l : labels) {
75+
try {
76+
var e = findOneInternal(application, profile, l, includeOrigin);
77+
environment.addAll(e.getPropertySources());
78+
environment.setVersion(concat(e.getVersion(), environment.getVersion()));
79+
}
80+
catch (Exception e) {
81+
logger.warn("Could not find label " + l + " for application " + application + " and profile " + profile,
82+
e);
83+
if (labels.size() == 1 || !this.isContinueOnMultipleLabelFailure) {
84+
throw e;
85+
}
86+
logger.debug("is-continue-on-multiple-label-failure is true, continuing with next label");
87+
}
7388
}
7489

7590
return this.cleaner.clean(environment, getWorkingDirectory().toURI().toString(), getUri());
@@ -122,4 +137,12 @@ public void setOrder(int order) {
122137
this.order = order;
123138
}
124139

140+
boolean isContinueOnMultipleLabelFailure() {
141+
return isContinueOnMultipleLabelFailure;
142+
}
143+
144+
void setContinueOnMultipleLabelFailure(boolean continueOnMultipleLabelFailure) {
145+
isContinueOnMultipleLabelFailure = continueOnMultipleLabelFailure;
146+
}
147+
125148
}

‎spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/support/AbstractScmAccessorProperties.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ public class AbstractScmAccessorProperties implements EnvironmentRepositoryPrope
6666
/** The default label to be used with the remote repository. */
6767
private String defaultLabel;
6868

69+
/**
70+
* Flag to indicate whether to continue on multiple label failure. Defaults to false.
71+
*/
72+
private boolean continueOnMultipleLabelFailure = false;
73+
6974
public String getUri() {
7075
return this.uri;
7176
}
@@ -139,4 +144,12 @@ public void setDefaultLabel(String defaultLabel) {
139144
this.defaultLabel = defaultLabel;
140145
}
141146

147+
public boolean isContinueOnMultipleLabelFailure() {
148+
return continueOnMultipleLabelFailure;
149+
}
150+
151+
public void setContinueOnMultipleLabelFailure(boolean continueOnMultipleLabelFailure) {
152+
this.continueOnMultipleLabelFailure = continueOnMultipleLabelFailure;
153+
}
154+
142155
}

‎spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282

8383
import static junit.framework.TestCase.assertTrue;
8484
import static org.assertj.core.api.Assertions.assertThat;
85+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
8586
import static org.mockito.ArgumentMatchers.any;
8687
import static org.mockito.ArgumentMatchers.anyString;
8788
import static org.mockito.ArgumentMatchers.eq;
@@ -217,6 +218,40 @@ public void multipleLabels() {
217218
assertThat(environment.getPropertySources()).hasSize(6);
218219
}
219220

221+
@Test
222+
public void multipleLabelsWithFailureButContinues() {
223+
try {
224+
this.repository.setBasedir(this.basedir);
225+
this.repository.setContinueOnMultipleLabelFailure(true);
226+
Environment environment = this.repository.findOne("bar", "staging", "master,doesnotexist,foo,raw");
227+
assertThat(environment.getPropertySources()).hasSize(6);
228+
}
229+
finally {
230+
this.repository.setContinueOnMultipleLabelFailure(false);
231+
}
232+
}
233+
234+
@Test
235+
public void singleLabelWithFailureButContinues() {
236+
try {
237+
this.repository.setBasedir(this.basedir);
238+
this.repository.setContinueOnMultipleLabelFailure(true);
239+
assertThatThrownBy(() -> this.repository.findOne("bar", "staging", "doesnotexist"))
240+
.isInstanceOf(NoSuchLabelException.class);
241+
242+
}
243+
finally {
244+
this.repository.setContinueOnMultipleLabelFailure(false);
245+
}
246+
}
247+
248+
@Test
249+
public void multipleLabelsWithFailure() {
250+
this.repository.setBasedir(this.basedir);
251+
assertThatThrownBy(() -> this.repository.findOne("bar", "staging", "master,doesnotexist,foo,raw"))
252+
.isInstanceOf(NoSuchLabelException.class);
253+
}
254+
220255
@Test
221256
public void basedirExists() throws Exception {
222257
assertThat(this.basedir.mkdirs()).isTrue();

‎spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/SVNKitEnvironmentRepositoryTests.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.core.env.StandardEnvironment;
3535

3636
import static org.assertj.core.api.Assertions.assertThat;
37+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
3738

3839
/**
3940
* @author Michael Prankl
@@ -124,6 +125,41 @@ public void testMultipleLabels() {
124125
assertThat(environment.getPropertySources().get(2).getName()).contains("branches/demobranch/bar.properties");
125126
}
126127

128+
@Test
129+
public void testMultipleLabelsWithFailure() {
130+
assertThatThrownBy(() -> this.repository.findOne("bar", "staging", "branches/demobranch,doesnotexist,trunk"))
131+
.isInstanceOf(NoSuchLabelException.class);
132+
}
133+
134+
@Test
135+
public void testSingleLabelWithFailure() {
136+
try {
137+
this.repository.setContinueOnMultipleLabelFailure(true);
138+
assertThatThrownBy(() -> this.repository.findOne("bar", "staging", "doesnotexist"))
139+
.isInstanceOf(NoSuchLabelException.class);
140+
}
141+
finally {
142+
this.repository.setContinueOnMultipleLabelFailure(false);
143+
}
144+
}
145+
146+
@Test
147+
public void testMultipleLabelsWithContinueOnFailure() {
148+
try {
149+
this.repository.setContinueOnMultipleLabelFailure(true);
150+
Environment environment = this.repository.findOne("bar", "staging",
151+
"branches/demobranch,doesnotexist,trunk");
152+
assertThat(environment.getPropertySources()).hasSize(3);
153+
assertThat(environment.getPropertySources().get(0).getName()).contains("bar.properties");
154+
assertThat(environment.getPropertySources().get(1).getName()).contains("application.yml");
155+
assertThat(environment.getPropertySources().get(2).getName())
156+
.contains("branches/demobranch/bar.properties");
157+
}
158+
finally {
159+
this.repository.setContinueOnMultipleLabelFailure(false);
160+
}
161+
}
162+
127163
@Test
128164
public void invalidLabel() {
129165
Assertions.assertThatThrownBy(() -> {

0 commit comments

Comments
 (0)