Enable removal of a key or subtree

[tillig]

While you can set a value for a key, if you want to remove an entire subtree of configuration settings the best you can do right now is to recursively iterate through all keys and subkeys and set every value found to null.

It would be nice to have a remove method to allow complete removal of keys.

I imagine it would be on the IConfiguration interface:

public interface IConfiguration
{
    string this[string key] { get; set; }
    void Remove(string key);
    IConfigurationSection GetSection(string key);
    IEnumerable<IConfigurationSection> GetChildren();
}

As well as on the IConfigurationSource interface

public interface IConfigurationProvider
{
    bool TryGet(string key, out string value);
    void Set(string key, string value);
    void Remove(string key);
    void Load();
    IEnumerable<string> GetChildKeys(
            IEnumerable<string> earlierKeys,
            string parentPath,
            string delimiter);
}

Implementation of the Remove method would be similar to the implementation of the Set method for Microsoft.Extensions.Configuration.ConfigurationSection where it would iterate through each provider and call Remove. Providers could each do the appropriate thing as needed to remove the specified key and all children from the source.

A use case this addresses is for providing an administrative interface to modify configuration. Users in the interface may need to add new keys or rename existing ones - you can't rename keys right now because there's no way to delete the old keys; only set the values to null. For implementations of IConfigurationProvider with a persistent backing store, this makes for a very messy and unmaintainable solution long-term.

[akatz0813]

+1 MSBuild Transforms have this

[davidfowl]

Would be a breaking API change.

[tillig]

It would be now... But an additional interface could be added with that method and sources could optionally support it.

[davidfowl]

Yep! We'll look at this in the future.

[HaoK]

We are not planning on changing the interface but feel free to submit a PR with this implemented as an extension method.

[tillig]

I don't think you can do this with an extension method unless you allow the extension method to cast (internally) all of the abstractions down to the concrete implementations and start poking around, which doesn't seem like the greatest idea. That would be necessary because none of the abstractions like IConfigurationRoot have access to the aggregated set of configuration providers. There'd be no way to iterate through the providers to call Remove or perform other necessary actions.

As it stands, the list of providers would have to be made public on ConfigurationRoot so you could cast down from IConfigurationRoot to ConfigurationRoot and act on that list without having to poke around with reflection to get the private field.

Without something to indicate whether the source supports key removal it may be hard to implement as an extension method. Some sort of ISupportKeyRemoval additional interface would work - something that an IConfigurationProvider could implement to indicate support.

public interface ISupportKeyRemoval
{
  void Remove(string key);
}

ConfigurationRoot could implement that interface, as could some of the configuration providers if they allow it. No interface change for the abstractions, just an additional optional implementation for providers that support the concept.

Then the extension method could totally work. It could cast the inbound object as ISupportKeyRemoval and if it's not null, call the Remove(string) method.

Does that sound reasonable?

[tillig]

It'd be nice if IConfigurationRoot added an IEnumerable<IConfigurationProvider> property with the list of providers, though I see that's a breaking change, too. Otherwise I'm not sure how to cleanly get that list of providers without assuming only ConfigurationRoot.

[divega]

@tillig what I had in mind when @HaoK and I discussed this is that an extension method could recursively walk down the tree (probably using the existing AsEnumerable() extension) and set all the values to null.

This already sets the value for that key to null on all underlying providers, so you probably wouldn't need access to list of providers just for this.

That said, in the current implementation of our providers setting the value of a section to null just sets the value for that key in an internal dictionary to null. Subsequent calls to GetChildren() would still return the descendant sections that have been nulled out. Hence it is more of a way to "zero out" a key and its children than it is a removal.

I think we can discuss/test changing the behavior of setting a null value to remove the entry from the dictionary instead of nulling the value. At least right now I cannot think of any reason that would be fundamentally bad and I cannot think of any way returning dangling null sections that contain null values is useful. My main concern with doing that is that the contract of the interface might be too loose and if third party providers don't comply with this behavior the Remove() method would not have the desired effect.

[tillig]

The problem with just nulling things out is that if you want to back configuration with something like Redis or another persistence layer, storing "null" may actually be valid... as opposed to actually removing the keys, which was the intent of the difference here. This is like the equivalent of a Dictionary<K,V> - the configuration interfaces have a set, they have a get, but there's no remove.

[divega]

@tillig however IConfiguration is not trying to be a dictionary. Yes, it has an accessor and most providers today use a dictionary as an implementation detail, but the metaphor falls short in other ways.

[tillig]

It doesn't seem any analogy works. If you consider it an arbitrary data source, it has CRU - Create, Read, Update... but no Delete. It just seems incomplete. Hence the desire to make it complete. I found in my own dealings that managing configuration through this thing - not reading, but actually also writing (which is supported) - becomes problematic when you get outside the very simple use cases of the original out-of-the-box providers (XML / JSON / environment). It seems a shame to not add some sort of way to opt in to allow for a complete configuration manager story.

[divega]

@tillig FWIW, the Create operation doesn't exist in IConfiguration either.

The following test attempts to demonstrate why it is desirable that setting a key to null removes it from the tree reachable through GetChildren() and AsEnumerable().

For brevity I used the Exists() extension method which was contribute by @andrewlock in aspnet/Configuration#521 and got merged today. IMO the behavior of this method is completely consistent with the semantics of IConfiguration: a section exists only if it has a value or if it has children sections.

