Resolving unused settings bug when plugin is disabled

[kobelb]

When a plugin is disabled, we were previously removing the plugin's schema and all the settings. This was causing as issue when trying to determine which settings were specified in the .yml but are unused in the following code:

return difference(keys(transformDeprecations(settings)), keys(configValues));

I was struggling to think of a scenario where leaving the settings in the config when a plugin is disabled would cause an issue, because the plugin itself shouldn't be loaded and using these settings. The only thing that I could come up with was if a plugin was disabled, and another plugin was somehow using the settings without checking to see if it was 'enabled' this could potentially cause an issue.

[spalger]

if a plugin was disabled, and another plugin was somehow using the settings without checking to see if it was 'enabled' this could potentially cause an issue.

This is definitely something I want to avoid... Do you have any ideas about how this could be solved without removing the unset logic?

[kobelb]

If we could potentially determine the list of disabled plugins, we could concatenate their config prefix with the list of active settings and diff against this. This would require the config's complete step to know about disabled plugins, which isn't ideal.

Or when we disabled a plugin, we could overwrite the config schema and values with a simple schema/value that only had 'enabled: false'. We could also try to use the config defaults, and set enabled to false, but that seems like it has the same issue as not removing them.

There might be others, but those are the first two that I thought of.

[kobelb]

We could also reintroduce the mutable 'pendingSets' approach to keep track of unused settings.

[spalger]

Or when we disabled a plugin, we could overwrite the config schema and values with a simple schema/value that only had 'enabled: false'. We could also try to use the config defaults, and set enabled to false, but that seems like it has the same issue as not removing them.

I actually like that a lot. It still prevents access to config entries like config.get('elasticsearch.url') when the plugin is disabled, and it doesn't require any more awareness of the config than necessary in getUnusedSettings()

[kobelb]

I will make it so! Thanks @spalger

[spalger]

LGTM