Repository: canonical/operator

Issue:
CharmMeta class needs to be updated for Metadata v2
The `CharmMeta` class currently only represents the v1 Metadata spec - it needs to be updated to support metadata v2: https://discourse.charmhub.io/t/charm-metadata-v2/3674



Discussion:
tonyandrewmeyer: @benhoyt this needs the new classes added to `__init__.py`, tests, and a few other bits of cleanup (including whatever is failing in the CI, likely formatting). Before I do those, could you quickly check that this is more-or-less what you intended for #498? No need for a full review - that can be done when I've finished everything.
tonyandrewmeyer: > Yep, this looks like the basic idea, thanks! I haven't gone over in detail.

Thanks! Just needed a santy check :smile:. You can go over in detail when I finish it off :smile:.

> What spec were you working against?

The one linked in the description: https://juju.is/docs/sdk/metadata-yaml - the Discourse post linked from the issue says that it should be used rather than the Discourse post.
tonyandrewmeyer: FYI, I did a basic sanity check across the ~150 charms I have locally, and loading the metadata doesn't crash:

```python
import pathlib

import ops

root = pathlib.Path(...)

for charm in root.iterdir():
    meta = charm / 'metadata.yaml'
    if meta.exists():
        assert ops.CharmMeta.from_yaml(meta.open()).name
```

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index a26f33c..c0049d3 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,6 +2,8 @@
 
 * Added support for Pebble Notices (`PebbleCustomNoticeEvent`, `get_notices`, and so on)
 * Added `Relation.active`, and excluded inactive relations from `Model.relations`
+* Added full support for charm metadata v2 (in particular, extended `ContainerMeta`,
+  and various info links in `CharmMeta`)
 
 # 2.9.0
 
