Skip to content

Adding Default Filters #43935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Matthew Metcalf (mrm9084) merged 9 commits into from
Feb 21, 2025
Merged

Adding Default Filters #43935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9d2ee1e
Adding Default Filters
mrm9084 Jan 24, 2025
bf72f92
Fixing Feature flag loading after both redesigns
mrm9084 Feb 5, 2025
ed723c2
Fixing issues after just 2.0 schema and properties
mrm9084 Feb 5, 2025
dccf44e
Clearing up feature return
mrm9084 Feb 5, 2025
7212319
fixing usage of default min backoff
mrm9084 Feb 5, 2025
0b4680e
Removed dead code + tests
mrm9084 Feb 5, 2025
09382e7
Update TargetingContextAccessorTestConfiguration.java
mrm9084 Feb 6, 2025
5c597cb
Merge branch 'SpringCloudAzure6.0-Preview' into DefaultFilters
mrm9084 Feb 13, 2025
f7a1f64
Update FeatureManagementConfiguration.java
mrm9084 Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions ...om/azure/spring/cloud/appconfiguration/config/AppConfigurationWatchAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
import com.azure.spring.cloud.appconfiguration.config.implementation.AppConfigurationReplicaClientFactory;
import com.azure.spring.cloud.appconfiguration.config.implementation.autofailover.ReplicaLookUp;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProperties;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProviderProperties;

