Skip to content

Commit

Permalink
feat(extension): Block on missing required config (#2187)
Browse files Browse the repository at this point in the history
This PR is related to 12 Factor [Block on missing required config
PR](canonical/paas-charm#42)

## Rationale

Some applications require certain config options to run and would crash
if these options are not provided. To prevent this, we need to add an
optional field to the config options and block the charm if a config
option marked as non-optional has not been provided with a message that
explains which config options are missing.

To stay backward compatible the default value for the optional field
will be true.

Since it doesn’t make sense for config options that are non-optional to
have a default value, charmcraft will look for this condition at pack
time and error out.


## Extension Changes

Adds a check for non-optional options with default values and throws
`ExtensionError` when found.
Adds documentation about the non-optional options.


Reviewers:
@jdkandersson 
@javierdelapuente 
@erinecon
  • Loading branch information
alithethird authored Feb 26, 2025
1 parent ecd6b41 commit c65d0dc
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 5 deletions.
14 changes: 14 additions & 0 deletions charmcraft/extensions/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def _check_input(self) -> None:
f"which conflict with the {self.framework}-framework extension, "
"please rename or remove it"
)
invalid_non_optionals = []
for config in self._get_nested(self.yaml_data, "config.options"):
for reserved_config_prefix in ("webserver-", f"{self.framework}-"):
if config.startswith(reserved_config_prefix):
Expand All @@ -122,6 +123,19 @@ def _check_input(self) -> None:
f" reserved configuration prefix {reserved_config_prefix!r}, "
"please rename or remove it"
)
config_option_dict = self._get_nested(
self.yaml_data, f"config.options.{config}"
)
if config_option_dict.get("optional") is False and config_option_dict.get(
"default"
):
invalid_non_optionals.append(config)

if invalid_non_optionals:
raise ExtensionError(
"Non-optional configuration options can not have default values.\n"
f"Please either remove the default value or set optional field to true or remove it for the {', '.join(invalid_non_optionals)} configuration option(s)."
)

def _get_root_snippet(self) -> dict[str, Any]:
"""Return the root snippet to be merged into the user charmcraft.yaml.
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/extensions/django-framework-extension.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ will set the ``DJANGO_ALLOWED_HOSTS`` environment variable, the ingress
URL or the Kubernetes service URL if there is no ingress integration,
will be set automatically.

.. include:: /reuse/reference/extensions/non_optional_config.rst


``peers``, ``provides``, and ``requires`` keys
----------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion docs/reference/extensions/fastapi-framework-extension.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ by running ``juju config <application> token=<token>``.
token:
description: The token for the service.
type: string
required: true
optional: false
.. include:: /reuse/reference/extensions/non_optional_config.rst


``charmcraft.yaml`` > ``peers``, ``provides``, ``requires``
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/extensions/flask-framework-extension.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ charm can set it by running ``juju config <application> token=<token>``.
description: The token for the service.
type: string
.. include:: /reuse/reference/extensions/non_optional_config.rst


``charmcraft.yaml`` > ``peers``, ``provides``, ``requires``
-----------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/extensions/go-framework-extension.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ charm can set it by running ``juju config <application> token=<token>``.
description: The token for the service.
type: string
.. include:: /reuse/reference/extensions/non_optional_config.rst


``charmcraft.yaml`` > ``peers``, ``provides``, ``requires``
-----------------------------------------------------------
Expand Down
22 changes: 22 additions & 0 deletions docs/reuse/reference/extensions/non_optional_config.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

In addition to this, you can set the configuration option to be
mandatory by setting the ``optional`` key to ``false``. This will
block the charm and stop services until the configuration is supplied. For example,
if your application needs an ``api-token`` to function correctly you can set
``optional``, as shown below. This will block the charm and stop the
services until the ``api-token`` configuration is supplied.

.. code-block:: yaml
:caption: charmcraft.yaml
config:
options:
api-token:
description: The token necessary for the service to run.
type: string
optional: false
.. note::

A configuration with ``optional: false`` can't also have a ``default`` key.
If it has both, the charm will fail to pack.
48 changes: 44 additions & 4 deletions tests/extensions/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@
GoFramework,
)

NON_OPTIONAL_OPTIONS = {
"options": {
"non-optional-string": {
"description": "Example of a non-optional string configuration option.",
"type": "string",
"optional": False,
}
}
}


def make_flask_input_yaml():
return {
Expand All @@ -33,6 +43,7 @@ def make_flask_input_yaml():
"description": "test description",
"bases": [{"name": "ubuntu", "channel": "22.04"}],
"extensions": ["flask-framework"],
"config": NON_OPTIONAL_OPTIONS,
}


Expand Down Expand Up @@ -69,7 +80,10 @@ def flask_input_yaml_fixture():
{"lib": "tempo_coordinator_k8s.tracing", "version": "0"},
],
"config": {
"options": {**FlaskFramework.options},
"options": {
**FlaskFramework.options,
**NON_OPTIONAL_OPTIONS["options"],
},
},
"parts": {
"charm": {
Expand Down Expand Up @@ -114,6 +128,7 @@ def flask_input_yaml_fixture():
"s390x": None,
},
"extensions": ["django-framework"],
"config": NON_OPTIONAL_OPTIONS,
},
False,
{
Expand Down Expand Up @@ -146,7 +161,10 @@ def flask_input_yaml_fixture():
{"lib": "tempo_coordinator_k8s.tracing", "version": "0"},
],
"config": {
"options": {**DjangoFramework.options},
"options": {
**DjangoFramework.options,
**NON_OPTIONAL_OPTIONS["options"],
},
},
"parts": {
"charm": {
Expand Down Expand Up @@ -186,6 +204,7 @@ def flask_input_yaml_fixture():
"amd64": None,
},
"extensions": ["go-framework"],
"config": NON_OPTIONAL_OPTIONS,
},
True,
{
Expand Down Expand Up @@ -213,7 +232,10 @@ def flask_input_yaml_fixture():
{"lib": "tempo_coordinator_k8s.tracing", "version": "0"},
],
"config": {
"options": {**GoFramework.options},
"options": {
**GoFramework.options,
**NON_OPTIONAL_OPTIONS["options"],
},
},
"parts": {
"charm": {
Expand Down Expand Up @@ -253,6 +275,7 @@ def flask_input_yaml_fixture():
"amd64": None,
},
"extensions": ["fastapi-framework"],
"config": NON_OPTIONAL_OPTIONS,
},
True,
{
Expand Down Expand Up @@ -280,7 +303,10 @@ def flask_input_yaml_fixture():
{"lib": "tempo_coordinator_k8s.tracing", "version": "0"},
],
"config": {
"options": {**FastAPIFramework.options},
"options": {
**FastAPIFramework.options,
**NON_OPTIONAL_OPTIONS["options"],
},
},
"parts": {
"charm": {
Expand Down Expand Up @@ -404,6 +430,20 @@ def test_flask_merge_charm_libs(flask_input_yaml, tmp_path):
{"config": {"options": {"flask-foobar": {"type": "string"}}}},
id="reserved-config-prefix-flask",
),
pytest.param(
{
"config": {
"options": {
"non-optional": {
"type": "string",
"optional": False,
"default": "default value",
}
}
}
},
id="non-optional-config-with-default",
),
]


Expand Down

0 comments on commit c65d0dc

Please sign in to comment.