Model config types are incorrect

[tonyandrewmeyer]

Model.config is typed to return a Model.ConfigData, which is a subclass of Model.LazyMapping, which is an ABC of Mapping[str, str]. However, the config is actually Mapping[str, int|float|str|bool].

RelationDataContent is also a LazyMapping subclass, and there the data is always [str, str].

The data is loaded into the lazy mapping via _ModelBackend.config_get, which is typed to return a dict[str, Model._ConfigOption]. Model._ConfigOption has a type literal string, a description string, and a default value (str, int, float, bool) - but that's the format for a dict loaded from the config/charmcraft YAML file. What we get from config-get, which is how the data is loaded, is:

root@secretconfig-0:/var/lib/juju/agents/unit-secretconfig-0/charm# ../../../tools/unit-secretconfig-0/config-get --format=json | jq
{
  "booleanopt": false,
  "floatopt": 3.14,
  "intopt": 42,
  "secretopt": "secret:co6tjmvmp25c77dhf980",
  "stringopt": "hello"
}

Problems:

• LazyMapping should be more loosely typed, e.g. Mapping[str, int|float|str|bool]
• _ConfigOption.type can be secret
• _ModelBackend.config_get should return a dict[str, int|float|str|bool]

[tonyandrewmeyer]

It seems like pyright should be picking up at least some of these, so that should be checked as well.

[tonyandrewmeyer]

_ConfigOption is odd - it's not actually used in model.py at all, so it feels like it's in the wrong file. We don't load the config in CharmMeta, so it's only used in Harness, and only to define _RawConfig:

_RawConfig = TypedDict("_RawConfig", {'options': Dict[str, _ConfigOption]})

pyright doesn't pick up this issue because what we're basically doing is saying "give me a dict from this YAML string and you should believe it's this type". We don't have any schema validation for the config metadata.

[tonyandrewmeyer]

pyright can't pick up the wrong type on config_get for a similar reason - we don't do any schema validation of what we get back from config-get - we just cast it to the type that we're expecting. There's no return type on ConfigData._load so pyright implicitly sets it to whatever the type of config_get is (even though that's different than the parent class). I think it fails to notice that data = self._lazy_data = self._load() has mismatching types because of the parent class definition. With ConfigData._load explicitly typed it does pick up problems.

[tonyandrewmeyer]

For LazyMapping, pyright doesn't pick it up because anywhere config is being used, you need to cast (or type:ignore) it to something other than str to do anything non-string with it, since it could actually be a string. So that masks the problem. For example, you can have code like:

p = ops.Port('tcp', self.config["port"])

And pyright will complain, because self.config[x] isn't an int - but it still might not be after the fix, either. Basically, the config typing is too dynamic for pyright to figure it out, unless we put in some sort of schema validation.

[benhoyt]

Excellent report and Pyright investigation -- thanks Tony.