/**
* Setup AppConfigurationRefresh when <i>spring.cloud.azure.appconfiguration.enabled</i> is enabled.
*/
@EnableAsync
@ConditionalOnProperty(prefix = AppConfigurationProperties.CONFIG_PREFIX, name = "enabled", matchIfMissing = true)
@EnableConfigurationProperties({ AppConfigurationProperties.class, AppConfigurationProviderProperties.class })
@EnableConfigurationProperties({ AppConfigurationProperties.class })
@AutoConfiguration
@ConditionalOnClass(RefreshEndpoint.class)
public class AppConfigurationWatchAutoConfiguration {
Expand All @@ -37,13 +36,12 @@ public AppConfigurationWatchAutoConfiguration() {

@Bean
@ConditionalOnMissingBean
AppConfigurationRefresh appConfigurationRefresh(AppConfigurationProperties properties,
AppConfigurationProviderProperties appProperties, BootstrapContext context) {
AppConfigurationRefresh appConfigurationRefresh(AppConfigurationProperties properties, BootstrapContext context) {
AppConfigurationReplicaClientFactory clientFactory = context
.get(AppConfigurationReplicaClientFactory.class);
ReplicaLookUp replicaLookUp = context.get(ReplicaLookUp.class);

return new AppConfigurationPullRefresh(clientFactory, properties.getRefreshInterval(),
appProperties.getDefaultMinBackoff(), replicaLookUp, new AppConfigurationRefreshUtil());
return new AppConfigurationPullRefresh(clientFactory, properties.getRefreshInterval(), replicaLookUp,
new AppConfigurationRefreshUtil());
}
}
15 changes: 12 additions & 3 deletions ...pconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@
// Licensed under the MIT License.
package com.azure.spring.cloud.appconfiguration.config.implementation;

import java.util.List;

import org.springframework.core.env.EnumerablePropertySource;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;

/**
* Azure App Configuration PropertySource unique per Store Label(Profile) combo.
*
Expand All @@ -19,21 +24,25 @@ class AppConfigurationFeatureManagementPropertySource extends EnumerableProperty

private static final String FEATURE_FLAG_KEY = FEATURE_MANAGEMENT_KEY + ".feature_flags";

private static final ObjectMapper MAPPER = JsonMapper.builder().build();

AppConfigurationFeatureManagementPropertySource(FeatureFlagClient featureFlagLoader) {
super(FEATURE_MANAGEMENT_KEY, featureFlagLoader);
this.featureFlagLoader = featureFlagLoader;
}

@Override
public String[] getPropertyNames() {
String[] names = { FEATURE_FLAG_KEY };
return names;
if (featureFlagLoader != null && featureFlagLoader.getFeatureFlags().size() > 0) {
return new String[]{ FEATURE_FLAG_KEY };
}
return new String[0];
}

@Override
public Object getProperty(String name) {
if (FEATURE_FLAG_KEY.equals(name)) {
return featureFlagLoader.getProperties();
return MAPPER.convertValue(featureFlagLoader.getFeatureFlags(), List.class);

@samsadsam Sami Sadfa (samsadsam) Feb 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this way faster?

@mrm9084 Matthew Metcalf (mrm9084) Feb 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other way doesn't work. This PR was the first one I created after merging both of the big PRs for the provider and FM and they didn't agree work. The old way you get a type error when trying to load feature flags as the Feature class in the Provider is different than the one in FM.

This make it into JSON which should be the same as reading from a file.

@samsadsam Sami Sadfa (samsadsam) Feb 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now, thanks

}
return null;
}
Expand Down
7 changes: 2 additions & 5 deletions ...g/cloud/appconfiguration/config/implementation/AppConfigurationKeyVaultClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,24 @@ class AppConfigurationKeyVaultClientFactory {
private final boolean credentialsConfigured;

private final boolean isConfigured;

private final int timeout;

AppConfigurationKeyVaultClientFactory(SecretClientCustomizer keyVaultClientProvider,
KeyVaultSecretProvider keyVaultSecretProvider, SecretClientBuilderFactory secretClientFactory,
boolean credentialsConfigured, int timeout) {
boolean credentialsConfigured) {
this.keyVaultClientProvider = keyVaultClientProvider;
this.keyVaultSecretProvider = keyVaultSecretProvider;
this.secretClientFactory = secretClientFactory;
keyVaultClients = new HashMap<>();
this.credentialsConfigured = credentialsConfigured;
isConfigured = keyVaultClientProvider != null || credentialsConfigured;
this.timeout = timeout;
}

AppConfigurationSecretClientManager getClient(String host) {
// Check if we already have a client for this key vault, if not we will make
// one
if (!keyVaultClients.containsKey(host)) {
AppConfigurationSecretClientManager client = new AppConfigurationSecretClientManager(host,
keyVaultClientProvider, keyVaultSecretProvider, secretClientFactory, credentialsConfigured, timeout);
keyVaultClientProvider, keyVaultSecretProvider, secretClientFactory, credentialsConfigured);
keyVaultClients.put(host, client);
}
return keyVaultClients.get(host);
Expand Down
8 changes: 3 additions & 5 deletions ...zure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPullRefresh.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public class AppConfigurationPullRefresh implements AppConfigurationRefresh {
private final AtomicBoolean running = new AtomicBoolean(false);

private ApplicationEventPublisher publisher;

private final Long defaultMinBackoff;
private final Long defaultMinBackoff = (long) 30;

private final AppConfigurationReplicaClientFactory clientFactory;

Expand All @@ -46,11 +46,9 @@ public class AppConfigurationPullRefresh implements AppConfigurationRefresh {
*
* @param clientFactory Clients stores used to connect to App Configuration. * @param defaultMinBackoff default
* @param refreshInterval time between refresh intervals
* @param defaultMinBackoff minimum time between backoff retries minimum backoff time
*/
public AppConfigurationPullRefresh(AppConfigurationReplicaClientFactory clientFactory, Duration refreshInterval,
Long defaultMinBackoff, ReplicaLookUp replicaLookUp, AppConfigurationRefreshUtil refreshUtils) {
this.defaultMinBackoff = defaultMinBackoff;
ReplicaLookUp replicaLookUp, AppConfigurationRefreshUtil refreshUtils) {
this.refreshInterval = refreshInterval;
this.clientFactory = clientFactory;
this.replicaLookUp = replicaLookUp;
Expand Down
5 changes: 2 additions & 3 deletions ...g/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,10 @@ public class AppConfigurationReplicaClientsBuilder {

private final boolean credentialConfigured;

private final int defaultMaxRetries;
private final int defaultMaxRetries = 2;

AppConfigurationReplicaClientsBuilder(int defaultMaxRetries, ConfigurationClientBuilderFactory clientFactory,
AppConfigurationReplicaClientsBuilder(ConfigurationClientBuilderFactory clientFactory,
ConfigurationClientCustomizer clientCustomizer, boolean credentialConfigured, boolean isKeyVaultConfigured) {
this.defaultMaxRetries = defaultMaxRetries;
this.credentialConfigured = credentialConfigured;
this.clientFactory = clientFactory;
this.clientCustomizer = clientCustomizer;
Expand Down
37 changes: 20 additions & 17 deletions .../spring/cloud/appconfiguration/config/implementation/AzureAppConfigBoostrapRegistrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import org.springframework.boot.BootstrapRegistry.InstanceSupplier;
import org.springframework.boot.context.config.ConfigDataLocationResolverContext;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.util.StringUtils;
Expand All @@ -15,9 +14,9 @@
import com.azure.spring.cloud.appconfiguration.config.SecretClientCustomizer;
import com.azure.spring.cloud.appconfiguration.config.implementation.autofailover.ReplicaLookUp;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProperties;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProviderProperties;
import com.azure.spring.cloud.autoconfigure.implementation.appconfiguration.AzureAppConfigurationProperties;
import com.azure.spring.cloud.autoconfigure.implementation.context.properties.AzureGlobalProperties;
import com.azure.spring.cloud.autoconfigure.implementation.keyvault.secrets.properties.AzureKeyVaultSecretProperties;
import com.azure.spring.cloud.autoconfigure.implementation.properties.core.AbstractAzureHttpConfigurationProperties;
import com.azure.spring.cloud.autoconfigure.implementation.properties.core.authentication.TokenCredentialConfigurationProperties;
import com.azure.spring.cloud.autoconfigure.implementation.properties.utils.AzureGlobalPropertiesUtils;
Expand All @@ -26,12 +25,10 @@
import com.azure.spring.cloud.service.implementation.appconfiguration.ConfigurationClientBuilderFactory;
import com.azure.spring.cloud.service.implementation.keyvault.secrets.SecretClientBuilderFactory;

@EnableConfigurationProperties(AppConfigurationProviderProperties.class)
class AzureAppConfigurationBootstrapRegistrar {

static void register(ConfigDataLocationResolverContext context, Binder binder,
AppConfigurationProperties properties, AppConfigurationProviderProperties appProperties,
ReplicaLookUp replicaLookup) {
AppConfigurationProperties properties, ReplicaLookUp replicaLookup) {

AzureGlobalProperties globalProperties = binder
.bind(AzureGlobalProperties.PREFIX, Bindable.of(AzureGlobalProperties.class))
Expand All @@ -46,9 +43,9 @@ static void register(ConfigDataLocationResolverContext context, Binder binder,
boolean isCredentialConfigured = isCredentialConfigured(loadedProperties);

AppConfigurationKeyVaultClientFactory keyVaultClientFactory = appConfigurationKeyVaultClientFactory(context,
isCredentialConfigured, appProperties.getMaxRetryTime());
AppConfigurationReplicaClientsBuilder replicaClientsBuilder = replicaClientBuilder(context, binder,
keyVaultClientFactory, loadedProperties, isCredentialConfigured, appProperties.getMaxRetries());
binder, isCredentialConfigured);
AppConfigurationReplicaClientsBuilder replicaClientsBuilder = replicaClientBuilder(context,
keyVaultClientFactory, loadedProperties, isCredentialConfigured);

context.getBootstrapContext().registerIfAbsent(AppConfigurationKeyVaultClientFactory.class,
InstanceSupplier.from(() -> keyVaultClientFactory));
Expand All @@ -57,17 +54,23 @@ static void register(ConfigDataLocationResolverContext context, Binder binder,
}

private static AppConfigurationKeyVaultClientFactory appConfigurationKeyVaultClientFactory(
ConfigDataLocationResolverContext context, boolean isCredentialConfigured, Integer maxRetryTime)
ConfigDataLocationResolverContext context, Binder binder, boolean isCredentialConfigured)
throws IllegalArgumentException {

SecretClientCustomizer customizer = context.getBootstrapContext().getOrElse(SecretClientCustomizer.class, null);
KeyVaultSecretProvider secretProvider = context.getBootstrapContext().getOrElse(KeyVaultSecretProvider.class,
null);
SecretClientBuilderFactory secretClientFactory = context.getBootstrapContext()
.getOrElse(SecretClientBuilderFactory.class, null);

return new AppConfigurationKeyVaultClientFactory(customizer, secretProvider, secretClientFactory,
isCredentialConfigured, maxRetryTime);
AzureKeyVaultSecretProperties secretClientProperties = binder
.bind(AzureKeyVaultSecretProperties.PREFIX, Bindable.of(AzureKeyVaultSecretProperties.class))
.orElseGet(AzureKeyVaultSecretProperties::new);
SecretClientBuilderFactory secretClientBuilderFactory = new SecretClientBuilderFactory(secretClientProperties);

context.getBootstrapContext().registerIfAbsent(SecretClientBuilderFactory.class,
InstanceSupplier.from(() -> secretClientBuilderFactory));
Comment on lines +64 to +70

@samsadsam Sami Sadfa (samsadsam) Feb 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand this change.

Is the previous way (getting the factory from the bootstrap context) wrong?

Can you explain to me this part in more detail please

@mrm9084 Matthew Metcalf (mrm9084) Feb 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be easier to explain just what the new code does.

  1. Binder is a Spring class that can take @Properties classes and bind them to the system. AzureKeyVaultSecretProperties is an Azure Spring class, not one of ours, we just use it.
  2. Checks to see if when exists and binds it to our binder.
  3. Creates a new one if one doesn't exist and then binds it.
  4. SecretClientBuilderFactory creates the clients we use to get secrets from key vault. This creates a @Bean of it, but only if it doesn't exist.
  5. Just us telling it how to create one if one doesn't exist.

This is part of our change over to the new configuration system. I found a scenario where the SecretClientBuilderFactory hadn't been created by something else in the system before we started. So, I have to do it.

@samsadsam Sami Sadfa (samsadsam) Feb 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So before, we assumed that SecretClientBuilderFactory will be created by someone else (who?)

@mrm9084 Matthew Metcalf (mrm9084) Feb 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you had a dependency on the Azure Spring Key Vault library it would be created. They have one that sort of works like us.

@samsadsam Sami Sadfa (samsadsam) Feb 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


return new AppConfigurationKeyVaultClientFactory(customizer, secretProvider, secretClientBuilderFactory,
isCredentialConfigured);
}

private static AppConfigurationReplicaClientFactory buildClientFactory(
Expand All @@ -78,8 +81,8 @@ private static AppConfigurationReplicaClientFactory buildClientFactory(

@SuppressWarnings("unchecked")
private static AppConfigurationReplicaClientsBuilder replicaClientBuilder(ConfigDataLocationResolverContext context,
Binder binder, AppConfigurationKeyVaultClientFactory keyVaultClientFactory,
AzureAppConfigurationProperties properties, boolean isCredentialConfigured, Integer maxRetries) {
AppConfigurationKeyVaultClientFactory keyVaultClientFactory, AzureAppConfigurationProperties properties,
boolean isCredentialConfigured) {

InstanceSupplier<AzureServiceClientBuilderCustomizer<ConfigurationClientBuilder>> customizer = context
.getBootstrapContext()
Expand Down Expand Up @@ -108,8 +111,8 @@ private static AppConfigurationReplicaClientsBuilder replicaClientBuilder(Config
clientCustomizer = configurationClientCustomizer.get(context.getBootstrapContext());
}

return new AppConfigurationReplicaClientsBuilder(maxRetries, clientFactory, clientCustomizer,
isCredentialConfigured, keyVaultClientFactory.isConfigured());
return new AppConfigurationReplicaClientsBuilder(clientFactory, clientCustomizer, isCredentialConfigured,
keyVaultClientFactory.isConfigured());
}

private static boolean isCredentialConfigured(AbstractAzureHttpConfigurationProperties properties) {
Expand Down
10 changes: 5 additions & 5 deletions ...m/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.springframework.boot.context.config.ConfigDataLoader;
import org.springframework.boot.context.config.ConfigDataLoaderContext;
import org.springframework.boot.context.config.ConfigDataResourceNotFoundException;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.logging.DeferredLog;
import org.springframework.boot.logging.DeferredLogFactory;
import org.springframework.core.env.EnumerablePropertySource;
Expand All @@ -24,12 +23,10 @@
import com.azure.data.appconfiguration.models.ConfigurationSetting;
import com.azure.spring.cloud.appconfiguration.config.implementation.feature.FeatureFlags;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationKeyValueSelector;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationProviderProperties;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationStoreMonitoring;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.AppConfigurationStoreMonitoring.PushNotification;
import com.azure.spring.cloud.appconfiguration.config.implementation.properties.FeatureFlagKeyValueSelector;

@EnableConfigurationProperties(AppConfigurationProviderProperties.class)
public class AzureAppConfigDataLoader implements ConfigDataLoader<AzureAppConfigDataResource> {

private static Log logger = new DeferredLog();
Expand All @@ -46,6 +43,10 @@ public class AzureAppConfigDataLoader implements ConfigDataLoader<AzureAppConfig

private Context requestContext;

private static final Instant START_DATE = Instant.now();

private static final Integer PREKILL_TIME = 5;

public AzureAppConfigDataLoader(DeferredLogFactory logFactory) {
logger = logFactory.getLog(getClass());
}
Expand Down Expand Up @@ -200,8 +201,7 @@ private List<FeatureFlags> createFeatureFlags(AppConfigurationReplicaClient clie

private void delayException() {
Instant currentDate = Instant.now();
Instant preKillTIme = resource.getAppProperties().getStartDate()
.plusSeconds(resource.getAppProperties().getPrekillTime());
Instant preKillTIme = START_DATE.plusSeconds(PREKILL_TIME);
if (currentDate.isBefore(preKillTIme)) {
long diffInMillies = Math.abs(preKillTIme.toEpochMilli() - currentDate.toEpochMilli());
try {
Expand Down
Loading