From 60f4093e72fc95ffa610b203afc94d6454adab6a Mon Sep 17 00:00:00 2001 From: Moary Chen Date: Fri, 6 Sep 2024 13:33:30 +0800 Subject: [PATCH] Fix to support multiple property sources auto refresh (#41703) --- sdk/spring/CHANGELOG.md | 1 + .../KeyVaultEnvironmentPostProcessor.java | 32 ++- .../environment/KeyVaultOperation.java | 185 ++------------- .../environment/KeyVaultPropertySource.java | 161 ++++++++++++- ...AzureKeyVaultPropertySourceProperties.java | 2 +- ...KeyVaultEnvironmentPostProcessorTests.java | 10 + .../environment/KeyVaultOperationTests.java | 216 ++++-------------- .../KeyVaultPropertySourceTests.java | 139 ++++++++++- .../KeyVaultSecretClientMockUtils.java | 39 ++++ .../keyvault/environment/OnePageResponse.java | 53 +++++ 10 files changed, 476 insertions(+), 362 deletions(-) create mode 100644 sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultSecretClientMockUtils.java create mode 100644 sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/OnePageResponse.java diff --git a/sdk/spring/CHANGELOG.md b/sdk/spring/CHANGELOG.md index ff70f39eb09cc..aa1f22f86db1b 100644 --- a/sdk/spring/CHANGELOG.md +++ b/sdk/spring/CHANGELOG.md @@ -13,6 +13,7 @@ This section includes changes in `spring-cloud-azure-autoconfigure` module. #### Bugs Fixed - Avoid always overriding the default binder when using Kafka binder. [#37337](https://github.com/Azure/azure-sdk-for-java/issues/37337). +- Fix to support multiple property sources auto refresh. [#26356](https://github.com/Azure/azure-sdk-for-java/issues/26356). ## 5.15.0 (2024-08-07) - This release is compatible with Spring Boot 3.0.0-3.0.13, 3.1.0-3.1.12, 3.2.0-3.2.7, 3.3.0-3.3.2. (Note: 3.0.x (x>13), 3.1.y (y>12), 3.2.z (z>7) and 3.3.m (m>2) should be supported, but they aren't tested with this release.) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java index 8d2c3744a5ff1..734fffc132118 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessor.java @@ -27,7 +27,9 @@ import org.springframework.util.StringUtils; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static org.springframework.core.env.StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME; @@ -47,8 +49,8 @@ public class KeyVaultEnvironmentPostProcessor implements EnvironmentPostProcesso private static final String SKIP_CONFIGURE_REASON_FORMAT = "Skip configuring Key Vault PropertySource because %s."; private final Log logger; - private final ConfigurableBootstrapContext bootstrapContext; + private final ConfigurableBootstrapContext bootstrapContext; /** * Creates a new instance of {@link KeyVaultEnvironmentPostProcessor}. @@ -85,6 +87,9 @@ public void postProcessEnvironment(ConfigurableEnvironment environment, SpringAp } final List propertiesList = secretProperties.getPropertySources(); + + checkDuplicatePropertySourceNames(propertiesList); + List keyVaultPropertySources = buildKeyVaultPropertySourceList(propertiesList); final MutablePropertySources propertySources = environment.getPropertySources(); // reverse iterate order making sure smaller index has higher priority. @@ -99,6 +104,16 @@ public void postProcessEnvironment(ConfigurableEnvironment environment, SpringAp } } + private void checkDuplicatePropertySourceNames(List propertiesList) { + List sourceNames = propertiesList.stream() + .map(AzureKeyVaultPropertySourceProperties::getName) + .toList(); + Set deduplicatedSourceNames = new HashSet<>(sourceNames); + if (propertiesList.size() != deduplicatedSourceNames.size()) { + throw new IllegalStateException("Duplicate property source name found: " + sourceNames); + } + } + private List buildKeyVaultPropertySourceList( List propertiesList) { List propertySources = new ArrayList<>(); @@ -120,14 +135,15 @@ private List buildKeyVaultPropertySourceList( private KeyVaultPropertySource buildKeyVaultPropertySource( AzureKeyVaultPropertySourceProperties properties) { try { - final KeyVaultOperation keyVaultOperation = new KeyVaultOperation( - buildSecretClient(properties), - properties.getRefreshInterval(), - properties.getSecretKeys(), - properties.isCaseSensitive()); - return new KeyVaultPropertySource(properties.getName(), keyVaultOperation); + final KeyVaultOperation keyVaultOperation = new KeyVaultOperation(buildSecretClient(properties)); + return new KeyVaultPropertySource( + properties.getName(), + properties.getRefreshInterval(), + keyVaultOperation, + properties.getSecretKeys(), + properties.isCaseSensitive()); } catch (final Exception exception) { - throw new IllegalStateException("Failed to configure KeyVault property source", exception); + throw new IllegalStateException("Failed to configure KeyVault property source '" + properties.getName() + "'", exception); } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperation.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperation.java index 580435e3e99d7..a720964c1d680 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperation.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperation.java @@ -8,199 +8,58 @@ import com.azure.security.keyvault.secrets.SecretClient; import com.azure.security.keyvault.secrets.models.KeyVaultSecret; import com.azure.security.keyvault.secrets.models.SecretProperties; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.lang.NonNull; -import java.time.Duration; -import java.util.HashMap; import java.util.List; -import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Timer; -import java.util.TimerTask; -import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; /** * KeyVaultOperation wraps the operations to access Key Vault. + * This operation can list secrets with given filter, and convert it to spring property name and value. * * @since 4.0.0 */ public class KeyVaultOperation { - private static final Logger LOGGER = LoggerFactory.getLogger(KeyVaultOperation.class); - - /** - * Stores the case-sensitive flag. - */ - private final boolean caseSensitive; - - /** - * Stores the properties. - */ - private Map properties = new HashMap<>(); - /** * Stores the secret client. */ private final SecretClient secretClient; - /** - * Stores the secret keys. - */ - private final List secretKeys; - /** - * Stores the timer object to schedule refresh task. - */ - private static Timer timer; - /** * Constructor. * @param secretClient the Key Vault secret client. - * @param refreshDuration the refresh in milliseconds (0 or less disables refresh). - * @param secretKeys the secret keys to look for. - * @param caseSensitive the case-sensitive flag. */ - public KeyVaultOperation(final SecretClient secretClient, - final Duration refreshDuration, - List secretKeys, - boolean caseSensitive) { - - this.caseSensitive = caseSensitive; + public KeyVaultOperation(final SecretClient secretClient) { this.secretClient = secretClient; - this.secretKeys = secretKeys; - - refreshProperties(); - - final long refreshInMillis = refreshDuration.toMillis(); - if (refreshInMillis > 0) { - synchronized (KeyVaultOperation.class) { - if (timer != null) { - try { - timer.cancel(); - timer.purge(); - } catch (RuntimeException runtimeException) { - LOGGER.error("Error of terminating Timer", runtimeException); - } - } - timer = new Timer(true); - final TimerTask task = new TimerTask() { - @Override - public void run() { - refreshProperties(); - } - }; - timer.scheduleAtFixedRate(task, refreshInMillis, refreshInMillis); - } - } } /** - * Get the property. - * - * @param property the property to get. - * @return the property value. + * Get the Key Vault secrets filtered by given secret keys. + * If the secret keys is empty, return all the secrets in Key Vault. */ - public String getProperty(String property) { - return properties.get(toKeyVaultSecretName(property)); - } - - /** - * Get the property names. - * - * @return the property names. - */ - public String[] getPropertyNames() { - if (!caseSensitive) { - return properties - .keySet() - .stream() - .flatMap(p -> Stream.of(p, p.replace("-", "."))) - .distinct() - .toArray(String[]::new); - } else { - return properties - .keySet() - .toArray(new String[0]); - } - } - - /** - * Refresh the properties by accessing key vault. - */ - private void refreshProperties() { + List listSecrets(List secretKeys) { + List keyVaultSecrets; if (secretKeys == null || secretKeys.isEmpty()) { - properties = Optional.of(secretClient) - .map(SecretClient::listPropertiesOfSecrets) - .map(ContinuablePagedIterable::iterableByPage) - .map(i -> StreamSupport.stream(i.spliterator(), false)) - .orElseGet(Stream::empty) - .map(PagedResponse::getElements) - .flatMap(i -> StreamSupport.stream(i.spliterator(), false)) - .filter(SecretProperties::isEnabled) - .map(p -> secretClient.getSecret(p.getName(), p.getVersion())) - .filter(Objects::nonNull) - .collect(Collectors.toMap( - s -> toKeyVaultSecretName(s.getName()), - KeyVaultSecret::getValue - )); - } else { - properties = secretKeys.stream() - .map(this::toKeyVaultSecretName) - .map(secretClient::getSecret) - .filter(Objects::nonNull) - .collect(Collectors.toMap( - s -> toKeyVaultSecretName(s.getName()), - KeyVaultSecret::getValue - )); - } - } - - /** - * For convention, we need to support all relaxed binding format from spring, these may include: - * - * - * - * - * - * - *
Spring relaxed binding names
acme.my-project.person.first-name
acme.myProject.person.firstName
acme.my_project.person.first_name
ACME_MYPROJECT_PERSON_FIRSTNAME
- * But azure key vault only allows ^[0-9a-zA-Z-]+$ and case-insensitive, so - * there must be some conversion between spring names and azure key vault - * names. For example, the 4 properties stated above should be converted to - * acme-myproject-person-firstname in key vault. - * - * @param property of secret instance. - * @return the value of secret with given name or null. - */ - private String toKeyVaultSecretName(@NonNull String property) { - if (!caseSensitive) { - if (property.matches("[a-z0-9A-Z-]+")) { - return property.toLowerCase(Locale.US); - } else if (property.matches("[A-Z0-9_]+")) { - return property.toLowerCase(Locale.US).replace("_", "-"); - } else { - return property.toLowerCase(Locale.US) - .replace("-", "") // my-project -> myproject - .replace("_", "") // my_project -> myproject - .replace(".", "-"); // acme.myproject -> acme-myproject - } + keyVaultSecrets = Optional.of(secretClient) + .map(SecretClient::listPropertiesOfSecrets) + .map(ContinuablePagedIterable::iterableByPage) + .map(i -> StreamSupport.stream(i.spliterator(), false)) + .orElseGet(Stream::empty) + .map(PagedResponse::getElements) + .flatMap(i -> StreamSupport.stream(i.spliterator(), false)) + .filter(SecretProperties::isEnabled) + .map(p -> secretClient.getSecret(p.getName(), p.getVersion())) + .filter(Objects::nonNull) + .toList(); } else { - return property; + keyVaultSecrets = secretKeys.stream() + .map(secretClient::getSecret) + .filter(Objects::nonNull) + .toList(); } + return keyVaultSecrets; } - - /** - * Set the properties. - * - * @param properties the properties. - */ - void setProperties(HashMap properties) { - this.properties = properties; - } - } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySource.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySource.java index 6763740aa8655..da693d288af1d 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySource.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySource.java @@ -3,7 +3,20 @@ package com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment; +import com.azure.security.keyvault.secrets.models.KeyVaultSecret; import org.springframework.core.env.EnumerablePropertySource; +import org.springframework.lang.NonNull; + +import java.time.Duration; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** @@ -13,26 +26,94 @@ */ public class KeyVaultPropertySource extends EnumerablePropertySource { - private final KeyVaultOperation operations; + /** + * Stores the properties. + */ + private Map properties = new HashMap<>(); + + private final KeyVaultOperation keyVaultOperation; + + /** + * Stores the case-sensitive flag. + */ + private final boolean caseSensitive; + + /** + * Stores the secret keys. + */ + private final List secretKeys; + + private final List keyVaultSecretKeys; + + /** + * Stores the timer objects to schedule refresh task. + */ + private static final ConcurrentHashMap TIMER_MANAGER = new ConcurrentHashMap<>(); /** * Create a new {@code KeyVaultPropertySource} with the given name and {@link KeyVaultOperation}. - * @param name the associated name - * @param operation the {@link KeyVaultOperation} + * @param name the associated name. + * @param refreshDuration the refresh in milliseconds (0 or less disables refresh). + * @param keyVaultOperation the {@link KeyVaultOperation}. + * @param secretKeys the secret keys to look for. + * @param caseSensitive the case-sensitive flag. */ - public KeyVaultPropertySource(String name, KeyVaultOperation operation) { - super(name, operation); - this.operations = operation; + public KeyVaultPropertySource(String name, + final Duration refreshDuration, + KeyVaultOperation keyVaultOperation, + List secretKeys, + boolean caseSensitive) { + super(name, keyVaultOperation); + this.caseSensitive = caseSensitive; + this.secretKeys = secretKeys; + this.keyVaultSecretKeys = convertToKeyVaultSecretNames(secretKeys, caseSensitive); + this.keyVaultOperation = keyVaultOperation; + loadProperties(); + enablePropertiesAutoRefresh(refreshDuration); } - @Override - public String[] getPropertyNames() { - return this.operations.getPropertyNames(); + void loadProperties() { + logger.debug("Loading the secrets in property source '" + name + "'."); + properties = keyVaultOperation.listSecrets(this.keyVaultSecretKeys) + .stream() + .collect(Collectors.toMap( + s -> toKeyVaultSecretName(caseSensitive, s.getName()), + KeyVaultSecret::getValue) + ); + logger.debug("The secrets loading in property source '" + name + "' has finished."); + } + + void enablePropertiesAutoRefresh(Duration refreshDuration) { + final long refreshInMillis = refreshDuration.toMillis(); + if (refreshInMillis > 0) { + synchronized (KeyVaultPropertySource.class) { + Timer timer; + if (TIMER_MANAGER.containsKey(name)) { + timer = TIMER_MANAGER.get(name); + try { + timer.cancel(); + timer.purge(); + } catch (RuntimeException runtimeException) { + logger.error("Error of terminating Timer", runtimeException); + } + } + timer = new Timer(name, true); + TIMER_MANAGER.put(name, timer); + + final TimerTask task = new TimerTask() { + @Override + public void run() { + loadProperties(); + } + }; + timer.scheduleAtFixedRate(task, refreshInMillis, refreshInMillis); + } + } } @Override - public Object getProperty(String name) { - return operations.getProperty(name); + public String getProperty(String property) { + return properties.get(toKeyVaultSecretName(caseSensitive, property)); } @Override @@ -40,4 +121,62 @@ public boolean containsProperty(String name) { return getProperty(name) != null; } + @Override + public String[] getPropertyNames() { + if (!caseSensitive) { + return properties + .keySet() + .stream() + .flatMap(p -> Stream.of(p, p.replace("-", "."))) + .distinct() + .toArray(String[]::new); + } else { + return properties + .keySet() + .toArray(new String[0]); + } + } + + private static List convertToKeyVaultSecretNames(List secretKeys, boolean caseSensitive) { + if (secretKeys == null) { + return null; + } + return secretKeys.stream() + .map(key -> toKeyVaultSecretName(caseSensitive, key)) + .toList(); + } + + /** + * For convention, we need to support all relaxed binding format from spring, these may include: + * + * + * + * + * + * + *
Spring relaxed binding names
acme.my-project.person.first-name
acme.myProject.person.firstName
acme.my_project.person.first_name
ACME_MYPROJECT_PERSON_FIRSTNAME
+ * But azure key vault only allows ^[0-9a-zA-Z-]+$ and case-insensitive, so + * there must be some conversion between spring names and azure key vault + * names. For example, the 4 properties stated above should be converted to + * acme-myproject-person-firstname in key vault. + * @param caseSensitive the case-sensitive flag. + * @param property of secret instance. + * @return the value of secret with given name or null. + */ + public static String toKeyVaultSecretName(boolean caseSensitive, @NonNull String property) { + if (!caseSensitive) { + if (property.matches("[a-z0-9A-Z-]+")) { + return property.toLowerCase(Locale.US); + } else if (property.matches("[A-Z0-9_]+")) { + return property.toLowerCase(Locale.US).replace("_", "-"); + } else { + return property.toLowerCase(Locale.US) + .replace("-", "") // my-project -> myproject + .replace("_", "") // my_project -> myproject + .replace(".", "-"); // acme.myproject -> acme-myproject + } + } else { + return property; + } + } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java index dd7202bd53dce..94bbbbb4a9aa1 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java @@ -27,7 +27,7 @@ public class AzureKeyVaultPropertySourceProperties extends AbstractAzureHttpConf */ private SecretServiceVersion serviceVersion; /** - * Name of this property source. + * Name of this property source. Each name must be unique. */ private String name; /** diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java index 2db4a7a93438a..29f223be86b62 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultEnvironmentPostProcessorTests.java @@ -205,6 +205,16 @@ void defaultPropertySourceNameTest() { assertTrue(propertySources.contains(processor.buildPropertySourceName(1))); } + @Test + void duplicatePropertySourceNameTest() { + environment.setProperty("spring.cloud.azure.keyvault.secret.property-source-enabled", "true"); + environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].name", "test"); + environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[0].endpoint", ENDPOINT_0); + environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[1].name", "test"); + environment.setProperty("spring.cloud.azure.keyvault.secret.property-sources[1].endpoint", ENDPOINT_1); + assertThrows(IllegalStateException.class, () -> processor.postProcessEnvironment(environment, application)); + } + @Test void keyVaultPropertySourceHasHighestPriorityIfEnvironmentPropertySourceNotExistTest() { environment.setProperty("spring.cloud.azure.keyvault.secret.property-source-enabled", "true"); diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperationTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperationTests.java index d337025b07ff3..0cce66b42dcdc 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperationTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultOperationTests.java @@ -3,34 +3,20 @@ package com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment; -import com.azure.core.http.HttpHeaders; -import com.azure.core.http.HttpRequest; -import com.azure.core.http.rest.PagedFlux; -import com.azure.core.http.rest.PagedIterable; -import com.azure.core.http.rest.PagedResponse; -import com.azure.core.util.IterableStream; import com.azure.security.keyvault.secrets.SecretClient; import com.azure.security.keyvault.secrets.models.KeyVaultSecret; -import com.azure.security.keyvault.secrets.models.SecretProperties; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import java.io.IOException; -import java.time.Duration; -import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.List; -import static com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultPropertySourceProperties.DEFAULT_REFRESH_INTERVAL; -import static org.assertj.core.api.Assertions.assertThat; +import static com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment.KeyVaultSecretClientMockUtils.mockSecretClientGetSecretMethod; +import static com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment.KeyVaultSecretClientMockUtils.mockSecretClientListPropertiesOfSecrets; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.when; public class KeyVaultOperationTests { @@ -40,25 +26,10 @@ public class KeyVaultOperationTests { private static final String SECRET_KEY_1 = "key1"; - private static final String TEST_SPRING_RELAXED_BINDING_NAME_0 = "acme.my-project.person.first-name"; - - private static final String TEST_SPRING_RELAXED_BINDING_NAME_1 = "acme.myProject.person.firstName"; - - private static final String TEST_SPRING_RELAXED_BINDING_NAME_2 = "acme.my_project.person.first_name"; - - private static final String TEST_SPRING_RELAXED_BINDING_NAME_3 = "ACME_MYPROJECT_PERSON_FIRSTNAME"; - - private static final String TEST_AZURE_KEYVAULT_NAME = "acme-myproject-person-firstname"; - - private static final List TEST_SPRING_RELAXED_BINDING_NAMES = Arrays.asList( - TEST_SPRING_RELAXED_BINDING_NAME_0, - TEST_SPRING_RELAXED_BINDING_NAME_1, - TEST_SPRING_RELAXED_BINDING_NAME_2, - TEST_SPRING_RELAXED_BINDING_NAME_3 - ); + private static final String SECRET_VALUE_1 = "value1"; @Mock - private SecretClient keyVaultClient; + private SecretClient secretClient; private KeyVaultOperation keyVaultOperation; @@ -67,6 +38,7 @@ public class KeyVaultOperationTests { @BeforeEach public void setup() { closeable = MockitoAnnotations.openMocks(this); + keyVaultOperation = new KeyVaultOperation(secretClient); } @AfterEach @@ -74,166 +46,74 @@ public void close() throws Exception { closeable.close(); } - public void setupSecretBundle(List secretKeysConfig) { - keyVaultOperation = new KeyVaultOperation(keyVaultClient, Duration.ZERO, secretKeysConfig, false); - } - @Test public void caseSensitive() { - final KeyVaultOperation keyOperation = new KeyVaultOperation( - keyVaultClient, - DEFAULT_REFRESH_INTERVAL, - new ArrayList<>(), - true); - final LinkedHashMap properties = new LinkedHashMap<>(); - properties.put("key1", "value1"); - properties.put("Key2", "Value2"); - keyOperation.setProperties(properties); - - assertEquals("value1", keyOperation.getProperty("key1")); - assertEquals("Value2", keyOperation.getProperty("Key2")); + String key1 = "KEY1"; + String value1 = "value1"; + String key2 = "Key2"; + String value2 = "value2"; + KeyVaultSecret keyVaultSecret1 = mockSecretClientGetSecretMethod(secretClient, key1, value1); + KeyVaultSecret keyVaultSecret2 = mockSecretClientGetSecretMethod(secretClient, key2, value2); + mockSecretClientListPropertiesOfSecrets(secretClient, keyVaultSecret1.getProperties(), keyVaultSecret2.getProperties()); + + List keyVaultSecrets = keyVaultOperation.listSecrets(null); + + assertEquals(value1, keyVaultSecrets.get(0).getValue()); + assertEquals(key1, keyVaultSecrets.get(0).getName()); + assertEquals(value2, keyVaultSecrets.get(1).getValue()); + assertEquals(key2, keyVaultSecrets.get(1).getName()); } @Test public void testGetWithNoSpecificSecretKeys() { - setupSecretBundle(null); + KeyVaultSecret keyVaultSecret = mockSecretClientGetSecretMethod(secretClient, SECRET_KEY_1, SECRET_VALUE_1); + mockSecretClientListPropertiesOfSecrets(secretClient, keyVaultSecret.getProperties()); - final LinkedHashMap properties = new LinkedHashMap<>(); - properties.put("testpropertyname1", TEST_PROPERTY_NAME_1); - keyVaultOperation.setProperties(properties); + List keyVaultSecrets = keyVaultOperation.listSecrets(null); - assertThat(keyVaultOperation.getProperty(TEST_PROPERTY_NAME_1)).isEqualToIgnoringCase(TEST_PROPERTY_NAME_1); + assertEquals(1, keyVaultSecrets.size()); + assertEquals(SECRET_KEY_1, keyVaultSecrets.get(0).getName()); + assertEquals(SECRET_VALUE_1, keyVaultSecrets.get(0).getValue()); } @Test public void testGetAndMissWhenSecretsProvided() { - setupSecretBundle(SECRET_KEYS_CONFIG); + mockSecretClientGetSecretMethod(secretClient, "key1", "value1"); + mockSecretClientGetSecretMethod(secretClient, "key2", "value2"); + mockSecretClientGetSecretMethod(secretClient, "key3", "value3"); - final LinkedHashMap properties = new LinkedHashMap<>(); - properties.put("key1", "value1"); - properties.put("key2", "value2"); - properties.put("key3", "value3"); - keyVaultOperation.setProperties(properties); + List keyVaultSecrets = keyVaultOperation.listSecrets(List.of("key1", "key2", "key3")); - assertThat(keyVaultOperation.getProperty(TEST_PROPERTY_NAME_1)).isEqualToIgnoringCase(null); + assertEquals(3, keyVaultSecrets.size()); + assertEquals("key1", keyVaultSecrets.get(0).getName()); + assertEquals("key2", keyVaultSecrets.get(1).getName()); + assertEquals("key3", keyVaultSecrets.get(2).getName()); } @Test public void testGetAndHitWhenSecretsProvided() { - when(keyVaultClient.getSecret("key1")).thenReturn(new KeyVaultSecret("key1", "key1")); - when(keyVaultClient.getSecret("key2")).thenReturn(new KeyVaultSecret("key2", "key2")); - when(keyVaultClient.getSecret("key3")).thenReturn(new KeyVaultSecret("key3", "key3")); - - setupSecretBundle(SECRET_KEYS_CONFIG); - - assertThat(keyVaultOperation.getProperty(SECRET_KEY_1)).isEqualToIgnoringCase(SECRET_KEY_1); - } + mockSecretClientGetSecretMethod(secretClient, SECRET_KEY_1, SECRET_VALUE_1); + mockSecretClientGetSecretMethod(secretClient, "key2", "value2"); + mockSecretClientGetSecretMethod(secretClient, "key3", "value3"); - @Test - public void testList() { - //test list with no specific secret keys - setupSecretBundle(null); - final LinkedHashMap properties = new LinkedHashMap<>(); - properties.put(TEST_PROPERTY_NAME_1, TEST_PROPERTY_NAME_1); - keyVaultOperation.setProperties(properties); - final String[] result = keyVaultOperation.getPropertyNames(); - assertThat(result.length).isEqualTo(1); - assertThat(result[0]).isEqualToIgnoringCase(TEST_PROPERTY_NAME_1); - - //test list with specific secret key configs - when(keyVaultClient.getSecret("key1")).thenReturn(new KeyVaultSecret("key1", "key1")); - when(keyVaultClient.getSecret("key2")).thenReturn(new KeyVaultSecret("key2", "key2")); - when(keyVaultClient.getSecret("key3")).thenReturn(new KeyVaultSecret("key3", "key3")); - setupSecretBundle(SECRET_KEYS_CONFIG); - final String[] specificResult = keyVaultOperation.getPropertyNames(); - assertThat(specificResult.length).isEqualTo(3); - assertThat(specificResult[0]).isEqualTo(SECRET_KEYS_CONFIG.get(0)); - } + List keyVaultSecrets = keyVaultOperation.listSecrets(SECRET_KEYS_CONFIG); - @Test - public void setTestSpringRelaxedBindingNames() { - //test list with no specific secret keys - setupSecretBundle(null); - LinkedHashMap properties = new LinkedHashMap<>(); - properties.put("acme-myproject-person-firstname", TEST_AZURE_KEYVAULT_NAME); - keyVaultOperation.setProperties(properties); - TEST_SPRING_RELAXED_BINDING_NAMES - .forEach(n -> assertThat(keyVaultOperation.getProperty(n)).isEqualTo(TEST_AZURE_KEYVAULT_NAME)); - - //test list with specific secret key configs - setupSecretBundle(Arrays.asList(TEST_AZURE_KEYVAULT_NAME)); - properties = new LinkedHashMap<>(); - properties.put(TEST_AZURE_KEYVAULT_NAME, TEST_AZURE_KEYVAULT_NAME); - keyVaultOperation.setProperties(properties); - TEST_SPRING_RELAXED_BINDING_NAMES - .forEach(n -> assertThat(keyVaultOperation.getProperty(n)).isEqualTo(TEST_AZURE_KEYVAULT_NAME)); - - setupSecretBundle(SECRET_KEYS_CONFIG); - properties = new LinkedHashMap<>(); - properties.put("key1", "key1"); - properties.put("key2", "key2"); - properties.put("key3", "key3"); - keyVaultOperation.setProperties(properties); - TEST_SPRING_RELAXED_BINDING_NAMES.forEach(n -> assertThat(keyVaultOperation.getProperty(n)).isEqualTo(null)); + assertEquals(3, keyVaultSecrets.size()); + assertEquals(SECRET_KEY_1, keyVaultSecrets.get(0).getName()); + assertEquals(SECRET_VALUE_1, keyVaultSecrets.get(0).getValue()); } @Test public void getSecretsWithoutDisabled() { - KeyVaultSecret enableSecret = new KeyVaultSecret("key1", "value1"); - enableSecret.getProperties().setEnabled(true); - - KeyVaultSecret disableSecret = new KeyVaultSecret("key2", "value2"); - disableSecret.getProperties().setEnabled(false); - - List properties = Arrays.asList(enableSecret.getProperties(), disableSecret.getProperties()); - OnePageResponse secretResponse = new OnePageResponse<>(properties); - when(keyVaultClient.getSecret("key1", null)).thenReturn(enableSecret); - when(keyVaultClient.listPropertiesOfSecrets()) - .thenReturn(new PagedIterable<>(new PagedFlux<>(() -> Mono.just(secretResponse)))); - setupSecretBundle(null); - assertThat(keyVaultOperation.getPropertyNames().length == 1); - assertThat(keyVaultOperation.getProperty("key1")).isNotNull(); - assertThat(keyVaultOperation.getProperty("key2")).isNull(); - - } - - static class OnePageResponse implements PagedResponse { - - List properties = null; - - OnePageResponse(List properties) { - this.properties = properties; - } + KeyVaultSecret enabledSecret = mockSecretClientGetSecretMethod(secretClient, "key1", "value1"); + KeyVaultSecret disabledSecret = mockSecretClientGetSecretMethod(secretClient, "key2", "value2", false); + mockSecretClientListPropertiesOfSecrets(secretClient, enabledSecret.getProperties(), disabledSecret.getProperties()); - @Override - public IterableStream getElements() { - Flux flux = Flux.fromIterable(properties); - return new IterableStream(flux); - } + List keyVaultSecrets = keyVaultOperation.listSecrets(null); - @Override - public String getContinuationToken() { - return null; - } - - @Override - public int getStatusCode() { - return 0; - } - - @Override - public HttpHeaders getHeaders() { - return null; - } - - @Override - public HttpRequest getRequest() { - return null; - } - - @Override - public void close() throws IOException { - - } + assertEquals(1, keyVaultSecrets.size()); + assertEquals("key1", keyVaultSecrets.get(0).getName()); + assertEquals("value1", keyVaultSecrets.get(0).getValue()); } + } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySourceTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySourceTests.java index e3a15f3a804a0..14b6f2504431f 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySourceTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultPropertySourceTests.java @@ -4,14 +4,26 @@ package com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment; +import com.azure.security.keyvault.secrets.SecretClient; +import com.azure.security.keyvault.secrets.models.KeyVaultSecret; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.time.Duration; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment.KeyVaultSecretClientMockUtils.mockSecretClientGetSecretMethod; +import static com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment.KeyVaultSecretClientMockUtils.mockSecretClientListPropertiesOfSecrets; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.anyString; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -19,23 +31,34 @@ public class KeyVaultPropertySourceTests { private static final String TEST_PROPERTY_NAME_1 = "testPropertyName1"; + private static final String TEST_PROPERTY_VALUE_1 = "testPropertyValue1"; + private AutoCloseable closeable; @Mock KeyVaultOperation keyVaultOperation; - KeyVaultPropertySource keyVaultPropertySource; + private static final String TEST_SPRING_RELAXED_BINDING_NAME_0 = "acme.my-project.person.first-name"; - @BeforeEach - public void setup() { - closeable = MockitoAnnotations.openMocks(this); + private static final String TEST_SPRING_RELAXED_BINDING_NAME_1 = "acme.myProject.person.firstName"; + + private static final String TEST_SPRING_RELAXED_BINDING_NAME_2 = "acme.my_project.person.first_name"; - final String[] propertyNameList = new String[]{TEST_PROPERTY_NAME_1}; + private static final String TEST_SPRING_RELAXED_BINDING_NAME_3 = "ACME_MYPROJECT_PERSON_FIRSTNAME"; - when(keyVaultOperation.getProperty(anyString())).thenReturn(TEST_PROPERTY_NAME_1); - when(keyVaultOperation.getPropertyNames()).thenReturn(propertyNameList); + private static final String TEST_AZURE_KEYVAULT_NAME = "acme-myproject-person-firstname"; + private static final String TEST_AZURE_KEYVAULT_VALUE = "testValue"; - keyVaultPropertySource = new KeyVaultPropertySource("azure-key-vault-secret-property-source", keyVaultOperation); + private static final List TEST_SPRING_RELAXED_BINDING_NAMES = Arrays.asList( + TEST_SPRING_RELAXED_BINDING_NAME_0, + TEST_SPRING_RELAXED_BINDING_NAME_1, + TEST_SPRING_RELAXED_BINDING_NAME_2, + TEST_SPRING_RELAXED_BINDING_NAME_3 + ); + + @BeforeEach + public void setup() { + closeable = MockitoAnnotations.openMocks(this); } @AfterEach @@ -45,6 +68,10 @@ public void close() throws Exception { @Test public void testGetPropertyNames() { + KeyVaultSecret keyVaultSecret = new KeyVaultSecret(TEST_PROPERTY_NAME_1, TEST_PROPERTY_VALUE_1); + when(keyVaultOperation.listSecrets(null)).thenReturn(List.of(keyVaultSecret)); + + KeyVaultPropertySource keyVaultPropertySource = new KeyVaultPropertySource("azure-key-vault-secret-property-source", Duration.ZERO, keyVaultOperation, null, true); final String[] result = keyVaultPropertySource.getPropertyNames(); assertThat(result.length).isEqualTo(1); @@ -53,7 +80,97 @@ public void testGetPropertyNames() { @Test public void testGetProperty() { - final String result = (String) keyVaultPropertySource.getProperty(TEST_PROPERTY_NAME_1); - assertThat(result).isEqualTo(TEST_PROPERTY_NAME_1); + KeyVaultSecret keyVaultSecret = new KeyVaultSecret(TEST_PROPERTY_NAME_1, TEST_PROPERTY_VALUE_1); + when(keyVaultOperation.listSecrets(null)).thenReturn(List.of(keyVaultSecret)); + + KeyVaultPropertySource keyVaultPropertySource = new KeyVaultPropertySource("azure-key-vault-secret-property-source", Duration.ZERO, keyVaultOperation, null, true); + final String result = keyVaultPropertySource.getProperty(TEST_PROPERTY_NAME_1); + + assertThat(result).isEqualTo(TEST_PROPERTY_VALUE_1); } + + @Test + public void caseSensitive() { + KeyVaultSecret keyVaultSecret1 = new KeyVaultSecret("KEY1", "value1"); + KeyVaultSecret keyVaultSecret2 = new KeyVaultSecret("Key2", "value2"); + when(keyVaultOperation.listSecrets(null)).thenReturn(List.of(keyVaultSecret1, keyVaultSecret2)); + + KeyVaultPropertySource keyVaultPropertySource = new KeyVaultPropertySource("azure-key-vault-secret-property-source", Duration.ZERO, keyVaultOperation, null, true); + + assertEquals("value1", keyVaultPropertySource.getProperty("KEY1")); + assertEquals(null, keyVaultPropertySource.getProperty("key1")); + assertEquals("value2", keyVaultPropertySource.getProperty("Key2")); + assertEquals(null, keyVaultPropertySource.getProperty("KEY2")); + } + + @Test + public void setTestSpringRelaxedBindingNames() { + KeyVaultSecret keyVaultSecret = new KeyVaultSecret(TEST_AZURE_KEYVAULT_NAME, TEST_AZURE_KEYVAULT_VALUE); + when(keyVaultOperation.listSecrets(null)).thenReturn(List.of(keyVaultSecret)); + KeyVaultPropertySource kvPropertySource = new KeyVaultPropertySource("KeyVault", Duration.ZERO, keyVaultOperation, null, false); + + TEST_SPRING_RELAXED_BINDING_NAMES + .forEach(n -> assertThat(kvPropertySource.getProperty(n)).isEqualTo(TEST_AZURE_KEYVAULT_VALUE)); + } + + @Test + @Timeout(5) + public void refreshTwoKeyVaultsPropertySources() throws InterruptedException { + CountDownLatch latchForRefreshing = new CountDownLatch(2); + new SecretRefreshing(latchForRefreshing, "KeyVault1", "test1", + "value1", "value1Updated").start(); + new SecretRefreshing(latchForRefreshing, "KeyVault2", "test2", + "value2", "value2Updated").start(); + latchForRefreshing.await(); + } + + static class SecretRefreshing extends Thread { + private final CountDownLatch latchForRefreshing; + private final String propertySourceName; + private final SecretClient secretClient; + private final String secretName; + private final String initialSecretValue; + private final String updatedSecretValue; + private static final int REFRESH_IN_SECONDS = 3; + + SecretRefreshing(CountDownLatch latchForRefreshing, + String propertySourceName, + String secretName, + String initialSecretValue, + String updatedSecretValue) { + this.latchForRefreshing = latchForRefreshing; + this.propertySourceName = propertySourceName; + this.secretClient = mock(SecretClient.class); + this.secretName = secretName; + this.initialSecretValue = initialSecretValue; + this.updatedSecretValue = updatedSecretValue; + } + + @Override + public void run() { + KeyVaultOperation secretOperation = new KeyVaultOperation(secretClient); + + KeyVaultSecret initialKeyVaultSecret = mockSecretClientGetSecretMethod(secretClient, secretName, initialSecretValue); + mockSecretClientListPropertiesOfSecrets(secretClient, initialKeyVaultSecret.getProperties()); + + KeyVaultPropertySource propertySource = new KeyVaultPropertySource( + propertySourceName, + Duration.ofSeconds(REFRESH_IN_SECONDS), + secretOperation, + null, + true); + assertThat(propertySource.getProperty(this.secretName)).isEqualTo(initialSecretValue); + + mockSecretClientGetSecretMethod(secretClient, secretName, updatedSecretValue); + try { + TimeUnit.SECONDS.sleep(REFRESH_IN_SECONDS + 1); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + assertThat(propertySource.getProperty(this.secretName)).isEqualTo(updatedSecretValue); + latchForRefreshing.countDown(); + } + + } + } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultSecretClientMockUtils.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultSecretClientMockUtils.java new file mode 100644 index 0000000000000..41deade288ee4 --- /dev/null +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/KeyVaultSecretClientMockUtils.java @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment; + +import com.azure.core.http.rest.PagedFlux; +import com.azure.core.http.rest.PagedIterable; +import com.azure.security.keyvault.secrets.SecretClient; +import com.azure.security.keyvault.secrets.models.KeyVaultSecret; +import com.azure.security.keyvault.secrets.models.SecretProperties; +import reactor.core.publisher.Mono; + +import java.util.List; + +import static org.mockito.Mockito.when; + +public class KeyVaultSecretClientMockUtils { + + public static void mockSecretClientListPropertiesOfSecrets(SecretClient secretClient, SecretProperties... secretPropertiesList) { + OnePageResponse secretResponse = new OnePageResponse<>(List.of(secretPropertiesList)); + when(secretClient.listPropertiesOfSecrets()) + .thenReturn(new PagedIterable<>(new PagedFlux<>(() -> Mono.just(secretResponse)))); + } + + public static KeyVaultSecret mockSecretClientGetSecretMethod(SecretClient secretClient, String secretName, String secretValue) { + return mockSecretClientGetSecretMethod(secretClient, secretName, secretValue, true); + } + + public static KeyVaultSecret mockSecretClientGetSecretMethod(SecretClient secretClient, + String secretName, + String secretValue, + boolean enabled) { + KeyVaultSecret keyVaultSecret = new KeyVaultSecret(secretName, secretValue); + keyVaultSecret.getProperties().setEnabled(enabled); + when(secretClient.getSecret(secretName, null)).thenReturn(keyVaultSecret); + when(secretClient.getSecret(secretName)).thenReturn(keyVaultSecret); + return keyVaultSecret; + } +} diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/OnePageResponse.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/OnePageResponse.java new file mode 100644 index 0000000000000..44e24e157d684 --- /dev/null +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/environment/OnePageResponse.java @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.spring.cloud.autoconfigure.implementation.keyvault.environment; + +import com.azure.core.http.HttpHeaders; +import com.azure.core.http.HttpRequest; +import com.azure.core.http.rest.PagedResponse; +import com.azure.core.util.IterableStream; +import reactor.core.publisher.Flux; + +import java.io.IOException; +import java.util.List; + +class OnePageResponse implements PagedResponse { + + List properties = null; + + OnePageResponse(List properties) { + this.properties = properties; + } + + @Override + public IterableStream getElements() { + Flux flux = Flux.fromIterable(properties); + return new IterableStream(flux); + } + + @Override + public String getContinuationToken() { + return null; + } + + @Override + public int getStatusCode() { + return 0; + } + + @Override + public HttpHeaders getHeaders() { + return null; + } + + @Override + public HttpRequest getRequest() { + return null; + } + + @Override + public void close() throws IOException { + + } +}