Skip to content

Commit 93b29d8

Browse files
mrm9084mrm9084
authored
App Config Spring - Push refresh fix (Azure#35388)
* check all instances * Fixing Push refresh with tests * Fixing Linting
1 parent 63d5ac4 commit 93b29d8

File tree

7 files changed

+98
-40
lines changed

7 files changed

+98
-40
lines changed

‎sdk/spring/spring-cloud-azure-appconfiguration-config-web/src/main/java/com/azure/spring/cloud/appconfiguration/config/web/implementation/AppConfigurationEndpoint.java‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.AppConfigurationWebConstants.DATA;
66
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.AppConfigurationWebConstants.SYNC_TOKEN;
77
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.AppConfigurationWebConstants.VALIDATION_CODE_KEY;
8-
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.AppConfigurationWebConstants.VALIDATION_TOPIC;
98

109
import java.io.IOException;
10+
import java.net.URI;
1111
import java.util.List;
12-
import java.util.Locale;
1312
import java.util.Map;
1413
import java.util.stream.Collectors;
1514

@@ -18,20 +17,17 @@
1817
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationStoreMonitoring.AccessToken;
1918
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationStoreMonitoring.PushNotification;
2019
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.ConfigStore;
21-
2220
import com.fasterxml.jackson.databind.JsonNode;
2321
import com.fasterxml.jackson.databind.ObjectMapper;
2422

2523
/**
2624
* Common class for authenticating refresh requests.
2725
*/
2826
public class AppConfigurationEndpoint {
27+
28+
private static final String CONFIG_STORE_SUBJECT = "subject";
2929

30-
private static final String CONFIG_STORE_TOPIC = "configurationstores";
31-
32-
private final String endpoint;
33-
34-
private final String store;
30+
private final URI endpoint;
3531

3632
private final List<ConfigStore> configStores;
3733

@@ -75,12 +71,10 @@ public AppConfigurationEndpoint(HttpServletRequest request, List<ConfigStore> co
7571

7672
validationResponse = requestBody.findValue(VALIDATION_CODE_KEY);
7773

78-
JsonNode requestTopic = requestBody.findValue(VALIDATION_TOPIC);
79-
if (requestTopic != null) {
80-
String topic = requestTopic.asText();
81-
store = topic.substring(
82-
topic.toLowerCase(Locale.ROOT).indexOf(CONFIG_STORE_TOPIC) + CONFIG_STORE_TOPIC.length() + 1);
83-
endpoint = String.format("https://%s", store);
74+
JsonNode requestSubject = requestBody.findValue(CONFIG_STORE_SUBJECT);
75+
if (requestSubject != null) {
76+
String subject = requestSubject.asText();
77+
endpoint = URI.create(subject);
8478
} else {
8579
throw new IllegalArgumentException("Refresh request missing topic field.");
8680
}
@@ -94,7 +88,7 @@ public AppConfigurationEndpoint(HttpServletRequest request, List<ConfigStore> co
9488
*/
9589
public boolean authenticate() {
9690
for (ConfigStore configStore : configStores) {
97-
if (configStore.getEndpoint().startsWith(endpoint)) {
91+
if (configStore.containsEndpoint(getEndpoint())) {
9892
PushNotification pushNotification = configStore.getMonitoring().getPushNotification();
9993

10094
// One of these need to be set
@@ -129,7 +123,7 @@ private boolean isTokenMatch(AccessToken token) {
129123
*/
130124
public boolean triggerRefresh() {
131125
for (ConfigStore configStore : configStores) {
132-
if (configStore.getEndpoint().startsWith(endpoint) && configStore.getMonitoring().isEnabled()) {
126+
if (configStore.containsEndpoint(getEndpoint()) && configStore.getMonitoring().isEnabled()) {
133127
return true;
134128
}
135129
}
@@ -155,14 +149,6 @@ public JsonNode getValidationResponse() {
155149
* @return the endpoint
156150
*/
157151
public String getEndpoint() {
158-
return endpoint;
159-
}
160-
161-
/**
162-
* @return the store
163-
*/
164-
public String getStore() {
165-
return store;
152+
return endpoint.getScheme() + "://" + endpoint.getHost();
166153
}
167-
168154
}

‎sdk/spring/spring-cloud-azure-appconfiguration-config-web/src/test/java/com/azure/spring/cloud/appconfiguration/config/web/implementation/AppConfigurationEndpointTest.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,13 @@ public void validationParsing() throws JsonGenerationException, JsonMappingExcep
6767
Map<String, String> allRequestParams = new HashMap<String, String>();
6868

6969
AppConfigurationEndpoint endpoint = new AppConfigurationEndpoint(request, configStores, allRequestParams);
70-
assertEquals("https://testConfig", endpoint.getEndpoint());
71-
assertEquals("testConfig", endpoint.getStore());
70+
assertEquals("https://fake.test.azconfig.io", endpoint.getEndpoint());
7271

7372
requestBody = mapper.readValue(new File(GET_TEST_REFRESH), JsonNode.class).toString();
7473
when(lines.collect(Mockito.any())).thenReturn(requestBody);
7574

7675
endpoint = new AppConfigurationEndpoint(request, configStores, allRequestParams);
77-
assertEquals("https://testConfig", endpoint.getEndpoint());
78-
assertEquals("testConfig", endpoint.getStore());
76+
assertEquals("https://testconfig.azconfig.io", endpoint.getEndpoint());
7977
}
8078

8179
@Test
@@ -109,7 +107,7 @@ public void authenticate() throws JsonParseException, JsonMappingException, IOEx
109107
assertFalse(endpoint.authenticate());
110108

111109
ConfigStore validConfigStore = new ConfigStore();
112-
validConfigStore.setEndpoint("https://testConfig.azconfig.io");
110+
validConfigStore.setEndpoint("https://fake.test.azconfig.io");
113111
configStores.add(validConfigStore);
114112
endpoint = new AppConfigurationEndpoint(request, configStores, allRequestParams);
115113
// Valid Config Store, no secrets
@@ -174,7 +172,7 @@ public void triggerRefresh() throws JsonParseException, JsonMappingException, IO
174172
assertFalse(endpoint.authenticate());
175173

176174
ConfigStore validConfigStore = new ConfigStore();
177-
validConfigStore.setEndpoint("https://testConfig.azconfig.io");
175+
validConfigStore.setEndpoint("https://fake.test.azconfig.io");
178176

179177
configStores.add(validConfigStore);
180178
endpoint = new AppConfigurationEndpoint(request, configStores, allRequestParams);

‎sdk/spring/spring-cloud-azure-appconfiguration-config-web/src/test/java/com/azure/spring/cloud/appconfiguration/config/web/implementation/pushbusrefresh/AppConfigurationBusRefreshEndpointTest.java‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.TestConstants.TRIGGER_LABEL;
88
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.TestConstants.VALIDATION_URL;
99
import static org.junit.jupiter.api.Assertions.assertEquals;
10+
import static org.mockito.ArgumentMatchers.argThat;
11+
import static org.mockito.Mockito.times;
12+
import static org.mockito.Mockito.verify;
1013
import static org.mockito.Mockito.when;
1114

1215
import java.io.BufferedReader;
@@ -84,6 +87,7 @@ public void setup() throws IOException {
8487
configStores = new ArrayList<>();
8588
configStore.setMonitoring(monitoring);
8689
configStore.setEndpoint("https://fake.test.azconfig.io");
90+
configStore.validateAndInit();
8791
configStores.add(configStore);
8892

8993
when(request.getReader()).thenReturn(reader);
@@ -110,10 +114,11 @@ public void webHookValidation() throws IOException {
110114
when(lines.collect(Mockito.any())).thenReturn("[{\r\n"
111115
+ " \"id\": \"2d1781af-3a4c-4d7c-bd0c-e34b19da4e66\",\r\n"
112116
+ " \"topic\":" + TOPIC + ",\r\n"
113-
+ " \"subject\": \"\",\r\n"
117+
+ " \"subject\": \"https://fake.test.azconfig.io/kv/Foo?label=FizzBuzz\",\r\n"
114118
+ " \"data\": {\r\n"
115119
+ " \"validationCode\": \"512d38b6-c7b8-40c8-89fe-f46f9e9622b6\",\r\n"
116-
+ " \"validationUrl\":" + VALIDATION_URL + "\r\n"
120+
+ " \"validationUrl\":" + VALIDATION_URL + ",\r\n"
121+
+ " \"syncToken\": \"zAJw6V16=MzoyMCMyODA3Mzc3;sn=2807377\"\r\n"
117122
+ " },\r\n"
118123
+ " \"eventType\": \"Microsoft.EventGrid.SubscriptionValidationEvent\",\r\n"
119124
+ " \"eventTime\": \"2018-01-25T22:12:19.4556811Z\",\r\n"
@@ -140,6 +145,11 @@ public void webHookRefresh() throws IOException {
140145
when(lines.collect(Mockito.any())).thenReturn(getResetNotification());
141146

142147
assertEquals(HttpStatus.OK.getReasonPhrase(), endpoint.refresh(request, response, allRequestParams));
148+
verify(publisher, times(1)).publishEvent(argThat(refreshEvent -> {
149+
boolean hasEndpoint = ((AppConfigurationBusRefreshEvent) refreshEvent).getEndpoint().equals("https://fake.test.azconfig.io");
150+
boolean hasSyncToken = ((AppConfigurationBusRefreshEvent) refreshEvent).getSyncToken().equals("zAJw6V16=MzoyMCMyODA3Mzc3;sn=2807377");
151+
return hasEndpoint && hasSyncToken;
152+
}));
143153
}
144154

145155
@Test
@@ -255,7 +265,8 @@ private String getResetNotification() {
255265
+ " \"data\" : {\r\n"
256266
+ " \"key\" : \"trigger_key\",\r\n"
257267
+ " \"label\" : \"trigger_label\",\r\n"
258-
+ " \"etag\" : \"r05tB2hfMQs0vo6ITcXu7ScIOhR\"\r\n"
268+
+ " \"etag\" : \"r05tB2hfMQs0vo6ITcXu7ScIOhR\",\r\n"
269+
+ " \"syncToken\": \"zAJw6V16=MzoyMCMyODA3Mzc3;sn=2807377\"\r\n"
259270
+ " },\r\n"
260271
+ " \"eventType\" : \"Microsoft.AppConfiguration.KeyValueModified\",\r\n"
261272
+ " \"dataVersion\" : \"1\",\r\n"

‎sdk/spring/spring-cloud-azure-appconfiguration-config-web/src/test/java/com/azure/spring/cloud/appconfiguration/config/web/implementation/pushrefresh/AppConfigurationRefreshEndpointTest.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.TestConstants.TRIGGER_LABEL;
88
import static com.azure.spring.cloud.appconfiguration.config.web.implementation.TestConstants.VALIDATION_URL;
99
import static org.junit.jupiter.api.Assertions.assertEquals;
10+
import static org.mockito.ArgumentMatchers.argThat;
11+
import static org.mockito.Mockito.times;
12+
import static org.mockito.Mockito.verify;
1013
import static org.mockito.Mockito.when;
1114

1215
import java.io.BufferedReader;
@@ -61,6 +64,8 @@ public class AppConfigurationRefreshEndpointTest {
6164
private ArrayList<AppConfigurationStoreTrigger> triggers;
6265

6366
private AppConfigurationStoreMonitoring monitoring;
67+
68+
private String endpoint = "https://fake.test.azconfig.io";
6469

6570
private String tokenName = "token";
6671

@@ -112,10 +117,11 @@ public void webHookValidation() throws IOException {
112117
when(lines.collect(Mockito.any())).thenReturn("[{\r\n"
113118
+ " \"id\": \"2d1781af-3a4c-4d7c-bd0c-e34b19da4e66\",\r\n"
114119
+ " \"topic\":" + TOPIC + ",\r\n"
115-
+ " \"subject\": \"\",\r\n"
120+
+ " \"subject\": \"https://fake.test.azconfig.io/kv/Foo?label=FizzBuzz\",\r\n"
116121
+ " \"data\": {\r\n"
117122
+ " \"validationCode\": \"512d38b6-c7b8-40c8-89fe-f46f9e9622b6\",\r\n"
118-
+ " \"validationUrl\":" + VALIDATION_URL + "\r\n"
123+
+ " \"validationUrl\":" + VALIDATION_URL + ",\r\n"
124+
+ " \"syncToken\": \"zAJw6V16=MzoyMCMyODA3Mzc3;sn=2807377\"\r\n"
119125
+ " },\r\n"
120126
+ " \"eventType\": \"Microsoft.EventGrid.SubscriptionValidationEvent\",\r\n"
121127
+ " \"eventTime\": \"2018-01-25T22:12:19.4556811Z\",\r\n"
@@ -141,6 +147,11 @@ public void webHookRefresh() throws IOException {
141147
when(lines.collect(Mockito.any())).thenReturn(getResetNotification());
142148

143149
assertEquals(HttpStatus.OK.getReasonPhrase(), endpoint.refresh(request, response, allRequestParams));
150+
verify(publisher, times(1)).publishEvent(argThat(refreshEvent -> {
151+
boolean hasEndpoint = ((AppConfigurationRefreshEvent) refreshEvent).getEndpoint().equals("https://fake.test.azconfig.io");
152+
boolean hasSyncToken = ((AppConfigurationRefreshEvent) refreshEvent).getSyncToken().equals("zAJw6V16=MzoyMCMyODA3Mzc3;sn=2807377");
153+
return hasEndpoint && hasSyncToken;
154+
}));
144155
}
145156

146157
@Test

‎sdk/spring/spring-cloud-azure-appconfiguration-config-web/src/test/resources/webHookValidation.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"id": "2d1781af-3a4c-4d7c-bd0c-e34b19da4e66",
44
"topic": "/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/resourceGroups/testGroup/providers/microsoft.appconfiguration/configurationstores/testConfig",
5-
"subject": "",
5+
"subject": "https://fake.test.azconfig.io/kv/Foo?label=FizzBuzz",
66
"data": {
77
"validationCode": "512d38b6-c7b8-40c8-89fe-f46f9e9622b8",
88
"validationUrl": "https://rp-eastus2.eventgrid.azure.net:553/eventsubscriptions/estest/validate?id=512d38b6-c7b8-40c8-89fe-f46f9e9622b6&t=2018-04-26T20:30:54.4538837Z&apiVersion=2018-05-01-preview&token=1A1A1A1A"

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33
package com.azure.spring.cloud.appconfiguration.config.implementation.properties;
44

5+
import java.net.MalformedURLException;
56
import java.net.URI;
67
import java.net.URISyntaxException;
78
import java.util.ArrayList;
@@ -20,7 +21,7 @@ public final class ConfigStore {
2021

2122
private static final String DEFAULT_KEYS = "/application/";
2223

23-
private String endpoint; // Config store endpoint
24+
private String endpoint = ""; // Config store endpoint
2425

2526
private List<String> endpoints = new ArrayList<>();
2627

@@ -165,6 +166,13 @@ public void setFeatureFlags(FeatureFlagStore featureFlags) {
165166
this.featureFlags = featureFlags;
166167
}
167168

169+
public boolean containsEndpoint(String endpoint) {
170+
if (this.endpoint.startsWith(endpoint)) {
171+
return true;
172+
}
173+
return endpoints.stream().anyMatch(storeEndpoint -> storeEndpoint.startsWith(endpoint));
174+
}
175+
168176
/**
169177
* @throws IllegalStateException Connection String URL endpoint is invalid
170178
*/
@@ -193,11 +201,11 @@ public void validateAndInit() {
193201
String endpoint = (AppConfigurationReplicaClientsBuilder.getEndpointFromConnectionString(connection));
194202
try {
195203
// new URI is used to validate the endpoint as a valid URI
196-
new URI(endpoint);
204+
new URI(endpoint).toURL();
197205
if (!StringUtils.hasText(this.endpoint)) {
198206
this.endpoint = endpoint;
199207
}
200-
} catch (URISyntaxException e) {
208+
} catch (MalformedURLException | URISyntaxException | IllegalArgumentException e) {
201209
throw new IllegalStateException("Endpoint in connection string is not a valid URI.", e);
202210
}
203211
}

‎sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/stores/ConfigStoreTest.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55

66
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
import static org.junit.jupiter.api.Assertions.assertFalse;
78
import static org.junit.jupiter.api.Assertions.assertThrows;
9+
import static org.junit.jupiter.api.Assertions.assertTrue;
810

911
import java.util.ArrayList;
1012
import java.util.List;
@@ -75,5 +77,47 @@ public void getLabelsTest() {
7577
configStore.setSelects(selects);
7678
assertEquals("\0", configStore.getSelects().get(0).getLabelFilter(new ArrayList<>())[0]);
7779
}
80+
81+
@Test
82+
public void testContainsEndpoint() {
83+
ConfigStore store = new ConfigStore();
84+
store.setEndpoint("endpoint");
85+
store.validateAndInit();
86+
assertTrue(store.containsEndpoint("endpoint"));
87+
assertFalse(store.containsEndpoint("invalidEndpoint"));
88+
89+
store = new ConfigStore();
90+
List<String> endpoints = new ArrayList<>();
91+
endpoints.add("endpoint");
92+
endpoints.add("secondEndpoint");
93+
store.setEndpoints(endpoints);
94+
store.validateAndInit();
95+
assertTrue(store.containsEndpoint("endpoint"));
96+
assertTrue(store.containsEndpoint("secondEndpoint"));
97+
assertFalse(store.containsEndpoint("invalidEndpoint"));
98+
}
99+
100+
@Test
101+
public void testValidateConnectionString() {
102+
ConfigStore store = new ConfigStore();
103+
store.setConnectionString("Endpoint=https://endpoint.io;Id=identifier;Secret=secret=");
104+
store.validateAndInit();
105+
106+
store = new ConfigStore();
107+
List<String> connectionStrings = new ArrayList<>();
108+
connectionStrings.add("Endpoint=https://endpoint.io;Id=identifier;Secret=secret=");
109+
connectionStrings.add("Endpoint=https://endpoint2.io;Id=identifier;Secret=secret=");
110+
store.setConnectionStrings(connectionStrings);
111+
store.validateAndInit();
112+
}
113+
114+
@Test
115+
public void testValidateConnectionStringInvalid() {
116+
ConfigStore store = new ConfigStore();
117+
List<String> connectionStrings = new ArrayList<>();
118+
connectionStrings.add("Endpoint=endpoint;Id=identifier;Secret=secret=");
119+
store.setConnectionStrings(connectionStrings);
120+
assertThrows(IllegalStateException.class, () -> store.validateAndInit());
121+
}
78122

79123
}

0 commit comments

Comments
 (0)