Re-design the configuration properties about key vault secrets

sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/keyvault/secrets/properties/AzureKeyVaultPropertySourceProperties.java

@@ -9,8 +9,6 @@
 import java.time.Duration;
 import java.util.List;
 
-import static com.azure.spring.cloud.autoconfigure.keyvault.environment.KeyVaultPropertySource.DEFAULT_AZURE_KEYVAULT_PROPERTYSOURCE_NAME;
-
 /**
  * Configurations to set when Azure Key Vault is used as an external property source.
  *
@@ -31,7 +29,7 @@ public class AzureKeyVaultPropertySourceProperties extends AbstractAzureHttpConf
     /**
      * Name of this property source.
      */
-    private String name = DEFAULT_AZURE_KEYVAULT_PROPERTYSOURCE_NAME;
+    private String name;
     /**
      * Defines the constant for the property that enables/disables case-sensitive keys.
      */

sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/keyvault/environment/KeyVaultEnvironmentPostProcessor.java

@@ -4,34 +4,32 @@
 package com.azure.spring.cloud.autoconfigure.keyvault.environment;
 
 import com.azure.security.keyvault.secrets.SecretClient;
+import com.azure.spring.cloud.autoconfigure.context.AzureGlobalProperties;
 import com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultPropertySourceProperties;
 import com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultSecretProperties;
-import com.azure.spring.cloud.autoconfigure.context.AzureGlobalProperties;
-import com.azure.spring.cloud.autoconfigure.implementation.properties.utils.AzureGlobalPropertiesUtils;
 import com.azure.spring.cloud.core.implementation.util.AzurePropertiesUtils;
 import com.azure.spring.cloud.service.implementation.keyvault.secrets.SecretClientBuilderFactory;
 import org.apache.commons.logging.Log;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor;
-import org.springframework.boot.context.properties.PropertyMapper;
 import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.boot.env.EnvironmentPostProcessor;
 import org.springframework.boot.logging.DeferredLog;
 import org.springframework.core.Ordered;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.MutablePropertySources;
-import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 
-import java.util.Collections;
+import java.util.ArrayList;
 import java.util.List;
 
 import static org.springframework.core.env.StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME;
 
 /**
- * Leverage {@link EnvironmentPostProcessor} to add Key Vault secrets as a property source.
+ * Leverage {@link EnvironmentPostProcessor} to insert {@link KeyVaultPropertySource}s into {@link ConfigurableEnvironment}.
+ * {@link KeyVaultPropertySource}s are constructed according to {@link AzureKeyVaultSecretProperties},
  *
  * @since 4.0.0
  */
