From 02d59362e6b770605bf81f1eadeaf5fdad2635c7 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 11 Dec 2019 18:25:57 -0500 Subject: [PATCH] Forces environment controller to be backwards compatible. Does this by adding produces application/json on backwards compatible controller methods. Otherwise, old clients would get the new format. fixes gh-1520 --- .../environment/EnvironmentController.java | 6 +- ...BootstrapConfigServerIntegrationTests.java | 9 +- .../server/CompositeIntegrationTests.java | 17 ++- ...ackwardsCompatibilityIntegrationTests.java | 114 ++++++++++++++++++ .../NativeConfigServerIntegrationTests.java | 8 +- ...freshableConfigServerIntegrationTests.java | 14 ++- ...ubversionConfigServerIntegrationTests.java | 9 +- .../VanillaConfigServerIntegrationTests.java | 7 +- .../EncryptionIntegrationTests.java | 9 +- .../server/test/ConfigServerTestUtils.java | 9 ++ 10 files changed, 178 insertions(+), 24 deletions(-) create mode 100644 spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ConfigClientBackwardsCompatibilityIntegrationTests.java diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java index 32ae18e4..25fa5d04 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java @@ -102,7 +102,8 @@ public class EnvironmentController { this.acceptEmpty = acceptEmpty; } - @RequestMapping("/{name}/{profiles:.*[^-].*}") + @RequestMapping(path = "/{name}/{profiles:.*[^-].*}", + produces = MediaType.APPLICATION_JSON_VALUE) public Environment defaultLabel(@PathVariable String name, @PathVariable String profiles) { return getEnvironment(name, profiles, null, false); @@ -115,7 +116,8 @@ public class EnvironmentController { return getEnvironment(name, profiles, null, true); } - @RequestMapping("/{name}/{profiles}/{label:.*}") + @RequestMapping(path = "/{name}/{profiles}/{label:.*}", + produces = MediaType.APPLICATION_JSON_VALUE) public Environment labelled(@PathVariable String name, @PathVariable String profiles, @PathVariable String label) { return getEnvironment(name, profiles, label, false); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/BootstrapConfigServerIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/BootstrapConfigServerIntegrationTests.java index 9d2b3b7f..1741dc21 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/BootstrapConfigServerIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/BootstrapConfigServerIntegrationTests.java @@ -30,11 +30,14 @@ import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.boot.web.server.LocalServerPort; import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.assertOriginTrackedValue; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; @RunWith(SpringRunner.class) @SpringBootTest(classes = ConfigServerApplication.class, @@ -63,8 +66,10 @@ public class BootstrapConfigServerIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = response.getBody(); assertThat(environment.getPropertySources()).hasSize(2); assertOriginTrackedValue(environment, 0, "bar", "foo"); assertOriginTrackedValue(environment, 1, "info.foo", "bar"); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/CompositeIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/CompositeIntegrationTests.java index 79599e1c..c122999a 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/CompositeIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/CompositeIntegrationTests.java @@ -27,11 +27,14 @@ import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.boot.web.server.LocalServerPort; import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; /** * @author Ryan Baxter @@ -64,9 +67,10 @@ public class CompositeIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", - Environment.class); + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = response.getBody(); assertThat(3).isEqualTo(environment.getPropertySources().size()); assertThat("overrides") .isEqualTo(environment.getPropertySources().get(0).getName()); @@ -125,9 +129,10 @@ public class CompositeIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", - Environment.class); + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = response.getBody(); assertThat(environment.getPropertySources()).hasSize(3); assertThat("overrides") .isEqualTo(environment.getPropertySources().get(0).getName()); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ConfigClientBackwardsCompatibilityIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ConfigClientBackwardsCompatibilityIntegrationTests.java new file mode 100644 index 00000000..9f4414f5 --- /dev/null +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ConfigClientBackwardsCompatibilityIntegrationTests.java @@ -0,0 +1,114 @@ +/* + * 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 org.springframework.cloud.config.server; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import org.eclipse.jgit.junit.MockSystemReader; +import org.eclipse.jgit.util.SystemReader; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +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.cloud.config.server.test.ConfigServerTestUtils; +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; + +@RunWith(SpringRunner.class) +@SpringBootTest(classes = ConfigServerApplication.class, + properties = { "spring.config.name:configserver" }, webEnvironment = RANDOM_PORT) +@ActiveProfiles({ "test", "native" }) +public class ConfigClientBackwardsCompatibilityIntegrationTests { + + @LocalServerPort + private int port; + + @Autowired + private ApplicationContext context; + + @BeforeClass + public static void init() throws IOException { + // mock Git configuration to make tests independent of local Git configuration + SystemReader.setInstance(new MockSystemReader()); + + ConfigServerTestUtils.prepareLocalRepo(); + } + + @Test + public void testBackwardsCompatibleFormat() { + Map environment = new TestRestTemplate().getForObject( + "http://localhost:" + this.port + "/foo/development/", Map.class); + Object value = getPropertySourceValue(environment); + assertThat(value).isInstanceOf(String.class).isEqualTo("true"); + } + + @Test + public void testBackwardsCompatibleFormatWithLabel() { + Map environment = new TestRestTemplate().getForObject( + "http://localhost:" + this.port + "/foo/development/master", Map.class); + Object value = getPropertySourceValue(environment); + assertThat(value).isInstanceOf(String.class).isEqualTo("true"); + } + + @Test + public void testNewFormat() { + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Map.class); + Object value = getPropertySourceValue(response.getBody()); + assertThat(value).isInstanceOf(Map.class); + Map valueMap = Map.class.cast(value); + assertThat(valueMap).containsEntry("value", "true"); + } + + @Test + public void testNewFormatWithLabel() { + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/master", + HttpMethod.GET, getV2AcceptEntity(), Map.class); + Object value = getPropertySourceValue(response.getBody()); + assertThat(value).isInstanceOf(Map.class); + Map valueMap = Map.class.cast(value); + assertThat(valueMap).containsEntry("value", "true"); + } + + private Object getPropertySourceValue(Map environment) { + assertThat(environment).isNotNull(); + assertThat(environment.containsKey("propertySources")); + List propertySources = (List) environment.get("propertySources"); + assertThat(propertySources).hasSizeGreaterThan(1); + Map source = (Map) propertySources.get(0); + assertThat(source).containsKeys("source"); + Map sourceValue = (Map) source.get("source"); + assertThat(sourceValue).containsKeys("spring.cloud.config.enabled"); + return sourceValue.get("spring.cloud.config.enabled"); + } + +} diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/NativeConfigServerIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/NativeConfigServerIntegrationTests.java index 69b99f7c..1d12c16b 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/NativeConfigServerIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/NativeConfigServerIntegrationTests.java @@ -29,6 +29,7 @@ import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.boot.web.server.LocalServerPort; import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; @@ -36,6 +37,7 @@ import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; @RunWith(SpringRunner.class) @SpringBootTest(classes = ConfigServerApplication.class, @@ -56,8 +58,10 @@ public class NativeConfigServerIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = response.getBody(); assertThat(environment.getPropertySources().isEmpty()).isFalse(); assertThat(environment.getPropertySources().get(0).getName()) .isEqualTo("overrides"); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/RefreshableConfigServerIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/RefreshableConfigServerIntegrationTests.java index 27d60a0a..2069ab29 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/RefreshableConfigServerIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/RefreshableConfigServerIntegrationTests.java @@ -41,6 +41,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.core.io.ByteArrayResource; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; @@ -52,6 +53,7 @@ import static org.mockito.ArgumentMatchers.isA; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.BDDMockito.given; import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.assertOriginTrackedValue; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; @RunWith(SpringRunner.class) @SpringBootTest(classes = TestConfiguration.class, @@ -91,8 +93,10 @@ public class RefreshableConfigServerIntegrationTests { */ @Test public void refreshOverrides() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + ResponseEntity entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = entity.getBody(); assertThat(environment.getPropertySources()).isEmpty(); String actuatorEndpoint = "http://localhost:" + this.port + "/actuator"; @@ -109,8 +113,10 @@ public class RefreshableConfigServerIntegrationTests { null, Void.class); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); - environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + environment = entity.getBody(); assertThat(environment.getPropertySources()).isNotEmpty(); assertOriginTrackedValue(environment, 0, "foo", "bar"); } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/SubversionConfigServerIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/SubversionConfigServerIntegrationTests.java index 52239926..4c72f897 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/SubversionConfigServerIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/SubversionConfigServerIntegrationTests.java @@ -31,12 +31,15 @@ import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.environment.SvnKitEnvironmentRepository; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.REPO_PREFIX; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; /** * @author Michael Prankl @@ -65,8 +68,10 @@ public class SubversionConfigServerIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + ResponseEntity exchange = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = exchange.getBody(); assertThat(environment.getPropertySources().isEmpty()).isFalse(); assertThat(environment.getPropertySources().get(0).getName()) .isEqualTo("overrides"); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/VanillaConfigServerIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/VanillaConfigServerIntegrationTests.java index e89e1d5a..97583745 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/VanillaConfigServerIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/VanillaConfigServerIntegrationTests.java @@ -40,6 +40,7 @@ import org.springframework.test.context.junit4.SpringRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; +import static org.springframework.cloud.config.server.test.ConfigServerTestUtils.getV2AcceptEntity; @RunWith(SpringRunner.class) @SpringBootTest(classes = ConfigServerApplication.class, @@ -62,8 +63,10 @@ public class VanillaConfigServerIntegrationTests { @Test public void contextLoads() { - Environment environment = new TestRestTemplate().getForObject( - "http://localhost:" + this.port + "/foo/development/", Environment.class); + ResponseEntity response = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/foo/development/", HttpMethod.GET, + getV2AcceptEntity(), Environment.class); + Environment environment = response.getBody(); assertThat(environment.getPropertySources().isEmpty()).isFalse(); assertThat(environment.getPropertySources().get(0).getName()) .isEqualTo("overrides"); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionIntegrationTests.java index 37e3e3d4..43343095 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionIntegrationTests.java @@ -99,10 +99,11 @@ public class EncryptionIntegrationTests { } @RunWith(SpringRunner.class) - @SpringBootTest(classes = { ConfigServerApplication.class }, properties = { - "spring.cloud.bootstrap.name:keystore-bootstrap", - "spring.cloud.config.server.encrypt.enabled=false", - "encrypt.keyStore.alias=myencryptionkey" }, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) + @SpringBootTest(classes = { ConfigServerApplication.class }, + properties = { "spring.cloud.bootstrap.name:keystore-bootstrap", + "spring.cloud.config.server.encrypt.enabled=false", + "encrypt.keyStore.alias=myencryptionkey" }, + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @ActiveProfiles({ "test", "git" }) @DirtiesContext public static class KeystoreConfigurationEncryptionOnlyIntegrationTests { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/test/ConfigServerTestUtils.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/test/ConfigServerTestUtils.java index 5e4596ed..3072aed8 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/test/ConfigServerTestUtils.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/test/ConfigServerTestUtils.java @@ -26,7 +26,10 @@ import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.springframework.cloud.config.environment.Environment; +import org.springframework.cloud.config.environment.EnvironmentMediaType; import org.springframework.cloud.config.environment.PropertySource; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; import org.springframework.util.FileSystemUtils; import org.springframework.util.StringUtils; @@ -163,4 +166,10 @@ public final class ConfigServerTestUtils { assertThat(map).containsEntry("value", expectedValue); } + public static HttpEntity getV2AcceptEntity() { + HttpHeaders headers = new HttpHeaders(); + headers.set(HttpHeaders.ACCEPT, EnvironmentMediaType.V2_JSON); + return new HttpEntity(headers); + } + }