refine the property binding logic using spring binder api (#810)

microsoft · Jan 14, 2020 · 4e657ca · 4e657ca
1 parent 2fae636
commit 4e657ca
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 26 deletions.
diff --git a/...spring-boot-test-keyvault/src/test/java/com/microsoft/azure/test/keyvault/KeyVaultIT.java b/...spring-boot-test-keyvault/src/test/java/com/microsoft/azure/test/keyvault/KeyVaultIT.java
@@ -113,7 +113,7 @@ public void keyVaultAsPropertySourceWithSpecificKeys() {
             app.property("azure.keyvault.client-id", access.clientId());
             app.property("azure.keyvault.client-key", access.clientSecret());
             app.property("azure.keyvault.tenant-id", access.tenant());
-            app.property("azure.keyvault.secret.keys", "key");
+            app.property("azure.keyvault.secret.keys", "key , azure-cosmosdb-key");
 
             app.start();
             assertEquals(KEY_VAULT_VALUE, app.getProperty("key"));

diff --git a/...main/java/com/microsoft/azure/keyvault/spring/KeyVaultEnvironmentPostProcessorHelper.java b/...main/java/com/microsoft/azure/keyvault/spring/KeyVaultEnvironmentPostProcessorHelper.java
@@ -15,14 +15,19 @@
 import com.azure.security.keyvault.secrets.SecretClientBuilder;
 import com.microsoft.azure.telemetry.TelemetrySender;
 import lombok.extern.slf4j.Slf4j;
+
+import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.MutablePropertySources;
 import org.springframework.core.env.StandardEnvironment;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
@@ -45,7 +50,9 @@ public void addKeyVaultPropertySource() {
         final Long refreshInterval = Optional.ofNullable(
                 this.environment.getProperty(Constants.AZURE_KEYVAULT_REFRESH_INTERVAL))
                 .map(Long::valueOf).orElse(Constants.DEFAULT_REFRESH_INTERVAL_MS);
-        final String secretKeys = this.environment.getProperty(Constants.AZURE_KEYVAULT_SECRET_KEYS);
+        final Binder binder = Binder.get(this.environment);
+        final List<String> secretKeys = binder.bind(Constants.AZURE_KEYVAULT_SECRET_KEYS, Bindable.listOf(String.class))
+                .orElse(Collections.emptyList());
 
         final TokenCredential tokenCredential = getCredentials();
         final SecretClient secretClient = new SecretClientBuilder()

diff --git a/azure-spring-boot/src/main/java/com/microsoft/azure/keyvault/spring/KeyVaultOperation.java b/azure-spring-boot/src/main/java/com/microsoft/azure/keyvault/spring/KeyVaultOperation.java
@@ -16,6 +16,7 @@
 import org.springframework.util.StringUtils;
 
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.locks.ReadWriteLock;
@@ -24,7 +25,7 @@
 @Slf4j
 public class KeyVaultOperation {
     private final long cacheRefreshIntervalInMs;
-    private final String[] secretKeys;
+    private final List<String> secretKeys;
 
     private final Object refreshLock = new Object();
     private final SecretClient keyVaultClient;
@@ -39,30 +40,15 @@ public class KeyVaultOperation {
     public KeyVaultOperation(final SecretClient keyVaultClient,
                              String vaultUri,
                              final long refreshInterval,
-                             final String secretKeysConfig) {
+                             final List<String> secretKeys) {
         this.cacheRefreshIntervalInMs = refreshInterval;
-        this.secretKeys = parseSecretKeys(secretKeysConfig);
+        this.secretKeys = secretKeys;
         this.keyVaultClient = keyVaultClient;
         // TODO(pan): need to validate why last '/' need to be truncated.
         this.vaultUri = StringUtils.trimTrailingCharacter(vaultUri.trim(), '/');
         fillSecretsList();
     }
 
-    private String[] parseSecretKeys(String secretKeysConfig) {
-        if (StringUtils.isEmpty(secretKeysConfig)) {
-            log.info("specific secret keys haven't set, so apply global list mode");
-            return new String[0];
-        }
-
-        final String[] split = secretKeysConfig.split(",");
-        if (split.length == 0) {
-            log.info("specific secret keys haven't set, so apply global list mode");
-            return new String[0];
-        }
-
-        return split;
-    }
-
     public String[] list() {
         try {
             this.rwLock.readLock().lock();
@@ -106,7 +92,7 @@ public String get(final String property) {
         final String secretName = getKeyvaultSecretName(property);
 
         //if user don't set specific secret keys, then refresh token
-        if (this.secretKeys == null || secretKeys.length == 0) {
+        if (this.secretKeys == null || secretKeys.size() == 0) {
             // refresh periodically
             refreshPropertyNames();
         }
@@ -132,7 +118,7 @@ private void refreshPropertyNames() {
     private void fillSecretsList() {
         try {
             this.rwLock.writeLock().lock();
-            if (this.secretKeys == null || secretKeys.length == 0) {
+            if (this.secretKeys == null || secretKeys.size() == 0) {
                 this.propertyNames.clear();
 
                 final PagedIterable<SecretProperties> secretProperties = keyVaultClient.listPropertiesOfSecrets();

diff --git a/...ing-boot/src/test/java/com/microsoft/azure/keyvault/spring/KeyVaultOperationUnitTest.java b/...ing-boot/src/test/java/com/microsoft/azure/keyvault/spring/KeyVaultOperationUnitTest.java
@@ -25,7 +25,7 @@
 
 @RunWith(MockitoJUnitRunner.class)
 public class KeyVaultOperationUnitTest {
-    private static final String secretKeysConfig = "key1,key2,key3";
+    private static final List<String> secretKeysConfig = Arrays.asList("key1", "key2", "key3");
 
     private static final String testPropertyName1 = "testPropertyName1";
 
@@ -54,7 +54,7 @@ public class KeyVaultOperationUnitTest {
     private SecretClient keyVaultClient;
     private KeyVaultOperation keyVaultOperation;
 
-    public void setupSecretBundle(String id, String value, String secretKeysConfig) {
+    public void setupSecretBundle(String id, String value, List<String> secretKeysConfig) {
         //provision for list
         when(keyVaultClient.listPropertiesOfSecrets()).thenReturn(new MockPage(new PagedFlux<>(() -> null), id));
         //provison for get
@@ -98,7 +98,7 @@ public void testList() {
         setupSecretBundle(testPropertyName1, testPropertyName1, secretKeysConfig);
         final String[] specificResult = keyVaultOperation.list();
         assertThat(specificResult.length).isEqualTo(3);
-        assertThat(specificResult[0]).isEqualTo(secretKeysConfig.split(",")[0]);
+        assertThat(specificResult[0]).isEqualTo(secretKeysConfig.get(0));
     }
 
     @Test
@@ -111,7 +111,7 @@ public void setTestSpringRelaxedBindingNames() {
         );
 
         //test list with specific secret key configs
-        setupSecretBundle(TEST_AZURE_KEYVAULT_NAME, TEST_AZURE_KEYVAULT_NAME, TEST_AZURE_KEYVAULT_NAME);
+        setupSecretBundle(TEST_AZURE_KEYVAULT_NAME, TEST_AZURE_KEYVAULT_NAME, Arrays.asList(TEST_AZURE_KEYVAULT_NAME));
         TEST_SPRING_RELAXED_BINDING_NAMES.forEach(
                 n -> assertThat(keyVaultOperation.get(n)).isEqualTo(TEST_AZURE_KEYVAULT_NAME)
         );