Feature/114 add django setup configuration

[Coperh]

fixes: #114

Implements django setup configuration in OIDC itself

[Coperh]

Mostly copied AdminOIDCConfigurationSettings from open inwoner and the documentation. Made the endpoints required since it fails to save without them. Converted the unittest to pytest

Example of it being used here open-zaak/open-zaak#1728

[Coperh]

I am also unsure about the package/file names

[sergei-maertens]

I took a preliminary glance, haven't looked into much details yet, but I have some concerns - perhaps about the way django-setup-configuration itself works:

amount of settings

there are a lot of settings being defined purely to facilitate bootstrapping configuration. Some of these setting names also have a big potential to cause confusion with settings that actually have some meaning at runtime. I don't think this will improve the developer experience

settings datatypes

I noticed in OIP that settings are used so that envvars can be used to specify them, but the datatypes of these settings (like lists of strings, periods/comma's having special meaning...) but also dict/mappings complicates passing them via the env

you basically have a Helm chart or Ansible playbook where the datatype is properly defined in YAML, this is then templated out to a string so it can be passed as an envvar, after which the application deserializes it again from a string into the expected datatype

I would much prefer one of the options below (with the first having my preference):

• load a YAML/TOML/JSON file that contains the data/config to initialize. you then only need to point to which file to load, plus it will scale in the future when we don't have solo models anymore but instead regular models. it's basically a django-agnostic fixture format
• read the values directly from the env and remove the need to define them as settings. the library knows how to interpret each envvar and parse it into the expected data-structure

IMO the former approach makes documenting the format a lot easier, you can simply provide an example template, you can add comments if you use YAML/TOML and the processing step of that file is able to populate missing information (the endpoints!) from the discovery endpoint if that's all that is specified.

[alextreme]

FYI for Conor, Paul is working on a related issue: https://taiga.maykinmedia.nl/project/open-inwoner/issue/2563

[Coperh]

Should it use the glob pattern here? I do not think it did in the original Open Inwoner step but does in the middleware

[Coperh]

Also I should probably make a test for this

[stevenbal]

If I read this correctly it will always create missing groups, regardless of the value of sync_groups? I would either remove sync_groups from the model and always set it to True then, or add it as a conditional here to make sure it works the same as in the admin

[Coperh]

This is an oversite and I assume it should use the sync_group value

[swrichards]

Useful to see the API being battle tested. I'll update the docs at least regarding defaults and testing best practices based on this. Otherwise LGTM.

[Coperh]

@stevenbal @SonnyBA This is ready to re-review if either of you have time.

[swrichards]

Basically done I think, just a few questions/comments mainly about the use of defaults.

[stevenbal]

@swrichards perhaps this could be an alternative to the generate documentation functionality?

[stevenbal]

If I read this correctly it will always create missing groups, regardless of the value of sync_groups? I would either remove sync_groups from the model and always set it to True then, or add it as a conditional here to make sure it works the same as in the admin

[swrichards]

@swrichards perhaps this could be an alternative to the generate documentation functionality?

@stevenbal It's definitely a good stopgap until the documentation generation is fixed, though I think we should add the auto-generation eventually (but later, because it's non-blocking for the MVP). Doing the extra work here and now makes sense because this is a library with consumers, but I think for the downstream projects we can afford to wait until the documentation generator is back up.