From 9dc4cf79c4cb9cf89ad4433ca252582d4d5e3940 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 18 May 2023 10:00:53 -0700 Subject: [PATCH 1/6] Fixes CredentialUnavailableException Issue --- .../AppConfigurationReplicaClient.java | 16 ++++++---- ...onFeatureManagementPropertySourceTest.java | 30 +++++++++++++++++++ .../AppConfigurationReplicaClientTest.java | 29 ++++++++++++++++++ .../implementation/ConnectionManagerTest.java | 8 +++++ 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java index 47352535f0138..c6532283f58a7 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java @@ -89,10 +89,12 @@ ConfigurationSetting getWatchKey(String key, String label) this.failedAttempts = 0; return watchKey; } catch (HttpResponseException e) { - int statusCode = e.getResponse().getStatusCode(); + if (e.getResponse() != null) { + int statusCode = e.getResponse().getStatusCode(); - if (statusCode == 429 || statusCode == 408 || statusCode >= 500) { - throw new AppConfigurationStatusException(e.getMessage(), e.getResponse(), e.getValue()); + if (statusCode == 429 || statusCode == 408 || statusCode >= 500) { + throw new AppConfigurationStatusException(e.getMessage(), e.getResponse(), e.getValue()); + } } throw e; } catch (Exception e) { // TODO (mametcal) This should be an UnknownHostException, but currently it isn't @@ -123,10 +125,12 @@ List listSettings(SettingSelector settingSelector) settings.forEach(setting -> configurationSettings.add(NormalizeNull.normalizeNullLabel(setting))); return configurationSettings; } catch (HttpResponseException e) { - int statusCode = e.getResponse().getStatusCode(); + if (e.getResponse() != null) { + int statusCode = e.getResponse().getStatusCode(); - if (statusCode == 429 || statusCode == 408 || statusCode >= 500) { - throw new AppConfigurationStatusException(e.getMessage(), e.getResponse(), e.getValue()); + if (statusCode == 429 || statusCode == 408 || statusCode >= 500) { + throw new AppConfigurationStatusException(e.getMessage(), e.getResponse(), e.getValue()); + } } throw e; } catch (Exception e) { // TODO (mametcal) This should be an UnknownHostException, but currently it isn't diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java index 163fac2aeb3e3..748be509de5d8 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java @@ -121,6 +121,36 @@ public void cleanup() throws Exception { MockitoAnnotations.openMocks(this).close(); } + @Test + public void overrideTest() { + String[] labels = {"test"}; + AppConfigurationFeatureManagementPropertySource propertySourceOverride = new AppConfigurationFeatureManagementPropertySource(TEST_STORE_NAME, clientMock, "/test/", + labels); + when(featureListMock.iterator()).thenReturn(FEATURE_ITEMS.iterator()); + when(clientMock.listSettings(Mockito.any())) + .thenReturn(featureListMock).thenReturn(featureListMock); + when(clientMock.getTracingInfo()).thenReturn(new TracingInfo(false, false, 0, Configuration.getGlobalConfiguration())); + featureFlagStore.setEnabled(true); + + propertySourceOverride.initProperties(); + + HashMap filters = new HashMap<>(); + FeatureFlagFilter ffec = new FeatureFlagFilter("TestFilter"); + filters.put(0, ffec); + Feature gamma = new Feature(); + gamma.setKey("Gamma"); + filters = new HashMap<>(); + ffec = new FeatureFlagFilter("TestFilter"); + LinkedHashMap parameters = new LinkedHashMap<>(); + parameters.put("key", "value"); + ffec.setParameters(parameters); + filters.put(0, ffec); + gamma.setEnabledFor(filters); + + assertEquals(gamma.getKey(), + ((Feature) propertySourceOverride.getProperty(FEATURE_MANAGEMENT_KEY + "Gamma")).getKey()); + } + @Test public void testFeatureFlagCanBeInitedAndQueried() { when(featureListMock.iterator()).thenReturn(FEATURE_ITEMS.iterator()); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java index f4f799ce0084b..3c3db6731a3db 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientTest.java @@ -24,6 +24,7 @@ import com.azure.data.appconfiguration.ConfigurationClient; import com.azure.data.appconfiguration.models.ConfigurationSetting; import com.azure.data.appconfiguration.models.SettingSelector; +import com.azure.identity.CredentialUnavailableException; import com.azure.spring.cloud.appconfiguration.config.implementation.http.policy.TracingInfo; public class AppConfigurationReplicaClientTest { @@ -101,6 +102,34 @@ public void listSettingsTest() { assertThrows(HttpResponseException.class, () -> client.listSettings(new SettingSelector())); } + @Test + public void listSettingsNoCredentialTest() { + AppConfigurationReplicaClient client = new AppConfigurationReplicaClient(endpoint, clientMock, + new TracingInfo(false, false, 0, Configuration.getGlobalConfiguration())); + + List configurations = new ArrayList<>(); + + when(clientMock.listConfigurationSettings(Mockito.any())) + .thenThrow(new CredentialUnavailableException("No Credential")); + when(settingsMock.iterator()).thenReturn(configurations.iterator()); + + assertThrows(CredentialUnavailableException.class, () -> client.listSettings(new SettingSelector())); + } + + @Test + public void getWatchNoCredentialTest() { + AppConfigurationReplicaClient client = new AppConfigurationReplicaClient(endpoint, clientMock, + new TracingInfo(false, false, 0, Configuration.getGlobalConfiguration())); + + List configurations = new ArrayList<>(); + + when(clientMock.getConfigurationSetting(Mockito.anyString(), Mockito.anyString())) + .thenThrow(new CredentialUnavailableException("No Credential")); + when(settingsMock.iterator()).thenReturn(configurations.iterator()); + + assertThrows(CredentialUnavailableException.class, () -> client.getWatchKey("key", "label")); + } + @Test public void backoffTest() { AppConfigurationReplicaClient client = new AppConfigurationReplicaClient(endpoint, clientMock, diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManagerTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManagerTest.java index 0f2e2279cfcc6..2368133b2e45e 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManagerTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManagerTest.java @@ -153,4 +153,12 @@ public void updateSyncTokenTest() { verify(replicaClient1, times(1)).updateSyncToken(Mockito.eq(fakeToken)); } + + @Test + public void getAvailableClientsNotLoadedTest() { + ConnectionManager manager = new ConnectionManager(clientBuilderMock, configStore); + + assertEquals(0, manager.getAvailableClients().size()); + assertEquals(AppConfigurationStoreHealth.NOT_LOADED, manager.getHealth()); + } } From dd8ca2e0b495c06817407c9db739198df4a831a0 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 22 May 2023 11:50:38 -0700 Subject: [PATCH 2/6] Update HostType.java --- .../cloud/appconfiguration/config/implementation/HostType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/HostType.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/HostType.java index 1b7e9c0fe738f..6465186dbeccd 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/HostType.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/HostType.java @@ -30,7 +30,7 @@ public enum HostType { /** * Host is Container App */ - CONTAINER_APP("ContainerApps"); + CONTAINER_APP("ContainerApp"); private final String text; From df1dafb435bca7b5344a2cf95019a0e07cb1ccc5 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 22 May 2023 13:05:13 -0700 Subject: [PATCH 3/6] Update CHANGELOG.md --- .../spring-cloud-azure-appconfiguration-config/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/CHANGELOG.md b/sdk/spring/spring-cloud-azure-appconfiguration-config/CHANGELOG.md index 56eadd118f358..51dccfc2c997d 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/CHANGELOG.md +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/CHANGELOG.md @@ -9,6 +9,8 @@ ### Bugs Fixed - Fixes issue where credential from Azure Spring global properties was being overridden. +- Fixes bug where Http Response wasn't checked before trying to use response. +- Fixes Tracing info for ContainerApp ### Other Changes From a51f125896c9b4500e3587b7cba5d722eedd4509 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 22 May 2023 19:32:29 -0700 Subject: [PATCH 4/6] Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java Co-authored-by: Xiaolu Dai <31124698+saragluna@users.noreply.github.com> --- .../AppConfigurationFeatureManagementPropertySourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java index 748be509de5d8..a9b0a4c1debe6 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java @@ -134,7 +134,7 @@ public void overrideTest() { propertySourceOverride.initProperties(); - HashMap filters = new HashMap<>(); + Map filters = new HashMap<>(); FeatureFlagFilter ffec = new FeatureFlagFilter("TestFilter"); filters.put(0, ffec); Feature gamma = new Feature(); From 1ec0b1e1b3d9286274013ec8087783d23cd14a38 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 22 May 2023 19:32:38 -0700 Subject: [PATCH 5/6] Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java Co-authored-by: Xiaolu Dai <31124698+saragluna@users.noreply.github.com> --- .../AppConfigurationFeatureManagementPropertySourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java index a9b0a4c1debe6..97ad73ac3210c 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java @@ -141,7 +141,7 @@ public void overrideTest() { gamma.setKey("Gamma"); filters = new HashMap<>(); ffec = new FeatureFlagFilter("TestFilter"); - LinkedHashMap parameters = new LinkedHashMap<>(); + Map parameters = new LinkedHashMap<>(); parameters.put("key", "value"); ffec.setParameters(parameters); filters.put(0, ffec); From ab0765249d68f785d9d040a96f0a886b47f82b97 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 23 May 2023 16:51:23 -0700 Subject: [PATCH 6/6] Update AppConfigurationFeatureManagementPropertySourceTest.java --- .../AppConfigurationFeatureManagementPropertySourceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java index 97ad73ac3210c..ca28db709181d 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySourceTest.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll;