ConfigurationFeatureDefinitionProvider ignores False when there are exists RequirementType

[DenomikoN]

Faced with the issue when due to the overriding configuration ConfigurationFeatureDefinitionProvider will falsely enable a feature flag.
Example setup:
appsettings.json

{
  "FeatureManagement":{
    "FeatureA": {
       "RequirementType": "All",
       "EnabledFor": [{
          "Name":"AlwaysOn"
       }]
    }
  }
}

Overriden by production configuration:
appsettings.production.json

{
  "FeatureManagement:FeatureA": "False" // Disable feature
}

That produces the following configuration key-values pairs:

["FeatureManagement:FeatureA"] = "False",
["FeatureManagement:FeatureA:RequirementType"] = "All",
["FeatureManagement:FeatureA:EnabledFor:0:Name"] = "AlwaysOn"

Expected result: feature manager returns False for IsEnabledAsync("FeatureA")
Test example:

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.FeatureManagement;

namespace TestProject
{
    public class Tests
    {
        [Test]
        public async Task FeatureWithDirectValueMustBeDisabled()
        {
            var configKeys = new Dictionary<string, string?>
            {
                ["FeatureManagement:FeatureA"] = "False",
                ["FeatureManagement:FeatureA:RequirementType"] = "All",
                ["FeatureManagement:FeatureA:EnabledFor:0:Name"] = "AlwaysOn"
            };
            var cfg = new ConfigurationBuilder()
                            .AddInMemoryCollection(configKeys)
                            .Build();

            var sc = new ServiceCollection();
            sc.AddFeatureManagement();
            sc.AddSingleton<IConfiguration>(cfg);

            var serviceProvider = sc.BuildServiceProvider();

            var featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

            var featureState = await featureManager.IsEnabledAsync("FeatureA");

            Assert.IsFalse(featureState, "FeatureA must be disabled as it's Value is False");
        }
    }
}

Actual result:: featureManager.IsEnabledAsync("FeatureA") returns True

Root cause: ConfigurationFeatureDefinitionProvider doesn't distinguish if there is an actual False value set or value can't be parsed in the method ParseDotnetSchemaFeatureDefinition:

 61: if (!string.IsNullOrEmpty(val) && bool.TryParse(val, out bool result) && result)

[zhiyuanliang-ms]

Hi, @DenomikoN Sorry for the inconvenience. I think the key issue here is that when merging different configuration source, the IConfiguration system will produce the below configuration

["FeatureManagement:FeatureA"] = "False",
["FeatureManagement:FeatureA:RequirementType"] = "All",
["FeatureManagement:FeatureA:EnabledFor:0:Name"] = "AlwaysOn"

This should not be a valid feature flag declaration.

To work around with the current behavior of IConfiguration, I suggest you switch to our new Microsoft feature management schema (json schema is here)
Using this schema can kind of mitigate the merging configuration issue.

From our side, the root cause you mentioned is correct. Do you think when ConfigurationFeatureDefinitionProvider meets some feature flag declaration which is invalid or cannot be parsed, it should throw exception or log warning?