@@ -51,115 +49,100 @@ public KeyVaultEnvironmentPostProcessor(Log logger) {
     }
 
     /**
-     * Construct a {@link KeyVaultEnvironmentPostProcessor} instance with default value.
+     * Construct a {@link KeyVaultEnvironmentPostProcessor} instance with a new {@link DeferredLog}.
      */
     public KeyVaultEnvironmentPostProcessor() {
         this.logger = new DeferredLog();
     }
 
     /**
-     * Post-process the environment.
-     *
-     * <p>
-     * Here we are going to process any key vault(s) and make them as available PropertySource(s). Note this supports
-     * both the singular key vault setup, as well as the multiple key vault setup.
-     * </p>
+     * Construct {@link KeyVaultPropertySource}s according to {@link AzureKeyVaultSecretProperties},
+     * then insert these {@link KeyVaultPropertySource}s into {@link ConfigurableEnvironment}.
      *
      * @param environment the environment.
      * @param application the application.
      */
     @Override
     public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) {
-        if (!isKeyVaultClientAvailable()) {
-            logger.info("Key Vault client is not present, skip the Key Vault property source");
+        if (!isKeyVaultClientOnClasspath()) {
+            logger.debug("Skip configuring Key Vault PropertySource. "
+                    + "Because com.azure:azure-security-keyvault-secrets doesn't exist in classpath.");
             return;
         }
 
-        final AzureKeyVaultSecretProperties keyVaultSecretProperties = loadProperties(Binder.get(environment));
-
-        // In propertySources list, smaller index has higher priority.
-        final List<AzureKeyVaultPropertySourceProperties> propertySources = keyVaultSecretProperties.getPropertySources();
-        Collections.reverse(propertySources);
-
-        if (propertySources.isEmpty() && StringUtils.hasText(keyVaultSecretProperties.getEndpoint())) {
-            propertySources.add(new AzureKeyVaultPropertySourceProperties());
+        final AzureKeyVaultSecretProperties secretProperties = loadProperties(environment);
+        if (!secretProperties.isPropertySourceEnabled()) {
+            logger.debug("Skip configuring Key Vault PropertySource. "
+                    + "Because spring.cloud.azure.keyvault.secret.property-source-enabled=false");
+            return;
+        }
+        if (secretProperties.getPropertySources().isEmpty()) {
+            logger.debug("Skip configuring Key Vault PropertySource. "
+                    + "Because spring.cloud.azure.keyvault.secret.property-sources is empty.");
+            return;
         }
 
-        if (isKeyVaultPropertySourceEnabled(keyVaultSecretProperties)) {
-            for (AzureKeyVaultPropertySourceProperties propertySource : propertySources) {
-                final AzureKeyVaultPropertySourceProperties properties = getMergeProperties(keyVaultSecretProperties,
-                                                                                            propertySource);
-                if (properties.isEnabled()) {
-                    addKeyVaultPropertySource(environment, properties);
-                }
+        final List<AzureKeyVaultPropertySourceProperties> propertiesList = secretProperties.getPropertySources();
+        List<KeyVaultPropertySource> keyVaultPropertySources = buildKeyVaultPropertySourceList(propertiesList);
+        final MutablePropertySources propertySources = environment.getPropertySources();
+        // reverse iterate order making sure smaller index has higher priority.
+        for (int i = keyVaultPropertySources.size() - 1; i >= 0; i--) {
+            KeyVaultPropertySource propertySource = keyVaultPropertySources.get(i);
+            logger.debug("Inserting Key Vault PropertySource. name = " + propertySource.getName());
+            if (propertySources.contains(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) {
+                propertySources.addAfter(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, propertySource);
+            } else {
+                propertySources.addFirst(propertySource);
             }
-        } else {
-            logger.debug("Key Vault 'propertySourceEnabled' or 'enabled' is not enabled");
         }
     }
 
-    // TODO (xiada) better way to implement this
-    private AzureKeyVaultPropertySourceProperties getMergeProperties(AzureKeyVaultSecretProperties secretProperties,
-                                                                     AzureKeyVaultPropertySourceProperties propertySource) {
-        AzureKeyVaultPropertySourceProperties mergedResult = new AzureKeyVaultPropertySourceProperties();
-        AzurePropertiesUtils.mergeAzureCommonProperties(secretProperties, propertySource, mergedResult);
-
-        mergedResult.setEndpoint(secretProperties.getEndpoint());
-        mergedResult.setServiceVersion(secretProperties.getServiceVersion());
-        mergedResult.setEnabled(propertySource.isEnabled());
-        mergedResult.setName(propertySource.getName());
-        mergedResult.setCaseSensitive(propertySource.isCaseSensitive());
-        mergedResult.setSecretKeys(propertySource.getSecretKeys());
-        mergedResult.setRefreshInterval(propertySource.getRefreshInterval());
-
-        PropertyMapper propertyMapper = PropertyMapper.get().alwaysApplyingWhenNonNull();
-        propertyMapper.from(propertySource.getEndpoint()).to(mergedResult::setEndpoint);
-        propertyMapper.from(propertySource.getServiceVersion()).to(mergedResult::setServiceVersion);
-
-        return mergedResult;
+    private List<KeyVaultPropertySource> buildKeyVaultPropertySourceList(
+            List<AzureKeyVaultPropertySourceProperties> propertiesList) {
+        List<KeyVaultPropertySource> propertySources = new ArrayList<>();
+        for (int i = 0; i < propertiesList.size(); i++) {
+            AzureKeyVaultPropertySourceProperties properties = propertiesList.get(i);
+            if (!properties.isEnabled()) {
+                logger.debug("Skip configuring Key Vault PropertySource. "
+                        + "Because spring.cloud.azure.keyvault.secret.property-sources[" + i + "].enabled = false.");
+                continue;
+            }
+            if (!StringUtils.hasText(properties.getEndpoint())) {
+                logger.debug("Skip configuring Key Vault PropertySource. "
+                        + "Because spring.cloud.azure.keyvault.secret.property-sources[" + i + "].endpoint is empty.");
+                continue;
+            }
+            propertySources.add(buildKeyVaultPropertySource(properties));
+        }
+        return propertySources;
     }
 
+    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);
+        } catch (final Exception exception) {
+            throw new IllegalStateException("Failed to configure KeyVault property source", exception);
+        }
+    }
 
-    /**
-     * Add a Key Vault property source.
-     *
-     * <p>
-     * The normalizedName is used to target a specific key vault (note if the name is the empty string it works as
-     * before with only one key vault present). The normalized name is the name of the specific key vault plus a
-     * trailing "." at the end.
-     * </p>
-     *
-     * @param environment The Spring environment.
-     * @param propertySource The property source properties.
-     * @throws IllegalStateException If KeyVaultOperations fails to initialize.
-     */
-    private void addKeyVaultPropertySource(ConfigurableEnvironment environment,
-                                          AzureKeyVaultPropertySourceProperties propertySource) {
-        Assert.notNull(propertySource.getEndpoint(), "endpoint must not be null!");
+    private SecretClient buildSecretClient(AzureKeyVaultPropertySourceProperties propertySourceProperties) {
+        AzureKeyVaultSecretProperties secretProperties = toAzureKeyVaultSecretProperties(propertySourceProperties);
+        return buildSecretClient(secretProperties);
+    }
 
+    private AzureKeyVaultSecretProperties toAzureKeyVaultSecretProperties(
+            AzureKeyVaultPropertySourceProperties propertySourceProperties) {
         AzureKeyVaultSecretProperties secretProperties = new AzureKeyVaultSecretProperties();
-        AzurePropertiesUtils.copyAzureCommonProperties(propertySource, secretProperties);
-        secretProperties.setServiceVersion(propertySource.getServiceVersion());
-        secretProperties.setEndpoint(propertySource.getEndpoint());
-        try {
-            final MutablePropertySources sources = environment.getPropertySources();
-            final SecretClient secretClient = buildSecretClient(secretProperties);
-            final KeyVaultOperation keyVaultOperation = new KeyVaultOperation(secretClient,
-                                                                              propertySource.getRefreshInterval(),
-                                                                              propertySource.getSecretKeys(),
-                                                                              propertySource.isCaseSensitive());
-            KeyVaultPropertySource keyVaultPropertySource = new KeyVaultPropertySource(propertySource.getName(),
-                                                                                       keyVaultOperation);
-            if (sources.contains(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) {
-                sources.addAfter(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, keyVaultPropertySource);
-            } else {
-                // TODO (xiada): confirm the order
-                sources.addFirst(keyVaultPropertySource);
-            }
-
-        } catch (final Exception ex) {
-            throw new IllegalStateException("Failed to configure KeyVault property source", ex);
-        }
+        AzurePropertiesUtils.copyAzureCommonProperties(propertySourceProperties, secretProperties);
+        secretProperties.setEndpoint(propertySourceProperties.getEndpoint());
+        secretProperties.setServiceVersion(propertySourceProperties.getServiceVersion());
+        return secretProperties;
     }
 
     /**
@@ -171,38 +154,60 @@ SecretClient buildSecretClient(AzureKeyVaultSecretProperties secretProperties) {
         return new SecretClientBuilderFactory(secretProperties).build().buildClient();
     }
 
-    private AzureKeyVaultSecretProperties loadProperties(Binder binder) {
-        AzureGlobalProperties azureProperties = binder
+    AzureKeyVaultSecretProperties loadProperties(ConfigurableEnvironment environment) {
+        Binder binder = Binder.get(environment);
+        AzureGlobalProperties globalProperties = binder
             .bind(AzureGlobalProperties.PREFIX, Bindable.of(AzureGlobalProperties.class))
             .orElseGet(AzureGlobalProperties::new);
 
-        AzureKeyVaultSecretProperties existingValue = new AzureKeyVaultSecretProperties();
-        AzureGlobalPropertiesUtils.loadProperties(azureProperties, existingValue);
+        AzureKeyVaultSecretProperties secretProperties = binder
+                .bind(AzureKeyVaultSecretProperties.PREFIX, Bindable.of(AzureKeyVaultSecretProperties.class))
+                .orElseGet(AzureKeyVaultSecretProperties::new);
+
+        List<AzureKeyVaultPropertySourceProperties> list = secretProperties.getPropertySources();
+
+        // Load properties from global properties.
+        for (int i = 0; i < list.size(); i++) {
+            list.set(i, buildMergedProperties(globalProperties, list.get(i)));
+        }
 
-        return binder
-            .bind(AzureKeyVaultSecretProperties.PREFIX,
-                  Bindable.of(AzureKeyVaultSecretProperties.class).withExistingValue(existingValue))
-            .orElseGet(AzureKeyVaultSecretProperties::new);
+        // Name must be unique for each property source.
+        // Because MutablePropertySources#add will remove property source with existing name.
+        for (int i = 0; i < list.size(); i++) {
+            AzureKeyVaultPropertySourceProperties propertySourceProperties = list.get(i);
+            if (!StringUtils.hasText(propertySourceProperties.getName())) {
+                propertySourceProperties.setName(buildPropertySourceName(i));
+            }
+        }
+        return secretProperties;
     }
 
-    /**
-     * Is the Key Vault property source enabled.
-     *
-     * @param properties The Azure Key Vault Secret properties.
-     * @return true if the key vault is enabled, false otherwise.
-     */
-    private boolean isKeyVaultPropertySourceEnabled(AzureKeyVaultSecretProperties properties) {
-        return properties.isEnabled()
-            && (properties.isPropertySourceEnabled() && !properties.getPropertySources().isEmpty());
+    private AzureKeyVaultPropertySourceProperties buildMergedProperties(
+            AzureGlobalProperties globalProperties,
+            AzureKeyVaultPropertySourceProperties propertySourceProperties) {
+        AzureKeyVaultPropertySourceProperties mergedProperties = new AzureKeyVaultPropertySourceProperties();
+        AzurePropertiesUtils.mergeAzureCommonProperties(globalProperties, propertySourceProperties, mergedProperties);
+        mergedProperties.setEnabled(propertySourceProperties.isEnabled());
+        mergedProperties.setName(propertySourceProperties.getName());
+        mergedProperties.setEndpoint(propertySourceProperties.getEndpoint());
+        mergedProperties.setServiceVersion(propertySourceProperties.getServiceVersion());
+        mergedProperties.setCaseSensitive(propertySourceProperties.isCaseSensitive());
+        mergedProperties.setSecretKeys(propertySourceProperties.getSecretKeys());
+        mergedProperties.setRefreshInterval(propertySourceProperties.getRefreshInterval());
+        return mergedProperties;
+    }
+
+    String buildPropertySourceName(int index) {
+        return "azure-key-vault-secret-property-source-" + index;
     }
 
-    private boolean isKeyVaultClientAvailable() {
+    private boolean isKeyVaultClientOnClasspath() {
         return ClassUtils.isPresent("com.azure.security.keyvault.secrets.SecretClient",
                                     KeyVaultEnvironmentPostProcessor.class.getClassLoader());
     }
 
     /**
-     *
+     * Get the order value of this object.
      * @return The order value.
      */
     @Override

sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/keyvault/environment/KeyVaultPropertySource.java

@@ -14,26 +14,14 @@
 public class KeyVaultPropertySource extends EnumerablePropertySource<KeyVaultOperation> {
 
     private final KeyVaultOperation operations;
-    public static final String DEFAULT_AZURE_KEYVAULT_PROPERTYSOURCE_NAME = "azurekv";
 
     /**
-     * Creates a new instance of {@link KeyVaultPropertySource}.
-     *
-     * @param keyVaultName the KeyVault name
-     * @param operation the KeyVault operation
+     * Create a new {@code KeyVaultPropertySource} with the given name and {@link KeyVaultOperation}.
+     * @param name the associated name
+     * @param operation the {@link KeyVaultOperation}
      */
-    public KeyVaultPropertySource(String keyVaultName, KeyVaultOperation operation) {
-        super(keyVaultName, operation);
-        this.operations = operation;
-    }
-
-    /**
-     * Creates a new instance of {@link KeyVaultPropertySource}.
-     *
-     * @param operation the KeyVault operation
-     */
-    public KeyVaultPropertySource(KeyVaultOperation operation) {
-        super(DEFAULT_AZURE_KEYVAULT_PROPERTYSOURCE_NAME, operation);
+    public KeyVaultPropertySource(String name, KeyVaultOperation operation) {
+        super(name, operation);
         this.operations = operation;
     }