Allow selecting enabled plugins

[mdellweg]

fixes #5235

[rochacbruno]

Where does ENABLED_PLUGINS comes from? an environment variable?

I see 2 simpler ways for doing it without requiring a hook

1. read the env directly

PRELOAD_FOR_DYNACONF=[
    "{}.app.settings".format(plugin_name) 
     for plugin_name in INSTALLED_PULP_PLUGINS 
     if plugin_name in os.getenv("PULP_ENABLED_PLUGINS", [])
],

The downside of above strategy is that variable will need to come from envvar, not set on a settings.py file.

2. Post load plugin data without the hook

settings = DjangoDynaconf(....)
# then imediatelly after
for plugin_name in INSTALLED_PULP_PLUGINS:
    if plugin_name not in settings.get("enabled_plugins", []):
        continue
    settings.load_file("{}.app.settings".format(plugin_name))

[rochacbruno]

this will also require a late validation, so replace

validators= [...]

with

settings = DjangoDynaconf(...)
for plugin_name in INSTALLED_PULP_PLUGINS:
    ....

# invoke validators
settings.validators.register(**list_of_validators)
settings.validators.validade()

[mdellweg]

It can come from any outside of pulp code base config source. e.g. /etc/pulp/settings.py or PULP_ENABLED_PLUGINS var.

Does this method ensure that the chainloaded plugin settings (like pulp_containers TOKEN_SERVER) have lower precedence than external config sources?
My impression from reading the docs is that they would overwrite local settings.

[rochacbruno]

You are right, my suggestion would put plugin loading at the end of the process and then override local settings.

That is why in the current implementation we are using PRELOAD to ensure plugins settings loads before other settings sources.

Looks like we needed to have support for pre hooks, but right now we only support post loading hooks.

One idea is to write a custom loader to bring plugin_settings and then change the order of loaders putting the plugin_loader before everything else.

ideas @pedro-psb ?

[pedro-psb]

If taking approach 2, I believe an explicit load of the settings files should ensure the external file precedence:

settings = DjangoDynaconf(...)
# (plugins load)
settings.load_file(settings.get("PULP_SETTINGS"))
# (validators)

But this original hook solution seems to do the trick, is there a particular reason not to use it?

[mdellweg]

The thing is that we need to keep the order, because "everything else" is what may provide the ENABLED_PLUGINS.

[ipanova]

does it make sense to add validatiion to the setting so that at least one plugin needs to be enabled?

[mdellweg]

Technically pulp would start just fine without it. So Maybe?

[mdellweg]

Something weird is going on here. _bypass_evaluation should not be in the final settings object...

In [4]: settings.TEMPLATES.to_list()
Out[4]: 
[{'BACKEND': 'django.template.backends.django.DjangoTemplates',
  'DIRS': [PosixPath('/src/pulpcore/pulpcore/app/templates')],
  'APP_DIRS': True,
  'OPTIONS': {'context_processors': ['django.template.context_processors.debug',
    'django.template.context_processors.request',
    'django.contrib.auth.context_processors.auth',
    'django.contrib.messages.context_processors.messages']},
  '_bypass_evaluation': True}]

[pedro-psb]

We added some fixes related to this in 3.2.2.
If you are using >=3.2.2 already, I can work on a fix (that looks easy) and do a release on friday.
Wdyt @rochacbruno?

[mdellweg]

We added some fixes related to this in 3.2.2. If you are using >=3.2.2 already, I can work on a fix (that looks easy) and do a release on friday. Wdyt @rochacbruno?

Using 3.2.4.

[mdellweg]

Looks like there are still some issues around dynaconf deeper merge strategies.

e.g. pulp_ansible needs to inject reusable ap conditions. It does not work this way.

[pedro-psb]

Does this satisfy the deeper merge strategies requirements? We'll merge it in 3.3.0 (probably next release). dynaconf/dynaconf#1001

If not, can you open an issue at dynaconf?

[stale]

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

[stale]

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!