[Fact]
public void ConfigurationRemoveTest()
{
    var config = new ConfigurationBuilder()
        .AddInMemoryCollection()
        .Build();

    var section = config.GetSection("a:b:c"); // represents an arbitrary child key in the key-space

    Assert.Null(section.Value); // does not have a value

    Assert.False(section.Exists()); // hence it doesn't exists yet

    Assert.Equal(config.AsEnumerable().Count(), 0); // in fact the configuration is still empty

    section.Value = "x"; // same as config["a:b:c"] = "x";

    Assert.True(config.GetSection("a").Exists()); // the grand parent exists because it has a grand child
    Assert.True(config.GetSection("a:b").Exists()); // the parent exists because it has a child
    Assert.True(config.GetSection("a:b:c").Exists()); // and the child exists because it has a value

    // Unfortunately things start getting inconsistent if you try to set the value to null

    config["a:b:c"] = null;

    Assert.True(config.GetSection("a").Exists()); // the grand parent exists because it has a grand child
    Assert.True(config.GetSection("a:b").Exists()); // the parent exists because it has a child
    Assert.False(config.GetSection("a:b:c").Exists()); // but the child doesn't actually exist!!!
}

[tillig]

@divega The "Create" part is what you did in the test by setting a value for a section that didn't previously exist. That's sort of how it works in dictionary land, too - you don't have to call Add(key, value) to create a new dictionary entry.

However, I think I'm starting to understand what you're saying. The inconsistency you point out is part of what I want to avoid; but also that adds the natural implications that "setting to null == removal" as well as "you can't store a null in configuration"; which is to say, even if I have a JSON configuration file like this...

{
  "first": 123,
  "second": null
}

...then changing the semantics to "null == removed" would imply that reading the null value (and any child null values) would just be skipped or ignored.

It would also mean this yields an empty configuration:

{
  "parent": {
    "child": null
  }
}

...which is fine, again, as long as the semantics change to be "a tree of things in which all values are null is effectively removed from configuration." Providers could deal with that reasonably.

Is that what you're getting at?

If so, it would mean the Exists() extension would need to be updated to correctly report all three of the final assertions in your test to be false.

  // This should be false, but is currently inconsistently/incorrectly true.
  Assert.True(config.GetSection("a").Exists());

  // Same here.
  Assert.True(config.GetSection("a:b").Exists());

  // The leaf is the only one correctly reporting.
  Assert.False(config.GetSection("a:b:c").Exists());

However, if that's not what you're saying then maybe I'm still misunderstanding. What I want is the ability to get the consistent results out of that unit test. Ideally, the actual call to GetChildren() wouldn't return any children if the entire child tree is null. Effectively removed, not just nulled out. If I need to add it back in, I can do that GetSection("a:b:c").Value = "x" set operation.

[divega]

@tillig I agree the indexer in Dictionary has an implicit Create. It is just not reversed when you set the value to null, and it makes sense for it not to be reversed in dictionary because null is just like any other value for Dictionary. In IConfiguration we just happen to have stronger semantics for null...

And yes, that is what I am getting at. I am still not sure it is feasible 😄, but I think it would be more consistent.

[tillig]

Excellent. If the semantics can be updated such that tree-of-nulls == removed and is reported that way throughout (e.g., through GetChildren() and such), I'd be cool with that over an explicit Remove() operation. In that case, you're right - Remove() could totally be an extension method of recursive null value setting operations.

[divega]

If so, it would mean the Exists() extension would need to be updated to correctly report all three of the final assertions in your test to be false.

Possibly. But I haven't tried just changing the implementation of Set to do Data.Remove when value == null. I would need to check what happens with your sample configuration files too.

[divega]

@tillig oh and by all means, feel free to experiment with this yourself if you would like to spend a PR.

[lfoust]

I would like to +1 the request to support Remove(). Another scenario where this is useful is for arrays. If you have:

{
   "array": [
        "item1",
        "item2"
   ]
}

And you want to remove an item with:

config["array:1"] = null;

you will end up with an array which has two items: "item1" and null

Being able to remove an entire subtree or individual keys would be very helpful here. If you want more details on my scenario (why I want to remove items from IConfigurationRoot) feel free to let me know.

[abspro]

How about we change this from Future to .Net 7. What do ya say?

[0xced]

I just stumbled on this issue with a very real use case. In my appsettings.json file I have a configuration for the TLS certificate that will be used in production as described on Kestrel documentation:

{
  "Kestrel": {
    "Certificates": {
      "Default": {
        "Path": "<path to .pfx file>",
        "Password": "$CREDENTIAL_PLACEHOLDER$"
      }
    }
  }
}

During development, I'd like to use the ASP.NET Core HTTPS development certificate but I can't remove the Certificates section from the configuration. I can't craft an appsettings.Development.json file that removes it nor can I do it programmatically.

Unfortunately I still haven't found a workaround to this issue.

Edit I figured out how to use the ASP.NET Core HTTPS development certificate for development by overriding the Path and Password to empty strings in the appsettings.Development.json file.

{
  "Kestrel": {
    "Certificates": {
      "Default": {
        "Path": "",
        "Password": ""
      }
    }
  }
}

But that's just luck because of how the certificate configuration loader works.

[Bertie2011]

+1 on a very similar use case.

I had to recreate HTTPS certificates and found the best way of doing that would be to temporarily allow the website to run in HTTP mode. Preferably I would have liked to accomplish this with a single flag to adjust various settings and middlewares throughout, but I couldn't find a way to just wipe out the https endpoint without going to my pipeline files to clear the env variables.

[dkjolith]

This "cheat" works:

    private void RemoveSection(IConfigurationRoot root, IConfigurationSection section)
    {
        var providers = root.Providers.ToList();

        foreach (var provider in providers)
        {
            var dataProperty = (provider as ConfigurationProvider)?.GetType().GetProperty("Data", BindingFlags.Instance | BindingFlags.NonPublic);

            if (dataProperty?.GetValue(provider) is Dictionary<string, string> data)
            {
                data.Remove(section.Path);
            }
        }
    }