diff --git a/docs/conf.py b/docs/conf.py
index 1e9bedf..8419739 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -87,6 +87,7 @@ nitpick_ignore = [
     ('py:class', '_TextOrBinaryIO'),
     ('py:class', '_WarningDict'),
     ('py:class', '_Writeable'),
+    ('py:class', 'ops.charm._ContainerBaseDict'),
     ('py:class', 'ops.model._AddressDict'),
     ('py:class', 'ops.model._ConfigOption'),
     ('py:class', 'ops.model._ModelBackend'),
diff --git a/ops/__init__.py b/ops/__init__.py
index 99c8c39..11dcfeb 100644
--- a/ops/__init__.py
+++ b/ops/__init__.py
@@ -55,12 +55,16 @@ __all__ = [
     'CollectMetricsEvent',
     'CollectStatusEvent',
     'ConfigChangedEvent',
+    'ContainerBase',
     'ContainerMeta',
     'ContainerStorageMeta',
     'HookEvent',
     'InstallEvent',
+    'JujuAssumes',
+    'JujuAssumesCondition',
     'LeaderElectedEvent',
     'LeaderSettingsChangedEvent',
+    'MetadataLinks',
     'PayloadMeta',
     'PebbleCustomNoticeEvent',
     'PebbleNoticeEvent',
@@ -191,12 +195,16 @@ from .charm import (  # noqa: F401
     CollectMetricsEvent,
     CollectStatusEvent,
     ConfigChangedEvent,
+    ContainerBase,
     ContainerMeta,
     ContainerStorageMeta,
     HookEvent,
     InstallEvent,
+    JujuAssumes,
+    JujuAssumesCondition,
     LeaderElectedEvent,
     LeaderSettingsChangedEvent,
+    MetadataLinks,
     PayloadMeta,
     PebbleCustomNoticeEvent,
     PebbleNoticeEvent,
diff --git a/ops/charm.py b/ops/charm.py
index 310db0f..317e9bf 100644
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -14,6 +14,7 @@
 
 """Base objects for the Charm, events and metadata."""
 
+import dataclasses
 import enum
 import logging
 import pathlib
@@ -81,6 +82,12 @@ if TYPE_CHECKING:
         total=False)
 
 
+class _ContainerBaseDict(TypedDict):
+    name: str
+    channel: str
+    architectures: List[str]
+
+
 logger = logging.getLogger(__name__)
 
 
@@ -671,8 +678,7 @@ class WorkloadEvent(HookEvent):
     """Base class representing workload-related events.
 
     Workload events are generated for all containers that the charm
-    expects in metadata. Workload containers currently only trigger
-    a :class:`PebbleReadyEvent`.
+    expects in metadata.
     """
 
     workload: 'model.Container'
@@ -1211,6 +1217,9 @@ class CharmMeta:
     maintainers: List[str]
     """List of email addresses of charm maintainers."""
 
+    links: 'MetadataLinks'
+    """Links to more details about the charm."""
+
     tags: List[str]
     """Charmhub tag metadata for categories associated with this charm."""
 
@@ -1230,6 +1239,9 @@ class CharmMeta:
     min_juju_version: Optional[str]
     """Indicates the minimum Juju version this charm requires."""
 
+    assumes: 'JujuAssumes'
+    """Juju features this charm requires."""
+
     containers: Dict[str, 'ContainerMeta']
     """Container metadata for each defined container."""
 
@@ -1270,18 +1282,37 @@ class CharmMeta:
         raw_: Dict[str, Any] = raw or {}
         actions_raw_: Dict[str, Any] = actions_raw or {}
 
+        # When running in production, this data is generally loaded from
+        # metadata.yaml. However, when running tests, this data is
+        # potentially loaded from charmcraft.yaml (which will be split out
+        # into a metadata.yaml as part of packing). Most of the field names
+        # are the same, but there are some differences that we handle here,
+        # and in _load_links(), so that loading from either file works.
         self.name = raw_.get('name', '')
         self.summary = raw_.get('summary', '')
         self.description = raw_.get('description', '')
+        # The metadata spec says that these should be display-name <email>
+        # (roughly 'name-addr' from RFC 5322). However, many charms have only
+        # an email, or have a URL, or something else, so we leave these as
+        # a plain string.
         self.maintainers: List[str] = []
+        # Note that metadata v2 only defines 'maintainers' not 'maintainer'.
         if 'maintainer' in raw_:
             self.maintainers.append(raw_['maintainer'])
         if 'maintainers' in raw_:
             self.maintainers.extend(raw_['maintainers'])
+        if 'links' in raw_ and 'contact' in raw_['links']:
+            self.maintainers.append(raw_['links']['contact'])
+        self._load_links(raw_)
+        # Note that metadata v2 does not define tags.
         self.tags = raw_.get('tags', [])
         self.terms = raw_.get('terms', [])
+        # Note that metadata v2 does not define series.
         self.series = raw_.get('series', [])
         self.subordinate = raw_.get('subordinate', False)
+        self.assumes = JujuAssumes.from_list(raw_.get('assumes', []))
+        # Note that metadata v2 does not define min-juju-version ('assumes'
+        # should be used instead).
         self.min_juju_version = raw_.get('min-juju-version')
         self.requires = {name: RelationMeta(RelationRole.requires, name, rel)
                          for name, rel in raw_.get('requires', {}).items()}
@@ -1301,12 +1332,38 @@ class CharmMeta:
                          for name, payload in raw_.get('payloads', {}).items()}
         self.extra_bindings = raw_.get('extra-bindings', {})
         self.actions = {name: ActionMeta(name, action) for name, action in actions_raw_.items()}
-        # This is taken from Charm Metadata v2, but only the "containers" and
-        # "containers.name" fields that we need right now for Pebble. See:
-        # https://discourse.charmhub.io/t/charm-metadata-v2/3674
         self.containers = {name: ContainerMeta(name, container)
                            for name, container in raw_.get('containers', {}).items()}
 
+    def _load_links(self, raw: Dict[str, Any]):
+        websites = raw.get('website', [])
+        if not websites and 'links' in raw:
+            websites = raw['links'].get('website', [])
+        # In YAML, this can be a single string, or a list of strings.
+        if isinstance(websites, str):
+            websites = [websites]
+        sources = raw.get('source', [])
+        if not sources and 'links' in raw:
+            sources = raw['links'].get('source', [])
+        # In YAML, this can be a single string, or a list of strings.
+        if isinstance(sources, str):
+            sources = [sources]
+        issues = raw.get('issues', [])
+        if not issues and 'links' in raw:
+            issues = raw['links'].get('issues', [])
+        # In YAML, this can be a single string, or a list of strings.
+        if isinstance(issues, str):
+            issues = [issues]
+        documentation = raw.get('docs')
+        if documentation is None:
+            documentation = raw.get('links', {}).get('documentation')
+        self.links = MetadataLinks(
+            websites=websites,
+            sources=sources,
+            issues=issues,
+            documentation=documentation,
+        )
+
     @classmethod
     def from_yaml(
             cls, metadata: Union[str, TextIO],
@@ -1431,6 +1488,9 @@ class StorageMeta:
     multiple_range: Optional[Tuple[int, Optional[int]]]
     """Range of numeric qualifiers when multiple storage units are used."""
 
+    properties = List[str]
+    """List of additional characteristics of the storage."""
+
     def __init__(self, name: str, raw: '_StorageMetaDict'):
         self.storage_name = name
         self.type = raw['type']
@@ -1447,6 +1507,7 @@ class StorageMeta:
             else:
                 range = range.split('-')
                 self.multiple_range = (int(range[0]), int(range[1]) if range[1] else None)
+        self.properties = raw.get('properties', [])
 
 
 class ResourceMeta:
@@ -1488,6 +1549,60 @@ class PayloadMeta:
         self.type = raw['type']
 
 
+@dataclasses.dataclass(frozen=True)
+class MetadataLinks:
+    """Links to additional information about a charm."""
+
+    websites: List[str]
+    """List of links to project websites."""
+
+    sources: List[str]
+    """List of links to the charm source code."""
+
+    issues: List[str]
+    """List of links to the charm issue tracker."""
+
+    documentation: Optional[str]
+    """Link to charm documentation."""
+
+
+class JujuAssumesCondition(enum.Enum):
+    """Distinguishes between :class:`JujuAssumes` that must match all or any features."""
+
+    ALL = 'all-of'
+    """All features are required to satisfy the requirement."""
+
+    ANY = 'any-of'
+    """Any of the features satisfies the requirement."""
+
+
+@dataclasses.dataclass(frozen=True)
+class JujuAssumes:
+    """Juju model features that are required by the charm.
+
+    See the `Juju docs <https://juju.is/docs/olm/supported-features>`_ for a
+    list of available features.
+    """
+
+    features: List[Union[str, 'JujuAssumes']]
+    condition: JujuAssumesCondition = JujuAssumesCondition.ALL
+
+    @classmethod
+    def from_list(cls, raw: List[Any],
+                  condition: JujuAssumesCondition = JujuAssumesCondition.ALL,
+                  ) -> 'JujuAssumes':
+        """Create new JujuAssumes object from list parsed from YAML."""
+        features: List[Union[str, 'JujuAssumes']] = []
+        for feature in raw:
+            if isinstance(feature, str):
+                features.append(feature)
+            else:
+                for nested_condition, nested_features in feature.items():
+                    features.append(JujuAssumes.from_list(
+                        nested_features, JujuAssumesCondition(nested_condition)))
+        return cls(features=features, condition=condition)
+
+
 class ActionMeta:
     """Object containing metadata about an action's definition."""
 
@@ -1501,23 +1616,69 @@ class ActionMeta:
         self.additional_properties = raw.get('additionalProperties', True)
 
 
-class ContainerMeta:
-    """Metadata about an individual container.
+@dataclasses.dataclass(frozen=True)
+class ContainerBase:
+    """Metadata to resolve a container image."""
 
-    NOTE: this is extremely lightweight right now, and just includes the fields we need for
-    Pebble interaction.
+    os_name: str
+    """Name of the OS.
+
+    For example: ``ubuntu``
+    """
+
+    channel: str
+    """Channel of the OS in format ``track[/risk][/branch]`` as used by Snaps.
+
+    For example: ``20.04/stable`` or ``18.04/stable/fips``
     """
 
+    architectures: List[str]
+    """List of architectures that this charm can run on."""
+
+    @classmethod
+    def from_dict(cls, d: '_ContainerBaseDict') -> 'ContainerBase':
+        """Create new ContainerBase object from dict parsed from YAML."""
+        return cls(
+            os_name=d['name'],
+            channel=d['channel'],
+            architectures=d['architectures'],
+        )
+
+
+class ContainerMeta:
+    """Metadata about an individual container."""
+
     name: str
     """Name of the container (key in the YAML)."""
 
+    resource: Optional[str]
+    """Reference for an entry in the ``resources`` field.
+
+    Specifies the oci-image resource used to create the container. Must not be
+    present if a base/channel is specified.
+    """
+
+    bases: Optional[List['ContainerBase']]
+    """List of bases for use in resolving a container image.
+
+    Sorted by descending order of preference, and must not be present if
+    resource is specified.
+    """
+
     def __init__(self, name: str, raw: Dict[str, Any]):
         self.name = name
         self._mounts: Dict[str, ContainerStorageMeta] = {}
+        self.bases = None
+        self.resource = None
 
         # This is not guaranteed to be populated/is not enforced yet
         if raw:
             self._populate_mounts(raw.get('mounts', []))
+            self.resource = raw.get('resource')
+            self.bases = [ContainerBase.from_dict(base) for base in raw.get('bases', ())]
+
+        if self.resource and self.bases:
+            raise model.ModelError('A container may specify a resource or base, not both.')
 
     @property
     def mounts(self) -> Dict[str, 'ContainerStorageMeta']:
@@ -1572,9 +1733,6 @@ class ContainerStorageMeta:
     :class:`StorageMeta`.
     """
 
-    location: str
-    """The location the storage is mounted at."""
-
     def __init__(self, storage: str, location: str):
         self.storage = storage
         self._locations: List[str] = [location]
@@ -1588,17 +1746,17 @@ class ContainerStorageMeta:
         """An accessor for the list of locations for a mount."""
         return self._locations
 
-    def __getattr__(self, name: str):
-        # TODO(benhoyt): this should just be a property "location"
-        if name == "location":
-            if len(self._locations) == 1:
-                return self._locations[0]
-            else:
-                raise RuntimeError(
-                    "container has more than one mount point with the same backing storage. "
-                    "Request .locations to see a list"
-                )
-        else:
-            raise AttributeError(
-                f"{self.__class__.__name__} has no such attribute: {name}!"
-            )
+    @property
+    def location(self) -> str:
+        """The location the storage is mounted at.
+
+        Raises:
+            RuntimeError: if there is more than one mount point with the same
+                backing storage - use :attr:`locations` instead.
+        """
+        if len(self._locations) == 1:
+            return self._locations[0]
+        raise RuntimeError(
+            "container has more than one mount point with the same backing storage. "
+            "Request .locations to see a list"
+        )


Test patch:
diff --git a/test/test_charm.py b/test/test_charm.py
index b338ca5..ac14784 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -21,7 +21,7 @@ from pathlib import Path
 
 import ops
 import ops.charm
-from ops.model import _ModelBackend
+from ops.model import ModelError, _ModelBackend
 from ops.storage import SQLiteStorage
 
 from .test_helpers import fake_script, fake_script_calls
@@ -561,6 +561,8 @@ storage:
   other:
     type: filesystem
     location: /test/other
+    properties:
+      - transient
 containers:
   test1:
     mounts:
@@ -568,11 +570,44 @@ containers:
         location: /test/storagemount
       - storage: other
         location: /test/otherdata
+    resource: ubuntu-22.10
+  test2:
+    bases:
+      - name: ubuntu
+        channel: '23.10'
+        architectures:
+         - amd64
+      - name: ubuntu
+        channel: 23.04/stable/fips
+        architectures:
+         - arm
 """)
         self.assertIsInstance(meta.containers['test1'], ops.ContainerMeta)
         self.assertIsInstance(meta.containers['test1'].mounts["data"], ops.ContainerStorageMeta)
         self.assertEqual(meta.containers['test1'].mounts["data"].location, '/test/storagemount')
         self.assertEqual(meta.containers['test1'].mounts["other"].location, '/test/otherdata')
+        self.assertEqual(meta.storages['other'].properties, ['transient'])
+        self.assertEqual(meta.containers['test1'].resource, 'ubuntu-22.10')
+        assert meta.containers['test2'].bases is not None
+        self.assertEqual(len(meta.containers['test2'].bases), 2)
+        self.assertEqual(meta.containers['test2'].bases[0].os_name, 'ubuntu')
+        self.assertEqual(meta.containers['test2'].bases[0].channel, '23.10')
+        self.assertEqual(meta.containers['test2'].bases[0].architectures, ['amd64'])
+        self.assertEqual(meta.containers['test2'].bases[1].os_name, 'ubuntu')
+        self.assertEqual(meta.containers['test2'].bases[1].channel, '23.04/stable/fips')
+        self.assertEqual(meta.containers['test2'].bases[1].architectures, ['arm'])
+        # It's an error to specify both the 'resource' and the 'bases' fields.
+        with self.assertRaises(ModelError):
+            ops.CharmMeta.from_yaml("""
+name: invalid-charm
+containers:
+  test1:
+    bases:
+      - name: ubuntu
+        channel: '23.10'
+        architectures: [amd64]
+    resource: ubuntu-23.10
+""")
 
     def test_containers_storage_multiple_mounts(self):
         meta = ops.CharmMeta.from_yaml("""
@@ -828,3 +863,89 @@ containers:
             ['status-set', '--application=True', 'active', ''],
             ['status-set', '--application=True', 'unknown', ''],
         ])
