Merge pull request #106 from tonyandrewmeyer/config-type-secret

fix: correct the consistency checker's config "type" validation

scenario/consistency_checker.py

@@ -2,10 +2,11 @@
 # Copyright 2023 Canonical Ltd.
 # See LICENSE file for licensing details.
 import os
+import re
 from collections import Counter
 from collections.abc import Sequence
 from numbers import Number
-from typing import TYPE_CHECKING, Iterable, List, NamedTuple, Tuple
+from typing import TYPE_CHECKING, Iterable, List, NamedTuple, Tuple, Union
 
 from scenario.runtime import InconsistentScenarioError
 from scenario.runtime import logger as scenario_logger
@@ -326,10 +327,17 @@ def check_storages_consistency(
     return Results(errors, [])
 
 
+def _is_secret_identifier(value: Union[str, int, float, bool]) -> bool:
+    """Return true iff the value is in the form `secret:{secret id}`."""
+    # cf. https://github.com/juju/juju/blob/13eb9df3df16a84fd471af8a3c95ddbd04389b71/core/secrets/secret.go#L48
+    return bool(re.match(r"secret:[0-9a-z]{20}$", str(value)))
+
+
 def check_config_consistency(
     *,
     state: "State",
     charm_spec: "_CharmSpec",
+    juju_version: Tuple[int, ...],
     **_kwargs,  # noqa: U101
 ) -> Results:
     """Check the consistency of the state.config with the charm_spec.config (config.yaml)."""
@@ -348,16 +356,21 @@ def check_config_consistency(
         converters = {
             "string": str,
             "int": int,
-            "integer": int,  # fixme: which one is it?
-            "number": float,
+            "float": float,
             "boolean": bool,
-            # "attrs": NotImplemented,  # fixme: wot?
+        }
+        if juju_version >= (3, 4):
+            converters["secret"] = str
+
+        validators = {
+            "secret": _is_secret_identifier,
         }
 
         expected_type_name = meta_config[key].get("type", None)
         if not expected_type_name:
             errors.append(f"config.yaml invalid; option {key!r} has no 'type'.")
             continue
+        validator = validators.get(expected_type_name)
 
         expected_type = converters.get(expected_type_name)
         if not expected_type:
@@ -371,6 +384,11 @@ def check_config_consistency(
                 f"but is of type {type(value)}.",
             )
 
+        elif validator and not validator(value):
+            errors.append(
+                f"config invalid: option {key!r} value {value!r} is not valid.",
+            )
+
     return Results(errors, [])
 
 

tests/test_consistency_checker.py

@@ -156,6 +156,69 @@ def test_bad_config_option_type():
     )
 
 
+@pytest.mark.parametrize(
+    "config_type",
+    (
+        ("string", "foo", 1),
+        ("int", 1, "1"),
+        ("float", 1.0, 1),
+        ("boolean", False, "foo"),
+    ),
+)
+def test_config_types(config_type):
+    type_name, valid_value, invalid_value = config_type
+    assert_consistent(
+        State(config={"foo": valid_value}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": type_name}}}),
+    )
+    assert_inconsistent(
+        State(config={"foo": invalid_value}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": type_name}}}),
+    )
+
+
+@pytest.mark.parametrize("juju_version", ("3.4", "3.5", "4.0"))
+def test_config_secret(juju_version):
+    assert_consistent(
+        State(config={"foo": "secret:co28kefmp25c77utl3n0"}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+        juju_version=juju_version,
+    )
+    assert_inconsistent(
+        State(config={"foo": 1}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+    )
+    assert_inconsistent(
+        State(config={"foo": "co28kefmp25c77utl3n0"}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+    )
+    assert_inconsistent(
+        State(config={"foo": "secret:secret"}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+    )
+    assert_inconsistent(
+        State(config={"foo": "secret:co28kefmp25c77utl3n!"}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+    )
+
+
+@pytest.mark.parametrize("juju_version", ("2.9", "3.3"))
+def test_config_secret_old_juju(juju_version):
+    assert_inconsistent(
+        State(config={"foo": "secret:co28kefmp25c77utl3n0"}),
+        Event("bar"),
+        _CharmSpec(MyCharm, {}, config={"options": {"foo": {"type": "secret"}}}),
+        juju_version=juju_version,
+    )
+
+
 @pytest.mark.parametrize("bad_v", ("1.0", "0", "1.2", "2.35.42", "2.99.99", "2.99"))
 def test_secrets_jujuv_bad(bad_v):
     secret = Secret("secret:foo", {0: {"a": "b"}})

tests/test_e2e/test_config.py

@@ -32,7 +32,7 @@ def check_cfg(charm: CharmBase):
         "update_status",
         mycharm,
         meta={"name": "foo"},
-        config={"options": {"foo": {"type": "string"}, "baz": {"type": "integer"}}},
+        config={"options": {"foo": {"type": "string"}, "baz": {"type": "int"}}},
         post_event=check_cfg,
     )