Skip to content

Commit

Permalink
fix: make JujuConfig validation accept empty options (#1424)
Browse files Browse the repository at this point in the history
fixes #1398
  • Loading branch information
lengau authored Dec 7, 2023
1 parent dec4d61 commit c1d3ad2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 11 deletions.
8 changes: 8 additions & 0 deletions charmcraft/metafiles/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ def parse_config_yaml(charm_dir: pathlib.Path, allow_broken=False) -> Optional[J
except OSError as exc:
raise CraftError(f"Cannot read the {JUJU_CONFIG_FILENAME} file: {exc!r}") from exc

if allow_broken and (not isinstance(config, dict) or not config.get("options")):
emit.progress(
"'config.yaml' is not a valid config file.",
permanent=True,
)
emit.debug(f"Ignoring {JUJU_CONFIG_FILENAME}")
return None

emit.debug(f"Validating {JUJU_CONFIG_FILENAME}")
try:
return JujuConfig.parse_obj(config)
Expand Down
8 changes: 6 additions & 2 deletions charmcraft/models/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

"""Charmcraft Juju Config pydantic model."""

from typing import Dict, Optional
from typing import Any, Dict, Optional

import pydantic

Expand All @@ -29,11 +29,15 @@ class JujuConfig(ModelConfigDefaults):
See also: https://juju.is/docs/sdk/config
"""

options: Optional[Dict[str, Dict]]
options: Optional[Dict[str, Dict[str, Any]]]

@pydantic.validator("options", pre=True)
def validate_actions(cls, options):
"""Verify options section."""
if options is None:
return None
if not isinstance(options, dict):
raise ValueError("'options' is not a dictionary")
for name, option in options.items():
if not isinstance(option, dict):
raise ValueError(f"'{name}' is not a dictionary")
Expand Down
22 changes: 13 additions & 9 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,18 @@ def test_load_config_in_config_yaml(tmp_path, prepare_charmcraft_yaml, prepare_c
}


def test_load_bad_config_in_config_yaml(tmp_path, prepare_charmcraft_yaml, prepare_config_yaml):
@pytest.mark.parametrize(
"config_yaml",
[
"",
"options:",
"options:\n test-int:",
"options: not a dict",
],
)
def test_load_bad_config_in_config_yaml(
tmp_path, prepare_charmcraft_yaml, prepare_config_yaml, config_yaml
):
"""Load a bad config in config.yaml. Should not raise an error since check unenforced."""
prepare_charmcraft_yaml(
dedent(
Expand All @@ -1206,14 +1217,7 @@ def test_load_bad_config_in_config_yaml(tmp_path, prepare_charmcraft_yaml, prepa
"""
),
)
prepare_config_yaml(
dedent(
"""
options:
test-int:
"""
),
)
prepare_config_yaml(config_yaml)
config = load(tmp_path)

assert config.config is None
Expand Down

0 comments on commit c1d3ad2

Please sign in to comment.