diff --git a/docs/decisions/0022-settings-simplification.rst b/docs/decisions/0022-settings-simplification.rst new file mode 100644 index 000000000000..ed10beeedc4b --- /dev/null +++ b/docs/decisions/0022-settings-simplification.rst @@ -0,0 +1,362 @@ +Django settings simplification +############################## + +Status +****** + +Accepted + +Implementation tracked by: https://github.com/openedx/edx-platform/issues/36215 + +Context +******* + +OEP-45 declares that sites will configure each IDA's (indepently-deployable +application's) Django settings with an ``_CFG`` YAML file, parsed and +loaded by a single upstream-provided ``DJANGO_SETTINGS_MODULE``. This contrasts +with the Django convention, which is that sites override Django settings using +their own ``DJANGO_SETTINGS_MODULE``. The rationale was that all Open edX +setting customization can be reasonably specified in YAML; therefore, it is +operationally safer to avoid using a custom ``DJANGO_SETTINGS_MODULE``, and it +is operationally desirable for all operation modes to execute the same Python +module for configuration. This was `briefly discussed in the oep-45 review +`_. + +For example, in theory, the upstream production LMS config might be named +``lms/settings/settings.py`` and work like this: + +* import ``lms/settings/required.py``, which declares settings that must be + overridden. +* import ``lms/settings/defaults.py``, which defines reasonable defaults for + all other settings. +* load ``/openedx/config/lms.yml``, which should override every setting + declared in required.py and override some settings defined in defaults.py. +* apply some minimal merging and/or conditional logic to handle yaml values + which are not simple overrides (e.g., ``FEATURES``, which needs to be + merged). + +The upstream production CMS config would exist in parallel. + +However, as of Sumac, we do not know of any site other than edx.org that +successfully uses only YAML files for configuration. Furthermore, the +upstream-provided ``DJANGO_SETTINGS_MODULE`` which loads these yaml files +(``lms/envs/production.py``) is not simple: it declares defaults, imports from +other Django settings modules, sets more defaults, handles dozens of special +cases, and has a special Open-edX-specific "derived settings" mechanism to +handle settings that depend on other settings. + +Tutor does provide YAML files, but *it also has custom production and +development settings files*! The result is that we have multiple layers of +indirection between edx-platform's common base settings, and the Django +settings rendered into the actual community-supported Open edX distribution +(Tutor). Specifically, production edx-platform configuration currently works +like this: + +* ``lms/envs/tutor/production.py``... + + * is generated by Tutor from the template + ``tutor/templates/apps/openedx/settings/lms/production.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_lms.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_all.py``; + + * and uses templates vars from Tutor configuration (``config.yml``), + + * and invokes hooks from any enabled Tutor plugins; + + * it imports ``lms/envs/production.py``, + + * which imports ``lms/envs/common.py``, + + * which sets production-inappropriate defaults; + + * it sets more defaults, some of them edX.org-specific; + + * it loads ``/openedx/config/lms.yml``... + + * which is generated by Tutor from template + ``tutor/templates/apps/openedx/config/lms.env.yml`` + + * which derives + ``tutor/templates/apps/openedx/config/partials/auth.yml``; + + * it reverts some of ``lms.yml`` with new "defaults"; + + * and it uses certain values from ``/openedx/config/lms.yml`` to + conditionally override more settings and update certain dictionary + settings, in a way which is not documented. + +* ``cms/envs/tutor/production.py``... + + * is generated by Tutor from the template + ``tutor/templates/apps/openedx/settings/cms/production.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_cms.py``, + + * which derives + ``tutor/templates/apps/openedx/settings/partials/common_all.py``; + + * and uses templates vars from Tutor configuration (``config.yml``), + + * and invokes hooks from any enabled Tutor plugins; + + * it imports ``cms/envs/production.py``, + + * it imports ``cms/envs/common.py``, which sets production-inappropriate + defaults, + + * and which imports ``lms/envs/common.py``, which also sets + production-inappropriate defaults; + + * it sets more defaults, some of the edX.org-specific; + + * it loads ``/openedx/config/cms.yml``... + + * which is generated by Tutor from template + ``tutor/templates/apps/openedx/config/cms.env.yml`` + + * which derives + ``tutor/templates/apps/openedx/config/partials/auth.yml``; + + * it reverts some of ``/openedx/config/cms.yml`` with new "defaults"; + + * and it uses certain values from ``/openedx/config/cms.yml`` to + conditionally override more settings and update certain dictionary + settings, in a way which is not documented. + +This is very difficult to reason about. Configuration complexity is frequently +cited as a chief area of pain for Open edX developers and operators. +Discussions in the Named Release Planning and Build-Test-Release Working Groups +frequently are encumbered with confusion and uncertainty of what the default +settings are in edx-platform, how they differ from Tutor's default settings, +what settings can be overriden, and how to do so. Only a minority of developers +and operators fully understand the configuration logic described above +end-to-end; even for those that do, following this override chain for any given +Django setting is time-consuming and error-prone. CAT-1 bugs and high-severity +security vulnerabilities have arisen due to misunderstanding of how +edx-platform Django settings are rendered. + +Developers are frequently instructed that if they need to override a Django +setting, the preferred way to do so is to "make a Tutor plugin". This is a +large amount of prior knowledge, boilerplate, and indirection, all required +to simply do something which Django provides out-of-the-box via a custom +``DJANGO_SETTINGS_MODULE``. + +Finally, it is worth nothing that all the complexity and toil exists alongside +other edx-platform configuration methods, such as Waffle, configuration models, +site configuration, XBlock configuration, and entry points. Those configuration +pathways are outside of the scope of this ADR, but are mentioned to demonstrate +the distressing level of complexity that developers and operators face when +working with the platform. + +Decision & Consequences +*********************** + +Overview +======== + +We orient edx-platform towards using standard Django settings configuration +patterns. Specifically, we will make it easy for operators to override settings +by supplying a custom ``DJANGO_SETTINGS_MODULE``. + +Moving towards this goals will need to be an iterative and careful process, +and it's likely that some aspects of the target structure or plan (described +below) will need to updated along the way. Nonetheless, once it becomes clear +that we are landing on a solid settings structure for edx-platform, we'll +propose an OEP-45 update to generalize the structure to all deployable Open edX +Django applications. + +Finally, based on what we learn throughout this process, our OEP-45 propsal +will either recommend to: + +1. Drop support for the ``_CFG`` YAML files, or + +2. Simplify the ``_CFG`` YAML schema, document it, and clarify that it + is an optional alternative to ``DJANGO_SETTINGS_MODULE`` rather than the + required/preferred configuration method. + +At the time, we do not have enough information whether option 1 or 2 would be +more beneficial overall to the community. +`The discussion on this sub-decisision will continue on this GitHub issue `_. + +Target settings structure for edx-platform +========================================== + +* ``openedx/envs/common.py``: Define as much shared configuration between LMS + and CMS as possible, including: (a) where possible, annotated definitions of + edx-platform-specific settings with *reasonable, production-ready* defaults; + (b) otherwise, annotated definitions of edx-platform-specific settings (like + secrets) with *obviously-wrong* defaults, ensuring they aren't used in + production; and (c) reasonable production-ready overrides of third-party + settings, ideally with explanatory comments (but not annotations). When a + particular setting's default should depend on the *final* value of another + setting, the former should be assigned to a + ``Derived(...)`` value, where ``...`` is a computation based on the latter. + + * ``lms/envs/common.py``: Extend ``openedx/envs/common.py`` to create, as + much as possible, a production-ready settings file for the LMS. These + extension may include: (a) annotated definitions of LMS-specific settings + with production-ready defaults; (b) annotated definitions of LMS-specific + settings with obviously-wrong defaults; and (c) LMS-specific + overrides of settings defined in ``openedx/envs/common.py`` and of + third-party settings, ideally with explanatory comments (but not + annotations). Again, ``Derived`` settings can be used as appropriate. This + will be the default settings file for running LMS management commands, + although tools can override this (as usual) by specifying a + ``DJANGO_SETTINGS_MODULE``. + + * ``lms/envs/test.py``: Override LMS settings for unit tests. Should work + in a local venv as well as in CI. Needs to invoke ``derive_settings`` in + order to render all previously-defined ``Derived`` settings. + + * ``lms/envs/yaml.py`` (only if we decide to retain YAML support): + Loads overrides from a YAML File at ``LMS_CFG``, plus some well-defined + special handling for mergable values like ``FEATURES``. This is adapted + from and replaces lms/envs/production.py. It will invoke + ``derive_settings``. + + * ``/lms_prod.py`` (example path): Open edX sites that do not + use ``lms/envs/yaml.py`` will instead have to have their + ``DJANGO_SETTINGS_MODULE`` environment variable pointed at a custom + settings module, derived from ``lms/envs/common.py``. It is important + that this module both (i) replaces the obviously-wrong settings with + appropriate production settings, and (ii) invokes ``derive_settings`` to + render all previously-defined ``Derived`` settings. If we decide not to + retain YAML support, then *every* Open edX deployment will need to be + pointed at such a custom settings module settings file, either maintained + by the operator or by a downstream tool (like Tutor). + + * ``lms/envs/dev.py``: Override LMS settings so that it can run + "bare metal" directly on a developer's local machine using debug-friendly + settings. Will use ``local.openedx.io`` (which resolves to 127.0.0.1) as + a base domain, which should be suitable for downstream tools as well. It + will invoke ``derive_settings``. + + * ``/lms_dev.py`` (example path): In order to + run the LMS, downstream tools (like Tutor, and 2U's devstack) will + need to separately maintain their own custom settings module derived + from ``lms/envs/dev.py``, and point their + ``DJANGO_SETTINGS_MODULE`` environment variable at this module. + + * ``cms/envs/common.py`` + + * ``cms/envs/test.py`` + + * ``cms/envs/yaml.py`` (only if we decide to retain YAML support) + + * ``/cms_prod.py`` (example path) + + * ``cms/envs/dev.py`` + + * ``/cms_dev.py`` (example path) + +Plan of action +============== + +These steps are non-breaking unless noted. + +* Introduce a dump_settings management command so that we can more easily + validate changes (or lack thereof) to the terminal edx-platform settings + modules. + +* Improve edx-platform's API for + deriving settings, as we are about to depend on it significantly more than we + currently do. This is a potentially BREAKING CHANGE to any downstream + settings files which imported from ``openedx.core.lib.derived``. + +* Remove redundant overrides in (cms,lms)/envs/production.py. Use Derived + settings defaults to further simplify the module without changing its output. + +* Create openedx/envs/common.py, ensuring that any annotations defined in it + are included in the edx-platform reference docs build. Move settings which + are shared between (cms,lms)/envs/common.py into openedx/envs/common.py. This + may be iteratively done across multiple PRs. + +* Find the best production-ready defaults between both + (lms,cms)/envs/production.py and Tutor's production.pys, and "bubble" them up + to (openedx,cms,lms)/common.py. Keep (lms,cms)/envs/production.py unchanged + through this process. This is a BREAKING CHANGE for any operator that derives + from (lms,cms)/envs/common.py directly. Most operators derive from + (lms,cms)/envs/production.py, so we do not expect this to affect many sites, + if any. + +* Develop (cms,lms)/envs/dev based off of (cms,lms)/envs/common.py. + Iterate until we can run "bare metal" development server for LMS and CMS + using these settings. + +* Deprecate and remove (cms,lms)/envs/devstack.py. This is a BREAKING CHANGE to + downstream development tools (like Tutor and 2U's devstack), as they will + now either need to maintain local copies of these modules, or "rebase" + themselves onto (lms,cms)/envs/dev.py. + +* Propose and, if accepted, implement an update to OEP-45 (Configuring and + Operating Open edX). `Progress on this update is tracked here`_. As mentioned + in the Decision section, based on (a) what we learn from previous steps and + (b) discussion in `"Should we continue to support YAML settings?" `_ + this update will either: + + 1. Revoke the OEP-45 sections regarding YAML. Deprecate and remove + (cms,lms)/envs/production.py. This is a BREAKING CHANGE for tools and + providers that use these settings modules, as they will either need to + maintain local copies of these modules, or "rebase" their internal + settings modules onto (cms,lms)/envs/common.py. Update operator + documenation as needed. + + 2. Update OEP-45 to clarify that YAML configuration is + optional. Operators can opt out of YAML by deriving directly from + (cms,lms)/envs/common.py, or they can opt into YAML by using + (cms,lms)/envs/yaml.py. Document a simplified YAML schema in OEP-45. + There will be several well-communicated BREAKING CHANGES in YAML behavior + in order to achieve the simplified schema. Furthermore, the rename of + (cms,lms)/envs/production.py to (cms,lms)/envs/yaml.py will be a BREAKING + CHANGE. + +* Create tickets to achieve a similar OEP-45-compliant settings structure in + any IDAs (independently-deployable applications) which exist in the openedx + GitHub organization, such as the Credentials service. + +.. _Progress on this update is tracked here: https://github.com/openedx/open-edx-proposals/issues/587 + +Alternatives Considered +*********************** + +One alternative settings structure +================================== + + +Here is an alternate structure that would de-dupe any shared LMS/CMS dev & test +logic by creating more shared modules within openedx/envs folder. Although +DRYer, this structure would increase the total number of edx-platform files and +potentially encourage more LMS-CMS coupling. So, we will not pursue this +structure, but will keep it in mind as an alternative if we enounter +difficulties with the plan laid out in this ADR. + +* ``openedx/envs/common.py`` + + * ``lms/envs/prod.py`` + + * ``/lms_prod.py`` + + * ``cms/envs/prod.py`` + + * ``/cms_prod.py`` + + * ``openedx/envs/test.py`` + + * ``lms/envs/test.py`` + + * ``cms/envs/test.py`` + + * ``openedx/envs/dev.py`` + + * ``lms/envs/dev.py`` + + * ``/lms_dev.py`` + + * ``cms/envs/dev.py`` + + * ``/cms_dev.py``