+
+
+class TestCharmMeta(unittest.TestCase):
+    def test_links(self):
+        # Each type of link can be a single item.
+        meta = ops.CharmMeta.from_yaml("""
+name: my-charm
+website: https://example.com
+source: https://git.example.com
+issues: https://bugs.example.com
+docs: https://docs.example.com
+""")
+        self.assertEqual(meta.links.websites, ['https://example.com'])
+        self.assertEqual(meta.links.sources, ['https://git.example.com'])
+        self.assertEqual(meta.links.issues, ['https://bugs.example.com'])
+        self.assertEqual(meta.links.documentation, 'https://docs.example.com')
+        # Other than documentation, they can also all be lists of items.
+        meta = ops.CharmMeta.from_yaml("""
+name: my-charm
+website:
+ - https://example.com
+ - https://example.org
+source:
+ - https://git.example.com
+ - https://bzr.example.com
+issues:
+ - https://bugs.example.com
+ - https://features.example.com
+""")
+        self.assertEqual(meta.links.websites, ['https://example.com', 'https://example.org'])
+        self.assertEqual(
+            meta.links.sources, [
+                'https://git.example.com', 'https://bzr.example.com'])
+        self.assertEqual(
+            meta.links.issues, [
+                'https://bugs.example.com', 'https://features.example.com'])
+
+    def test_links_charmcraft_yaml(self):
+        meta = ops.CharmMeta.from_yaml("""
+links:
+  documentation: https://discourse.example.com/
+  issues:
+  - https://git.example.com/
+  source:
+  - https://git.example.com/issues/
+  website:
+  - https://example.com/
+  contact: Support Team <help@example.com>
+""")
+        self.assertEqual(meta.links.websites, ['https://example.com/'])
+        self.assertEqual(meta.links.sources, ['https://git.example.com/issues/'])
+        self.assertEqual(meta.links.issues, ['https://git.example.com/'])
+        self.assertEqual(meta.links.documentation, 'https://discourse.example.com/')
+        self.assertEqual(meta.maintainers, ['Support Team <help@example.com>'])
+
+    def test_assumes(self):
+        meta = ops.CharmMeta.from_yaml("""
+assumes:
+  - juju
+""")
+        self.assertEqual(meta.assumes.features, ['juju'])
+        meta = ops.CharmMeta.from_yaml("""
+assumes:
+  - juju > 3
+  - k8s-api
+""")
+        self.assertEqual(meta.assumes.features, ['juju > 3', 'k8s-api'])
+        meta = ops.CharmMeta.from_yaml("""
+assumes:
+  - k8s-api
+  - any-of:
+      - all-of:
+          - juju >= 2.9.44
+          - juju < 3
+      - all-of:
+          - juju >= 3.1.5
+          - juju < 4
+""")
+        self.assertEqual(meta.assumes.features, [
+            'k8s-api',
+            ops.JujuAssumes(
+                [ops.JujuAssumes(['juju >= 2.9.44', 'juju < 3']),
+                 ops.JujuAssumes(['juju >= 3.1.5', 'juju < 4'])],
+                ops.JujuAssumesCondition.ANY
+            ),
+        ])