App Configuration Provider - Tag filters #47985
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrm9084
requested review from
a team,
Netyyyy,
moarychan,
rossgrambo,
rujche and
saragluna
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for filtering App Configuration settings and feature flags by tag expressions (via tags-filter), and updates selector model JavaDocs intended to improve configuration metadata/YAML hinting.
Changes:
- Adds
tagsFilterto key/value selector models and propagates it intoSettingSelectorwhen listing settings and feature flags. - Updates loader/client/property source APIs to thread tags filter through the loading pipeline.
- Expands unit test coverage for tags filter behavior (null/empty/custom) and snapshot incompatibility.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java | Passes tagsFilter into setting/feature-flag loads and watched-settings selectors. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java | Adds tagsFilter field and applies it to SettingSelector for listing settings. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationSnapshotPropertySource.java | Updates superclass constructor call for new tags-filter parameter. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java | Adds tags-filter support when listing feature flags. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelector.java | Introduces tagsFilter, adds snapshot vs tags validation, and refreshes JavaDocs. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/ConfigStore.java | Updates selector documentation/default initialization behavior (relies on selector default key filter behavior). |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java | Introduces tagsFilter and refreshes JavaDocs for feature-flag selection behavior. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagStore.java | JavaDoc refresh and minor init-doc clarification. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationProperties.java | JavaDoc refresh for configuration properties root. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationStoreMonitoring.java | JavaDoc refresh and clarified validation description. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationStoreTrigger.java | JavaDoc refresh for trigger semantics. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySourceTest.java | Adds tests asserting tags-filter propagation into SettingSelector for settings loads. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySourceKeyVaultTest.java | Updates constructor usage for new tags-filter parameter. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClientTest.java | Updates client calls for new tags-filter parameter and adds tags-filter propagation tests. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/AppConfigurationKeyValueSelectorTest.java | Adds tests for tags-filter get/set behavior and snapshot incompatibility. |
| sdk/spring/spring-cloud-azure-appconfiguration-config/src/test/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelectorTest.java | Adds tests for tags-filter get/set behavior. |
Comments suppressed due to low confidence (1)
sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/properties/FeatureFlagKeyValueSelector.java:91
getLabelFilter(List<String> profiles)reverses the providedprofileslist in-place. This can throwUnsupportedOperationExceptionwhenprofilesis immutable (e.g., Spring Boot may supply an unmodifiable list), and it also mutates caller state. Consider making a defensive copy (e.g., new ArrayList<>(profiles)) before reversing, similar toAppConfigurationKeyValueSelector.getLabelFilter’s approach.
public String[] getLabelFilter(List<String> profiles) {
if (labelFilter == null && profiles.size() > 0) {
Collections.reverse(profiles);
return profiles.toArray(new String[profiles.size()]);
} else if (!StringUtils.hasText(labelFilter)) {
Comment on lines
+140
to
+149
| /** | ||
| * Sets the tag filters for feature flags. | ||
| * | ||
| * @param tagsFilter list of tag expressions in {@code tagName=tagValue} format | ||
| * @return this {@link FeatureFlagKeyValueSelector} for chaining | ||
| */ | ||
| public FeatureFlagKeyValueSelector setTagsFilter(List<String> tagsFilter) { | ||
| this.tagsFilter = tagsFilter; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
The JavaDoc says each tags entry must be in tagName=tagValue format, but setTagsFilter/validateAndInit don’t enforce or validate this (nulls/empty strings/missing = would be accepted silently). Either add validation (and fail fast with a clear message) or relax the JavaDoc to reflect pass-through behavior.
Comment on lines
+52
to
+53
| * Filters configurations by tags. Each entry must follow the {@code tagName=tagValue} | ||
| * format. When multiple entries are provided, they are combined using AND logic. |
There was a problem hiding this comment.
The JavaDoc states that each entry in tagsFilter must follow tagName=tagValue, but there’s currently no validation ensuring that (or preventing null/blank entries). Please either validate the list in validateAndInit/setter (with a helpful exception message) or adjust the documentation so it doesn’t promise constraints the code doesn’t enforce.
Suggested change
| * Filters configurations by tags. Each entry must follow the {@code tagName=tagValue} | |
| * format. When multiple entries are provided, they are combined using AND logic. | |
| * Filters configurations by tags. Each entry is interpreted as a tag-based filter, | |
| * typically in the {@code tagName=tagValue} format. When multiple entries are | |
| * provided, they are combined using AND logic. |
|
|
||
| /** | ||
| * Loads configurations from a named snapshot. Cannot be used together with | ||
| * key, label, or tag filters. Must not contain asterisks ({@code *}). |
There was a problem hiding this comment.
JavaDoc says snapshotName “Must not contain asterisks (*)”, but validateAndInit() doesn’t currently check snapshotName for *. Either add the corresponding validation (fail fast) or remove that constraint from the JavaDoc so it matches actual behavior.
Suggested change
| * key, label, or tag filters. Must not contain asterisks ({@code *}). | |
| * key, label, or tag filters. |
...in/java/com/azure/spring/cloud/appconfiguration/config/implementation/FeatureFlagClient.java
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
tags-filterlabel-filterAll SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines