-
Notifications
You must be signed in to change notification settings - Fork 7
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: add api MFE configuration (#745)
* Merge pull request #30473 from eduNEXT/mfmz/mfe-config-api feat: add mfe config api * feat!: change names of dynamic MFE config Django settings Formerly, the settings were: * `MFE_CONFIG` for common config. * `MFE_CONFIG_<APP_ID>` for app-specific overrides, with each app getting its own Django setting. This commit changes it to: * `MFE_CONFIG` for common config (unchanged) * `MFE_CONFIG_OVERRIDES` for app-specific overrides, where each app gets a top-level key in the dictionary. Why the change? * We want common.py to have a complete list of overridable settings, as it helps operators reason about configuration and allows us to generate config documentation using toggle annotations. Dynamically generating setting names based on arbitrary APP_IDs makes this impossible. * getattr(...) generally makes code more complicated bug prone. Tools like pylint and mypy cannot effectively analyze any code that uses dynamic attribute access. * docs: add more detail to MFE Config API documentation No functional changes here. This just uses the edx_api_doc_tools package to add some additional documentation to the new API. The documentation can be read from the code, or viewed by visiting http://<LMS_ROOT>/api-docs and searching for "mfe_config". * feat!: change /api/v1/mfe_config to /api/mfe_config/v1 * This changes the API's path. The reasoning is that this is Version 1 of the mfe_config API, not Version 1 of the LMS's entire API, so the v1 should come after mfe_config. * Why does this matter? Firstly, consistency. Secondly, it affects our generated API documentation. If you visited https://courses.edx.org/api-docs, you could see that the API was listed under "v1" instead of "mfe_config". --------- Co-authored-by: Felipe Montoya <felipe.montoya@edunext.co> Co-authored-by: Kyle McCormick <kdmc@pm.me>
- Loading branch information
3 people
authored
Jun 17, 2023
1 parent
7e37b5a
commit a91a5a8
Showing
11 changed files
with
405 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
72 changes: 72 additions & 0 deletions
72
lms/djangoapps/mfe_config_api/docs/decisions/0001-mfe-config-api.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| 0001 MFE CONFIG API | ||
| #################### | ||
|
|
||
| Status | ||
| ****** | ||
|
|
||
| Accepted | ||
|
|
||
| Context | ||
| ******* | ||
|
|
||
| Currently, MFE settings are set via command line environment variables or an .env file that is read during the build process, causing the operators to rebuild mfes each time when any variables are changed. The creation of the ``mfe_config_api`` allows configuration at runtime and avoids rebuilds. | ||
| `MFE Configuration during Runtime`_. | ||
|
|
||
| Decision | ||
| ******** | ||
|
|
||
| - A lightweight API will be created that returns the mfe configuration variables from the site configuration or django settings. `PR Discussion about django settings`_ | ||
| - The API will be enabled or disabled using the setting ``ENABLE_MFE_CONFIG_API``. | ||
| - The API will take the mfe configuration in the ``MFE_CONFIG`` keyset in the site configuration (admin > site configuration > your domain) or in django settings. | ||
| - This API allows to consult the configurations by specific MFE. Making a request like ``/api/mfe_config/v1?mfe=mymfe`` will return the configuration defined in ``MFE_CONFIG_OVERRIDES["mymfe"]`` merged with the ``MFE_CONFIG`` configuration. | ||
| - The API will have a mechanism to cache the response with ``MFE_CONFIG_API_CACHE_TIMEOUT`` variable. | ||
| - The API will live in lms/djangoapps because this is not something Studio needs to serve and it is a lightweight API. `PR Discussion`_ | ||
| - The API will not require authentication or authorization. | ||
| - The API request and response will be like: | ||
|
|
||
| Request:: | ||
|
|
||
| GET http://lms.base.com/api/mfe_config/v1?mfe=learning | ||
|
|
||
| Response:: | ||
|
|
||
| { | ||
| "BASE_URL": "https://name_of_mfe.example.com", | ||
| "LANGUAGE_PREFERENCE_COOKIE_NAME": "example-language-preference", | ||
| "CREDENTIALS_BASE_URL": "https://credentials.example.com", | ||
| "DISCOVERY_API_BASE_URL": "https://discovery.example.com", | ||
| "LMS_BASE_URL": "https://courses.example.com", | ||
| "LOGIN_URL": "https://courses.example.com/login", | ||
| "LOGOUT_URL": "https://courses.example.com/logout", | ||
| "STUDIO_BASE_URL": "https://studio.example.com", | ||
| "LOGO_URL": "https://courses.example.com/logo.png" | ||
|
|
||
| } | ||
|
|
||
| Consequences | ||
| ************ | ||
|
|
||
| - We have to change all the mfes so that they take the information from the API. `Issue MFE runtime configuration in frontend-wg`_ | ||
| - Initialize the MFE could have a delay due to the HTTP method. | ||
| - `Site configuration is going to be deprecated`_ so later we have to clean the code that uses site configuration. | ||
| - The operator is responsible for configuring the settings in site configuration or django settings. | ||
| - We can have duplicate keys in site configuration (example: we can have a logo definition for each mfe). | ||
| - If the request is made from a domain that does not have a site configuration, it returns django settings. | ||
|
|
||
| Rejected Alternatives | ||
| ********************** | ||
|
|
||
| - It was not made as a plugin or IDA because it is a lightweight implementation `PR Discussion`_ | ||
|
|
||
| References | ||
| ********** | ||
|
|
||
| .. _MFE Configuration during Runtime: https://docs.google.com/document/d/1-FHIQmyeQZu3311x8eYUNMru4JX7Yb3UlqjmJxvM8do/edit?usp=sharing | ||
|
|
||
| .. _PR Discussion: https://github.com/openedx/edx-platform/pull/30473#issuecomment-1146176151 | ||
|
|
||
| .. _Site configuration is going to be deprecated: https://github.com/openedx/platform-roadmap/issues/21 | ||
|
|
||
| .. _Issue MFE runtime configuration in frontend-wg: https://github.com/openedx/frontend-wg/issues/103 | ||
|
|
||
| .. _PR Discussion about django settings: https://github.com/openedx/edx-platform/pull/30473#discussion_r916263245 |
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| """ | ||
| Test the use cases of the views of the mfe api. | ||
| """ | ||
|
|
||
| from unittest.mock import call, patch | ||
|
|
||
| import ddt | ||
| from django.conf import settings | ||
| from django.test import override_settings | ||
| from django.urls import reverse | ||
| from rest_framework import status | ||
| from rest_framework.test import APITestCase | ||
|
|
||
|
|
||
| @ddt.ddt | ||
| class MFEConfigTestCase(APITestCase): | ||
| """ | ||
| Test the use case that exposes the site configuration with the mfe api. | ||
| """ | ||
| def setUp(self): | ||
| self.mfe_config_api_url = reverse("mfe_config_api:config") | ||
| return super().setUp() | ||
|
|
||
| @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") | ||
| def test_get_mfe_config(self, configuration_helpers_mock): | ||
| """Test the get mfe config from site configuration with the mfe api. | ||
| Expected result: | ||
| - The get_value method of the configuration_helpers in the views is called once with the | ||
| parameters ("MFE_CONFIG", settings.MFE_CONFIG) | ||
| - The status of the response of the request is a HTTP_200_OK. | ||
| - The json of the response of the request is equal to the mocked configuration. | ||
| """ | ||
| configuration_helpers_mock.get_value.return_value = {"EXAMPLE_VAR": "value"} | ||
| response = self.client.get(self.mfe_config_api_url) | ||
|
|
||
| configuration_helpers_mock.get_value.assert_called_once_with("MFE_CONFIG", settings.MFE_CONFIG) | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertEqual(response.json(), {"EXAMPLE_VAR": "value"}) | ||
|
|
||
| @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") | ||
| def test_get_mfe_config_with_queryparam(self, configuration_helpers_mock): | ||
| """Test the get mfe config with a query param from site configuration. | ||
| Expected result: | ||
| - The get_value method of the configuration_helpers in the views is called twice, once with the | ||
| parameters ("MFE_CONFIG", settings.MFE_CONFIG) | ||
| and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES). | ||
| - The json of the response is the merge of both mocked configurations. | ||
| """ | ||
| configuration_helpers_mock.get_value.side_effect = [ | ||
| {"EXAMPLE_VAR": "value", "OTHER": "other"}, | ||
| {"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, | ||
| ] | ||
|
|
||
| response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| calls = [call("MFE_CONFIG", settings.MFE_CONFIG), | ||
| call("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES)] | ||
| configuration_helpers_mock.get_value.assert_has_calls(calls) | ||
| self.assertEqual(response.json(), {"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}) | ||
|
|
||
| @ddt.unpack | ||
| @ddt.data( | ||
| dict( | ||
| mfe_config={}, | ||
| mfe_config_overrides={}, | ||
| expected_response={}, | ||
| ), | ||
| dict( | ||
| mfe_config={"EXAMPLE_VAR": "value"}, | ||
| mfe_config_overrides={}, | ||
| expected_response={"EXAMPLE_VAR": "value"}, | ||
| ), | ||
| dict( | ||
| mfe_config={}, | ||
| mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, | ||
| expected_response={"EXAMPLE_VAR": "mymfe_value"}, | ||
| ), | ||
| dict( | ||
| mfe_config={"EXAMPLE_VAR": "value"}, | ||
| mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, | ||
| expected_response={"EXAMPLE_VAR": "mymfe_value"}, | ||
| ), | ||
| dict( | ||
| mfe_config={"EXAMPLE_VAR": "value", "OTHER": "other"}, | ||
| mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, | ||
| expected_response={"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}, | ||
| ), | ||
| dict( | ||
| mfe_config={"EXAMPLE_VAR": "value"}, | ||
| mfe_config_overrides={"yourmfe": {"EXAMPLE_VAR": "yourmfe_value"}}, | ||
| expected_response={"EXAMPLE_VAR": "value"}, | ||
| ), | ||
| dict( | ||
| mfe_config={"EXAMPLE_VAR": "value"}, | ||
| mfe_config_overrides={ | ||
| "yourmfe": {"EXAMPLE_VAR": "yourmfe_value"}, | ||
| "mymfe": {"EXAMPLE_VAR": "mymfe_value"}, | ||
| }, | ||
| expected_response={"EXAMPLE_VAR": "mymfe_value"}, | ||
| ), | ||
| ) | ||
| @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") | ||
| def test_get_mfe_config_with_queryparam_multiple_configs( | ||
| self, | ||
| configuration_helpers_mock, | ||
| mfe_config, | ||
| mfe_config_overrides, | ||
| expected_response, | ||
| ): | ||
| """Test the get mfe config with a query param and different settings in mfe_config and mfe_config_overrides with | ||
| the site configuration to test that the merge of the configurations is done correctly and mymfe config take | ||
| precedence. | ||
| Expected result: | ||
| - The get_value method of the configuration_helpers in the views is called twice, once with the | ||
| parameters ("MFE_CONFIG", settings.MFE_CONFIG) | ||
| and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES). | ||
| - The json of the response is the expected_response passed by ddt.data. | ||
| """ | ||
| configuration_helpers_mock.get_value.side_effect = [mfe_config, mfe_config_overrides] | ||
|
|
||
| response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| calls = [call("MFE_CONFIG", settings.MFE_CONFIG), | ||
| call("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES)] | ||
| configuration_helpers_mock.get_value.assert_has_calls(calls) | ||
| self.assertEqual(response.json(), expected_response) | ||
|
|
||
| def test_get_mfe_config_from_django_settings(self): | ||
| """Test that when there is no site configuration, the API takes the django settings. | ||
| Expected result: | ||
| - The status of the response of the request is a HTTP_200_OK. | ||
| - The json response is equal to MFE_CONFIG in lms/envs/test.py""" | ||
| response = self.client.get(self.mfe_config_api_url) | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertEqual(response.json(), settings.MFE_CONFIG) | ||
|
|
||
| def test_get_mfe_config_with_queryparam_from_django_settings(self): | ||
| """Test that when there is no site configuration, the API with queryparam takes the django settings. | ||
| Expected result: | ||
| - The status of the response of the request is a HTTP_200_OK. | ||
| - The json response is equal to MFE_CONFIG merged with MFE_CONFIG_OVERRIDES['mymfe'] | ||
| """ | ||
| response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| expected = {**settings.MFE_CONFIG, **settings.MFE_CONFIG_OVERRIDES["mymfe"]} | ||
| self.assertEqual(response.json(), expected) | ||
|
|
||
| @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") | ||
| @override_settings(ENABLE_MFE_CONFIG_API=False) | ||
| def test_404_get_mfe_config(self, configuration_helpers_mock): | ||
| """Test the 404 not found response from get mfe config. | ||
| Expected result: | ||
| - The get_value method of configuration_helpers is not called. | ||
| - The status of the response of the request is a HTTP_404_NOT_FOUND. | ||
| """ | ||
| response = self.client.get(self.mfe_config_api_url) | ||
| configuration_helpers_mock.get_value.assert_not_called() | ||
| self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| """ URLs configuration for the mfe api.""" | ||
|
|
||
| from django.urls import path | ||
|
|
||
| from lms.djangoapps.mfe_config_api.views import MFEConfigView | ||
|
|
||
| app_name = 'mfe_config_api' | ||
| urlpatterns = [ | ||
| path('', MFEConfigView.as_view(), name='config'), | ||
| ] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| """ | ||
| MFE API Views for useful information related to mfes. | ||
| """ | ||
|
|
||
| import edx_api_doc_tools as apidocs | ||
| from django.conf import settings | ||
| from django.http import HttpResponseNotFound, JsonResponse | ||
| from django.utils.decorators import method_decorator | ||
| from django.views.decorators.cache import cache_page | ||
| from rest_framework import status | ||
| from rest_framework.views import APIView | ||
|
|
||
| from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers | ||
|
|
||
|
|
||
| class MFEConfigView(APIView): | ||
| """ | ||
| Provides an API endpoint to get the MFE configuration from settings (or site configuration). | ||
| """ | ||
|
|
||
| @method_decorator(cache_page(settings.MFE_CONFIG_API_CACHE_TIMEOUT)) | ||
| @apidocs.schema( | ||
| parameters=[ | ||
| apidocs.query_parameter( | ||
| 'mfe', | ||
| str, | ||
| description="Name of an MFE (a.k.a. an APP_ID).", | ||
| ), | ||
| ], | ||
| ) | ||
| def get(self, request): | ||
| """ | ||
| Return the MFE configuration, optionally including MFE-specific overrides. | ||
| **Usage** | ||
| Get common config: | ||
| GET /api/mfe_config/v1 | ||
| Get app config (common + app-specific overrides): | ||
| GET /api/mfe_config/v1?mfe=name_of_mfe | ||
| **GET Response Values** | ||
| ``` | ||
| { | ||
| "BASE_URL": "https://name_of_mfe.example.com", | ||
| "LANGUAGE_PREFERENCE_COOKIE_NAME": "example-language-preference", | ||
| "CREDENTIALS_BASE_URL": "https://credentials.example.com", | ||
| "DISCOVERY_API_BASE_URL": "https://discovery.example.com", | ||
| "LMS_BASE_URL": "https://courses.example.com", | ||
| "LOGIN_URL": "https://courses.example.com/login", | ||
| "LOGOUT_URL": "https://courses.example.com/logout", | ||
| "STUDIO_BASE_URL": "https://studio.example.com", | ||
| "LOGO_URL": "https://courses.example.com/logo.png", | ||
| ... and so on | ||
| } | ||
| ``` | ||
| """ | ||
|
|
||
| if not settings.ENABLE_MFE_CONFIG_API: | ||
| return HttpResponseNotFound() | ||
|
|
||
| mfe_config = configuration_helpers.get_value('MFE_CONFIG', settings.MFE_CONFIG) | ||
| if request.query_params.get('mfe'): | ||
| mfe = str(request.query_params.get('mfe')) | ||
| app_config = configuration_helpers.get_value( | ||
| 'MFE_CONFIG_OVERRIDES', | ||
| settings.MFE_CONFIG_OVERRIDES, | ||
| ) | ||
| mfe_config.update(app_config.get(mfe, {})) | ||
| return JsonResponse(mfe_config, status=status.HTTP_200_OK) |
Oops, something went wrong.