diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs index a5657c2b..3690e3f4 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using System; using System.Collections.Generic; +using System.Linq; namespace Microsoft.Extensions.Configuration { @@ -96,7 +97,12 @@ public static IConfigurationBuilder AddAzureAppConfiguration( { if (!_isProviderDisabled) { - configurationBuilder.Add(new AzureAppConfigurationSource(action, optional)); + // Count Azure App Configuration sources already registered on this builder so that + // the new source can pick a feature flag index offset that avoids colliding with + // flags emitted by those earlier sources. + int priorAppConfigSourceCount = configurationBuilder.Sources.OfType().Count(); + + configurationBuilder.Add(new AzureAppConfigurationSource(action, optional, priorAppConfigSourceCount)); } return configurationBuilder; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index e5b6e258..1d443625 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -149,6 +149,22 @@ internal IEnumerable Adapters /// internal FeatureFlagTracing FeatureFlagTracing { get; set; } = new FeatureFlagTracing(); + /// + /// The starting index used when emitting feature flags into the configuration system under + /// the Microsoft schema (feature_management:feature_flags:<index>). + /// + /// During migration from classic to new Azure App Configuration feature flags, multiple + /// providers may write to the same configuration section. .NET's configuration system + /// merges arrays by index position, causing flags from different providers to overwrite + /// each other. To prevent this, the provider automatically assigns an offset based on the + /// number of other Azure App Configuration providers already registered on the + /// configuration builder: the first provider uses offset 0, the second uses + /// , the + /// third uses 2 * , + /// and so on. + /// + internal int FeatureFlagIndexOffset { get; set; } = 0; + /// /// Options used to configure provider startup. /// @@ -168,7 +184,7 @@ public AzureAppConfigurationOptions() { new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider()), new JsonKeyValueAdapter(), - new FeatureManagementKeyValueAdapter(FeatureFlagTracing) + new FeatureManagementKeyValueAdapter(FeatureFlagTracing, this) }; // Adds the default query to App Configuration if and are never called. diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationSource.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationSource.cs index 83d20e2f..0ea511de 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationSource.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationSource.cs @@ -3,6 +3,7 @@ // using Azure.Data.AppConfiguration; using Microsoft.Extensions.Azure; +using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement; using System; using System.Collections.Generic; using System.Linq; @@ -14,12 +15,22 @@ internal class AzureAppConfigurationSource : IConfigurationSource private readonly bool _optional; private readonly Func _optionsProvider; - public AzureAppConfigurationSource(Action optionsInitializer, bool optional = false) + public AzureAppConfigurationSource(Action optionsInitializer, bool optional = false, int priorAppConfigSourceCount = 0) { _optionsProvider = () => { var options = new AzureAppConfigurationOptions(); optionsInitializer(options); + + // If the caller didn't explicitly set an offset, derive it from the position of this + // source relative to other Azure App Configuration sources on the configuration + // builder. This avoids index collisions when multiple Azure App Configuration + // providers emit feature flags under the Microsoft schema. + if (options.FeatureFlagIndexOffset == 0 && priorAppConfigSourceCount > 0) + { + options.FeatureFlagIndexOffset = priorAppConfigSourceCount * FeatureManagementConstants.FeatureFlagIndexStride; + } + return options; }; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementConstants.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementConstants.cs index d344896a..4a5a2518 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementConstants.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementConstants.cs @@ -13,6 +13,12 @@ internal class FeatureManagementConstants public const string FeatureManagementSectionName = "feature_management"; public const string FeatureFlagsSectionName = "feature_flags"; + // Stride used when offsetting feature flag indices to avoid collisions between + // multiple Azure App Configuration providers writing to the same configuration + // section. The Nth provider on a configuration builder uses an offset of + // N * FeatureFlagIndexStride when emitting flags under the Microsoft schema. + public const int FeatureFlagIndexStride = 10000; + // Feature flag properties public const string Id = "id"; public const string Enabled = "enabled"; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs index fdd7f2fd..11ec6613 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs @@ -20,12 +20,15 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManage internal class FeatureManagementKeyValueAdapter : IKeyValueAdapter { private FeatureFlagTracing _featureFlagTracing; + private readonly AzureAppConfigurationOptions _options; private int _featureFlagIndex = 0; + private bool _warnedAboutIndexStrideOverflow = false; private bool _fmSchemaCompatibilityDisabled = false; - public FeatureManagementKeyValueAdapter(FeatureFlagTracing featureFlagTracing) + public FeatureManagementKeyValueAdapter(FeatureFlagTracing featureFlagTracing, AzureAppConfigurationOptions options) { _featureFlagTracing = featureFlagTracing ?? throw new ArgumentNullException(nameof(featureFlagTracing)); + _options = options ?? throw new ArgumentNullException(nameof(options)); _fmSchemaCompatibilityDisabled = EnvironmentVariableHelper.GetBoolOrDefault(EnvironmentVariableNames.FmSchemacompatibilityDisabled); } @@ -42,7 +45,7 @@ public Task>> ProcessKeyValue(Configura featureFlag.Allocation != null || featureFlag.Telemetry != null) { - keyValues = ProcessMicrosoftSchemaFeatureFlag(featureFlag, setting, endpoint); + keyValues = ProcessMicrosoftSchemaFeatureFlag(featureFlag, setting, endpoint, logger); } else { @@ -83,6 +86,7 @@ public void OnChangeDetected(ConfigurationSetting setting = null) public void OnConfigUpdated() { _featureFlagIndex = 0; + _warnedAboutIndexStrideOverflow = false; return; } @@ -140,7 +144,7 @@ private List> ProcessDotnetSchemaFeatureFlag(Featur return keyValues; } - private List> ProcessMicrosoftSchemaFeatureFlag(FeatureFlag featureFlag, ConfigurationSetting setting, Uri endpoint) + private List> ProcessMicrosoftSchemaFeatureFlag(FeatureFlag featureFlag, ConfigurationSetting setting, Uri endpoint, Logger logger) { var keyValues = new List>(); @@ -149,10 +153,25 @@ private List> ProcessMicrosoftSchemaFeatureFlag(Fea return keyValues; } - string featureFlagPath = $"{FeatureManagementConstants.FeatureManagementSectionName}:{FeatureManagementConstants.FeatureFlagsSectionName}:{_featureFlagIndex}"; + int absoluteIndex = _options.FeatureFlagIndexOffset + _featureFlagIndex; + + string featureFlagPath = $"{FeatureManagementConstants.FeatureManagementSectionName}:{FeatureManagementConstants.FeatureFlagsSectionName}:{absoluteIndex}"; _featureFlagIndex++; + // Warn once when this provider has emitted more flags than the offset stride can + // accommodate; further flags will collide with the next provider's offset slot. + if (!_warnedAboutIndexStrideOverflow && _featureFlagIndex >= FeatureManagementConstants.FeatureFlagIndexStride) + { + logger?.LogWarning( + $"Azure App Configuration provider emitted {_featureFlagIndex} feature flags, which meets or exceeds " + + $"the per-provider stride of {FeatureManagementConstants.FeatureFlagIndexStride}. Feature flags from " + + $"different Azure App Configuration providers may collide in the configuration system. " + + $"Consider reducing the number of feature flags loaded per provider."); + + _warnedAboutIndexStrideOverflow = true; + } + keyValues.Add(new KeyValuePair($"{featureFlagPath}:{FeatureManagementConstants.Id}", featureFlag.Id)); keyValues.Add(new KeyValuePair($"{featureFlagPath}:{FeatureManagementConstants.Enabled}", featureFlag.Enabled.ToString())); diff --git a/tests/Tests.AzureAppConfiguration/Unit/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/Unit/FeatureManagementTests.cs index 439c63bb..515ba3cc 100644 --- a/tests/Tests.AzureAppConfiguration/Unit/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/Unit/FeatureManagementTests.cs @@ -2360,6 +2360,176 @@ public void EnvironmentVariableForcesMicrosoftSchemaForAllFlags() Assert.Equal("Big", configWithoutEnvVar["feature_management:feature_flags:0:variants:0:name"]); } + [Fact] + public void FeatureFlagIndexOffset_SingleProvider_UsesNoOffset() + { + // Regression check: a single Azure App Configuration provider should still emit + // feature flags starting at index 0 under the Microsoft schema. + var mockClient = new Mock(MockBehavior.Strict); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { CreateMicrosoftSchemaFlag("FlagA") })); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(mockClient.Object); + options.UseFeatureFlags(); + }) + .Build(); + + Assert.Equal("FlagA", config["feature_management:feature_flags:0:id"]); + Assert.Null(config[$"feature_management:feature_flags:{FeatureManagementConstants.FeatureFlagIndexStride}:id"]); + } + + [Fact] + public void FeatureFlagIndexOffset_TwoProviders_SecondUsesStrideOffset() + { + // When two Azure App Configuration providers are registered on the same builder, the + // second one should offset its feature flag indices by FeatureFlagIndexStride so its + // flags do not collide with the first provider's flags during array merging. + var classicMock = new Mock(MockBehavior.Strict); + classicMock.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List + { + CreateMicrosoftSchemaFlag("ClassicFlagA"), + CreateMicrosoftSchemaFlag("ClassicFlagB") + })); + + var newMock = new Mock(MockBehavior.Strict); + newMock.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List + { + CreateMicrosoftSchemaFlag("NewFlagA"), + CreateMicrosoftSchemaFlag("NewFlagB") + })); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(classicMock.Object); + options.UseFeatureFlags(); + }) + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(newMock.Object); + options.UseFeatureFlags(); + }) + .Build(); + + // Classic provider keeps indices 0, 1 + Assert.Equal("ClassicFlagA", config["feature_management:feature_flags:0:id"]); + Assert.Equal("ClassicFlagB", config["feature_management:feature_flags:1:id"]); + + // New provider shifted to indices 10000, 10001 + int stride = FeatureManagementConstants.FeatureFlagIndexStride; + Assert.Equal("NewFlagA", config[$"feature_management:feature_flags:{stride}:id"]); + Assert.Equal("NewFlagB", config[$"feature_management:feature_flags:{stride + 1}:id"]); + } + + [Fact] + public void FeatureFlagIndexOffset_ThreeProviders_IncrementingOffsets() + { + // Verify the offset scales linearly with provider registration position so that an + // arbitrary number of Azure App Configuration providers can coexist without index + // collisions, up to the per-provider stride. + ConfigurationClient MakeClient(string flagId) + { + var mock = new Mock(MockBehavior.Strict); + mock.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { CreateMicrosoftSchemaFlag(flagId) })); + return mock.Object; + } + + ConfigurationClient client0 = MakeClient("Flag0"); + ConfigurationClient client1 = MakeClient("Flag1"); + ConfigurationClient client2 = MakeClient("Flag2"); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(client0); + options.UseFeatureFlags(); + }) + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(client1); + options.UseFeatureFlags(); + }) + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(client2); + options.UseFeatureFlags(); + }) + .Build(); + + int stride = FeatureManagementConstants.FeatureFlagIndexStride; + Assert.Equal("Flag0", config["feature_management:feature_flags:0:id"]); + Assert.Equal("Flag1", config[$"feature_management:feature_flags:{stride}:id"]); + Assert.Equal("Flag2", config[$"feature_management:feature_flags:{2 * stride}:id"]); + } + + [Fact] + public void FeatureFlagIndexOffset_DuplicateFlagIds_LaterProviderWins() + { + // Document and lock in the "new flag wins on duplicate" behavior that customers rely + // on during migration. Because the second provider's indices come after the first's, + // the Feature Management library's LastOrDefault resolution picks the second. + var classicMock = new Mock(MockBehavior.Strict); + classicMock.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List + { + CreateMicrosoftSchemaFlag("Beta", enabled: false) + })); + + var newMock = new Mock(MockBehavior.Strict); + newMock.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List + { + CreateMicrosoftSchemaFlag("Beta", enabled: true) + })); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(classicMock.Object); + options.UseFeatureFlags(); + }) + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(newMock.Object); + options.UseFeatureFlags(); + }) + .Build(); + + int stride = FeatureManagementConstants.FeatureFlagIndexStride; + + // Both providers emit the flag at distinct indices. + Assert.Equal("Beta", config["feature_management:feature_flags:0:id"]); + Assert.Equal("False", config["feature_management:feature_flags:0:enabled"]); + Assert.Equal("Beta", config[$"feature_management:feature_flags:{stride}:id"]); + Assert.Equal("True", config[$"feature_management:feature_flags:{stride}:enabled"]); + } + + private static ConfigurationSetting CreateMicrosoftSchemaFlag(string id, bool enabled = true) + { + // A non-null telemetry block forces the adapter to emit under the Microsoft schema, + // which is the schema affected by the index offset strategy. + return ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + id, + value: $@" + {{ + ""id"": ""{id}"", + ""enabled"": {enabled.ToString().ToLowerInvariant()}, + ""telemetry"": {{ + ""enabled"": true + }} + }} + ", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")); + } + Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) { return Response.FromValue(FirstKeyValue, new MockResponse(200));