models: do not enforce pydantic models on legacy files (#1204)

Only warn for invalid content for backwards compatibility

Fixes #1208

charmcraft/metafiles/actions.py

@@ -16,15 +16,18 @@
 
 """Charmcraft project handle actions.yaml file."""
 
+import contextlib
 import pathlib
 import logging
 import shutil
 from typing import Optional, TYPE_CHECKING
 
+import pydantic
 import yaml
 
 from craft_cli import emit, CraftError
 from charmcraft.const import JUJU_ACTIONS_FILENAME
+from charmcraft.format import format_pydantic_errors
 from charmcraft.models.actions import JujuActions
 from charmcraft.metafiles import read_yaml
 
@@ -34,7 +37,7 @@
 logger = logging.getLogger(__name__)
 
 
-def parse_actions_yaml(charm_dir: pathlib.Path) -> Optional[JujuActions]:
+def parse_actions_yaml(charm_dir: pathlib.Path, allow_broken=False) -> Optional[JujuActions]:
     """Parse project's actions.yaml.
 
     :param charm_dir: Directory to read actions.yaml from.
@@ -51,7 +54,17 @@ def parse_actions_yaml(charm_dir: pathlib.Path) -> Optional[JujuActions]:
         raise CraftError(f"Cannot read the {JUJU_ACTIONS_FILENAME} file: {exc!r}") from exc
 
     emit.debug(f"Validating {JUJU_ACTIONS_FILENAME}")
-    return JujuActions.parse_obj({"actions": actions, "legacy": True})
+    try:
+        return JujuActions.parse_obj({"actions": actions})
+    except pydantic.ValidationError as error:
+        if allow_broken:
+            emit.progress(
+                format_pydantic_errors(error.errors(), file_name=JUJU_ACTIONS_FILENAME),
+                permanent=True,
+            )
+            emit.debug(f"Ignoring {JUJU_ACTIONS_FILENAME}")
+            return None
+        raise
 
 
 def create_actions_yaml(
@@ -65,23 +78,24 @@ def create_actions_yaml(
 
     :returns: Path to created actions.yaml.
     """
-    if charmcraft_config.actions is None or charmcraft_config.actions.actions is None:
-        return None
-
-    file_path = basedir / JUJU_ACTIONS_FILENAME
-
-    if charmcraft_config.actions.legacy:
-        try:
-            shutil.copyfile(charmcraft_config.project.dirpath / JUJU_ACTIONS_FILENAME, file_path)
-        except shutil.SameFileError:
-            pass
+    original_file_path = charmcraft_config.project.dirpath / JUJU_ACTIONS_FILENAME
+    target_file_path = basedir / JUJU_ACTIONS_FILENAME
+
+    # Copy actions.yaml if it exists, otherwise create it from CharmcraftConfig.
+    if original_file_path.exists():
+        # In the build / test process, the original file may be the same as the target file.
+        with contextlib.suppress(shutil.SameFileError):
+            shutil.copyfile(original_file_path, target_file_path)
     else:
-        file_path.write_text(
-            yaml.dump(
-                charmcraft_config.actions.dict(
-                    include={"actions"}, exclude_none=True, by_alias=True
-                )["actions"]
+        if charmcraft_config.actions:
+            target_file_path.write_text(
+                yaml.dump(
+                    charmcraft_config.actions.dict(
+                        include={"actions"}, exclude_none=True, by_alias=True
+                    )["actions"]
+                )
             )
-        )
+        else:
+            return None
 
-    return file_path
+    return target_file_path

charmcraft/metafiles/config.py

@@ -16,15 +16,18 @@
 
 """Charmcraft project handle config.yaml file."""
 
+import contextlib
 import pathlib
 import logging
 import shutil
 from typing import Optional, TYPE_CHECKING
 
+import pydantic
 import yaml
 
 from craft_cli import emit, CraftError
 from charmcraft.const import JUJU_CONFIG_FILENAME
+from charmcraft.format import format_pydantic_errors
 from charmcraft.models.config import JujuConfig
 from charmcraft.metafiles import read_yaml
 
@@ -34,7 +37,7 @@
 logger = logging.getLogger(__name__)
 
 
-def parse_config_yaml(charm_dir: pathlib.Path) -> Optional[JujuConfig]:
+def parse_config_yaml(charm_dir: pathlib.Path, allow_broken=False) -> Optional[JujuConfig]:
     """Parse project's config.yaml.
 
     :param charm_dir: Directory to read config.yaml from.
@@ -51,7 +54,17 @@ def parse_config_yaml(charm_dir: pathlib.Path) -> Optional[JujuConfig]:
         raise CraftError(f"Cannot read the {JUJU_CONFIG_FILENAME} file: {exc!r}") from exc
 
     emit.debug(f"Validating {JUJU_CONFIG_FILENAME}")
-    return JujuConfig.parse_obj({"legacy": True, **config})
+    try:
+        return JujuConfig.parse_obj(config)
+    except pydantic.ValidationError as error:
+        if allow_broken:
+            emit.progress(
+                format_pydantic_errors(error.errors(), file_name=JUJU_CONFIG_FILENAME),
+                permanent=True,
+            )
+            emit.debug(f"Ignoring {JUJU_CONFIG_FILENAME}")
+            return None
+        raise
 
 
 def create_config_yaml(
@@ -65,23 +78,24 @@ def create_config_yaml(
 
     :returns: Path to created config.yaml.
     """
-    if charmcraft_config.config is None or charmcraft_config.config.options is None:
-        return None
-
-    file_path = basedir / JUJU_CONFIG_FILENAME
-
-    if charmcraft_config.config.legacy:
-        try:
-            shutil.copyfile(charmcraft_config.project.dirpath / JUJU_CONFIG_FILENAME, file_path)
-        except shutil.SameFileError:
-            pass
+    original_file_path = charmcraft_config.project.dirpath / JUJU_CONFIG_FILENAME
+    target_file_path = basedir / JUJU_CONFIG_FILENAME
+
+    # Copy config.yaml if it exists, otherwise create it from CharmcraftConfig.
+    if original_file_path.exists():
+        # In the build / test process, the original file may be the same as the target file.
+        with contextlib.suppress(shutil.SameFileError):
+            shutil.copyfile(original_file_path, target_file_path)
     else:
-        file_path.write_text(
-            yaml.dump(
-                charmcraft_config.config.dict(
-                    include={"options"}, exclude_none=True, by_alias=True
+        if charmcraft_config.config:
+            target_file_path.write_text(
+                yaml.dump(
+                    charmcraft_config.config.dict(
+                        include={"options"}, exclude_none=True, by_alias=True
+                    )
                 )
             )
-        )
+        else:
+            return None
 
-    return file_path
+    return target_file_path

charmcraft/metafiles/metadata.py

@@ -16,15 +16,18 @@
 
 """Handlers for metadata.yaml file."""
 
+import contextlib
 import shutil
 import pathlib
 import logging
 from typing import Any, Dict, TYPE_CHECKING
 
+import pydantic
 import yaml
 from craft_cli import emit, CraftError
 
 from charmcraft.const import METADATA_FILENAME, CHARM_METADATA_KEYS, CHARM_METADATA_KEYS_ALIAS
+from charmcraft.format import format_pydantic_errors
 from charmcraft.models.metadata import CharmMetadataLegacy, BundleMetadataLegacy
 
 if TYPE_CHECKING:
@@ -44,7 +47,9 @@ def read_metadata_yaml(charm_dir: pathlib.Path) -> Dict[str, Any]:
         return yaml.safe_load(fh)
 
 
-def parse_charm_metadata_yaml(charm_dir: pathlib.Path) -> CharmMetadataLegacy:
+def parse_charm_metadata_yaml(
+    charm_dir: pathlib.Path, allow_basic: bool = False
+) -> CharmMetadataLegacy:
     """Parse project's legacy metadata.yaml that used for charms.
 
     :returns: a CharmMetadataLegacy object.
@@ -59,7 +64,19 @@ def parse_charm_metadata_yaml(charm_dir: pathlib.Path) -> CharmMetadataLegacy:
         raise CraftError(f"The {charm_dir / METADATA_FILENAME} file is not valid YAML.")
 
     emit.debug("Validating metadata keys")
-    return CharmMetadataLegacy.unmarshal(metadata)
+    try:
+        return CharmMetadataLegacy.unmarshal(metadata)
+    except pydantic.ValidationError as error:
+        if allow_basic:
+            emit.progress(
+                format_pydantic_errors(error.errors(), file_name=METADATA_FILENAME), permanent=True
+            )
+            emit.debug("Falling back to basic metadata.yaml")
+            metadata_basic = {
+                k: v for k, v in metadata.items() if k in ("name", "summary", "description")
+            }
+            return CharmMetadataLegacy.unmarshal(metadata_basic)
+        raise
 
 
 def parse_bundle_metadata_yaml(charm_dir: pathlib.Path) -> BundleMetadataLegacy:
@@ -93,13 +110,14 @@ def create_metadata_yaml(
 
     :returns: Path to created metadata.yaml.
     """
-    file_path = charm_dir / METADATA_FILENAME
-    # metadata.yaml should be copied if it exists in the project
-    if charmcraft_config.metadata_legacy:
-        try:
-            shutil.copyfile(charmcraft_config.project.dirpath / METADATA_FILENAME, file_path)
-        except shutil.SameFileError:
-            pass
+    original_file_path = charmcraft_config.project.dirpath / METADATA_FILENAME
+    target_file_path = charm_dir / METADATA_FILENAME
+
+    # Copy metadata.yaml if it exists, otherwise create it from CharmcraftConfig.
+    if original_file_path.exists():
+        # In the build / test process, the original file may be the same as the target file.
+        with contextlib.suppress(shutil.SameFileError):
+            shutil.copyfile(original_file_path, target_file_path)
     else:
         # metadata.yaml not exists, create it from config
         metadata = charmcraft_config.dict(
@@ -124,6 +142,6 @@ def create_metadata_yaml(
             if (website := links.pop("website", None)) is not None:
                 metadata["website"] = website
 
-        file_path.write_text(yaml.dump(metadata))
+        target_file_path.write_text(yaml.dump(metadata))
 
-    return file_path
+    return target_file_path

charmcraft/models/actions.py

@@ -34,7 +34,6 @@ class JujuActions(ModelConfigDefaults):
 
     _action_name_regex = re.compile(r"^[a-zA-Z_][a-zA-Z0-9-_]*$")
     actions: Optional[Dict[str, Dict]]
-    legacy: bool = False
 
     @pydantic.validator("actions")
     def validate_actions(cls, actions, values):

charmcraft/models/charmcraft.py

@@ -125,7 +125,7 @@ class CharmcraftConfig(
     bases: Optional[List[BasesConfiguration]]
     analysis: AnalysisConfig = AnalysisConfig()
     actions: Optional[JujuActions]
-    assumes: Optional[List[Union[str, Dict[str, List[str]]]]]
+    assumes: Optional[List[Union[str, Dict[str, Union[List, Dict]]]]]
     containers: Optional[Dict[str, Any]]
     devices: Optional[Dict[str, Any]]
     title: Optional[pydantic.StrictStr]
@@ -223,7 +223,7 @@ def validate_actions(cls, actions, values):
         And individual "actions.yaml" should not exists when actions
         is defined in charmcraft.yaml.
         """
-        actions_yaml = parse_actions_yaml(values["project"].dirpath)
+        actions_yaml = parse_actions_yaml(values["project"].dirpath, allow_broken=True)
         if actions is None:
             return actions_yaml
         else:
@@ -233,7 +233,7 @@ def validate_actions(cls, actions, values):
                     "defined in 'charmcraft.yaml'"
                 )
 
-            return JujuActions.parse_obj({"actions": actions, "legacy": False})
+            return JujuActions.parse_obj({"actions": actions})
 
     @pydantic.validator("config", pre=True, always=True)
     def validate_config(cls, config, values):
@@ -243,7 +243,7 @@ def validate_config(cls, config, values):
         And individual "actions.yaml" should not exists when actions
         is defined in charmcraft.yaml.
         """
-        config_yaml = parse_config_yaml(values["project"].dirpath)
+        config_yaml = parse_config_yaml(values["project"].dirpath, allow_broken=True)
         if config is None:
             return config_yaml
         else:
@@ -253,7 +253,7 @@ def validate_config(cls, config, values):
                     "defined in 'charmcraft.yaml'"
                 )
 
-            return JujuConfig.parse_obj({"legacy": False, **config})
+            return JujuConfig.parse_obj(config)
 
     @classmethod
     def expand_short_form_bases(cls, bases: List[Dict[str, Any]]) -> None:
@@ -267,7 +267,7 @@ def expand_short_form_bases(cls, bases: List[Dict[str, Any]]) -> None:
 
             try:
                 converted_base = Base(**base)
-            except pydantic.error_wrappers.ValidationError as error:
+            except pydantic.ValidationError as error:
                 # Rewrite location to assist user.
                 pydantic_errors = error.errors()
                 for pydantic_error in pydantic_errors:
@@ -308,7 +308,7 @@ def unmarshal(cls, obj: Dict[str, Any], project: Project):
                         )
 
                 if obj.get("type") == "charm":
-                    metadata_legacy = parse_charm_metadata_yaml(project.dirpath)
+                    metadata_legacy = parse_charm_metadata_yaml(project.dirpath, allow_basic=True)
 
                     # need to copy 3 fields from metadata_legacy to charmcraft config
                     return cls.parse_obj(
@@ -340,7 +340,7 @@ def unmarshal(cls, obj: Dict[str, Any], project: Project):
                     pass
 
             return cls.parse_obj({"project": project, **obj})
-        except pydantic.error_wrappers.ValidationError as error:
+        except pydantic.ValidationError as error:
             raise CraftError(format_pydantic_errors(error.errors()))
 
     @classmethod

charmcraft/models/config.py

@@ -30,7 +30,6 @@ class JujuConfig(ModelConfigDefaults):
     """
 
     options: Optional[Dict[str, Dict]]
-    legacy: bool = False
 
     @pydantic.validator("options", pre=True)
     def validate_actions(cls, options, values):

charmcraft/models/metadata.py

@@ -21,7 +21,6 @@
 import pydantic
 
 from craft_cli import CraftError
-from charmcraft.format import format_pydantic_errors
 from charmcraft.const import METADATA_FILENAME
 
 
@@ -43,7 +42,7 @@ class CharmMetadataLegacy(
     name: pydantic.StrictStr
     summary: pydantic.StrictStr
     description: pydantic.StrictStr
-    assumes: Optional[List[Union[str, Dict[str, List[str]]]]]
+    assumes: Optional[List[Union[str, Dict[str, Union[List, Dict]]]]]
     containers: Optional[Dict[str, Any]]
     devices: Optional[Dict[str, Any]]
     display_name: Optional[pydantic.StrictStr]
@@ -79,10 +78,7 @@ def unmarshal(cls, obj: Dict[str, Any]):
             obj["maintainers"] = [obj["maintainer"]]
             del obj["maintainer"]
 
-        try:
-            return cls.parse_obj(obj)
-        except pydantic.error_wrappers.ValidationError as error:
-            raise CraftError(format_pydantic_errors(error.errors(), file_name=METADATA_FILENAME))
+        return cls.parse_obj(obj)
 
 
 class BundleMetadataLegacy(
@@ -110,7 +106,4 @@ def unmarshal(cls, obj: Dict[str, Any]):
 
         :raises CraftError: On failure to unmarshal object.
         """
-        try:
-            return cls.parse_obj(obj)
-        except pydantic.error_wrappers.ValidationError as error:
-            raise CraftError(format_pydantic_errors(error.errors(), file_name=METADATA_FILENAME))
+        return cls.parse_obj(obj)