Merge pull request #782 from canonical/validate-entrypoint-after-build

Validate the entrypoint after build.

charmcraft/commands/build.py

@@ -198,6 +198,9 @@ def build_charm(self, bases_config: BasesConfiguration) -> str:
         )
         lifecycle.run(Step.PRIME)
 
+        # validate after all processing
+        self._post_lifecycle_validation(lifecycle.prime_dir)
+
         # run linters and show the results
         linting_results = linters.analyze(self.config, lifecycle.prime_dir)
         self.show_linting_results(linting_results)
@@ -214,6 +217,14 @@ def build_charm(self, bases_config: BasesConfiguration) -> str:
         return zipname
 
     def _handle_deprecated_cli_arguments(self):
+        """Handle the mix of deprecated CLI args and config options.
+
+        This function also validate the result files from the mix when they are needed for
+        the lifecycle process itself (e.g. requirements.txt).
+        """
+        # XXX Facundo 2022-06-07: after these CLI args are removed, we could transform this
+        # function in a `_pre_lifecycle_validation` one.
+
         # verify if deprecated --requirement is used and update the plugin property
         if self._special_charm_part.get("charm-requirements"):
             if self.requirement_paths:
@@ -246,23 +257,43 @@ def _handle_deprecated_cli_arguments(self):
             entrypoint = self.entrypoint
             self.entrypoint = None
         else:
+            # XXX Facundo 2022-06-06: this default should come from the config itself, but
+            # that can only be done when the command line option for entrypoint is removed
+            # (otherwise we couldn't validate that the two options conflict).
             entrypoint = "src/charm.py"
 
         # store the entrypoint always relative to the project's path (no matter if the origin
         # was relative or absolute)
         rel_entrypoint = (self.charmdir / entrypoint).relative_to(self.charmdir)
         self._special_charm_part["charm-entrypoint"] = rel_entrypoint.as_posix()
 
-        # validate the entrypoint (no matter if it came from the config or it's the default)
+        # check that the entrypoint, if exists, is inside the project (this cannot be done
+        # after the lifecycle process, when the file would be just copied in)
         filepath = (self.charmdir / self._special_charm_part["charm-entrypoint"]).resolve()
-        if not filepath.exists():
-            raise CraftError("Charm entry point was not found: {!r}".format(str(filepath)))
-        if self.charmdir not in filepath.parents:
+        if filepath.exists() and self.charmdir not in filepath.parents:
             raise CraftError(
                 "Charm entry point must be inside the project: {!r}".format(str(filepath))
             )
-        if not os.access(filepath, os.X_OK):
-            raise CraftError("Charm entry point must be executable: {!r}".format(str(filepath)))
+
+    def _post_lifecycle_validation(self, basedir):
+        """Validate files after lifecycle steps.
+
+        Validations here happen *after all steps*, as in some cases required files (e.g. the
+        entrypoint itself) are generated during that process.
+
+        Current validation:
+
+        - the charm's entrypoint, no matter if it came from the config or it's the default.
+        """
+        if self._special_charm_part is not None:
+            # XXX Facundo 2022-06-06: we should move this to be a proper linter, but that can only
+            # be done cleanly when the command line option for entrypoint is removed (and the
+            # source of truth, including the default value, is the config)
+            fpath = (basedir / self._special_charm_part["charm-entrypoint"]).resolve()
+            if not fpath.exists():
+                raise CraftError("Charm entry point was not found: {!r}".format(str(fpath)))
+            if not os.access(fpath, os.X_OK):
+                raise CraftError("Charm entry point must be executable: {!r}".format(str(fpath)))
 
     def _set_prime_filter(self):
         """Add mandatory and optional charm files to the prime filter.

tests/commands/test_build.py

@@ -25,7 +25,7 @@
 from textwrap import dedent
 from typing import List
 from unittest import mock
-from unittest.mock import call, patch
+from unittest.mock import call, patch, MagicMock
 
 import pytest
 import yaml
@@ -621,7 +621,8 @@ def test_build_with_debug_no_error(
         debug=True,
     )
 
-    charms = builder.run(destructive_mode=True)
+    with patch.object(Builder, "_post_lifecycle_validation"):
+        charms = builder.run(destructive_mode=True)
 
     assert len(charms) == 1
     assert mock_launch_shell.mock_calls == []
@@ -671,7 +672,8 @@ def test_build_with_shell_after(
         shell_after=True,
     )
 
-    charms = builder.run(destructive_mode=True)
+    with patch.object(Builder, "_post_lifecycle_validation"):
+        charms = builder.run(destructive_mode=True)
 
     assert len(charms) == 1
     assert mock_launch_shell.mock_calls == [mock.call()]
@@ -1546,8 +1548,83 @@ def test_build_entrypoint_from_both(basic_project, monkeypatch):
     )
 
 
-def test_build_entrypoint_default_missing(basic_project, monkeypatch):
-    """The entrypoint is not specified and there is no file for its default."""
+def test_build_entrypoint_from_parts_outside(basic_project, monkeypatch):
+    """The specified entrypoint not points to a file outside the project."""
+    host_base = get_host_as_base()
+    outside = basic_project.parent
+    charmcraft_yaml_content = f"""\
+        type: charm
+        bases:
+          - build-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+            run-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+        parts:
+          charm:
+            charm-entrypoint: ../my_entrypoint.py
+        """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    # file exists and it's fine
+    entrypoint = outside / "my_entrypoint.py"
+    entrypoint.touch(mode=0o755)
+
+    builder = get_builder(config, entrypoint=None)
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == (
+        f"Charm entry point must be inside the project: {str(entrypoint.resolve())!r}"
+    )
+
+
+def test_build_postlifecycle_validation_is_properly_called(basic_project, monkeypatch):
+    """Check how the entrypoint validation helper is called."""
+    host_base = get_host_as_base()
+    charmcraft_file = basic_project / "charmcraft.yaml"
+    charmcraft_file.write_text(
+        dedent(
+            f"""\
+                type: charm
+                bases:
+                  - build-on:
+                      - name: {host_base.name!r}
+                        channel: {host_base.channel!r}
+                    run-on:
+                      - name: {host_base.name!r}
+                        channel: {host_base.channel!r}
+                parts:
+                  charm:
+                    charm-entrypoint: my_entrypoint.py
+                    charm-requirements: ["reqs.txt"]
+                """
+        )
+    )
+    config = load(basic_project)
+    builder = get_builder(config, entrypoint=None)
+
+    entrypoint = basic_project / "my_entrypoint.py"
+    entrypoint.touch(mode=0o700)
+
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+    with patch("charmcraft.parts.PartsLifecycle") as mock_lifecycle:
+        mock_lifecycle.return_value = mock_lifecycle_instance = MagicMock()
+        mock_lifecycle_instance.prime_dir = basic_project
+        mock_lifecycle_instance.run().return_value = None
+        with patch("charmcraft.linters.analyze"):
+            with patch.object(Builder, "show_linting_results"):
+                with patch.object(Builder, "_post_lifecycle_validation") as mock_validation:
+                    builder.run([0])
+
+    # check that the entrypoint validation was called correctly
+    mock_validation.assert_called_with(basic_project)
+
+
+def test_build_postlifecycle_validation_no_charm(basic_project):
+    """Post lifecycle validation when it's not a charm."""
     host_base = get_host_as_base()
     charmcraft_yaml_content = f"""\
        type: charm
@@ -1560,25 +1637,19 @@ def test_build_entrypoint_default_missing(basic_project, monkeypatch):
                channel: {host_base.channel!r}
        parts:
          charm:
-            charm-requirements: ["reqs.txt"]
+           source: .
+           plugin: reactive
+           build-snaps: [charm]
        """
     (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
     config = load(basic_project)
-    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
-
-    # remove the default entrypoint to make it fail
-    entrypoint = basic_project / "src" / "charm.py"
-    entrypoint.unlink()
 
     builder = get_builder(config, entrypoint=None, force=True)
-    with pytest.raises(CraftError) as cm:
-        builder.run([0])
-    assert str(cm.value) == f"Charm entry point was not found: {str(entrypoint)!r}"
+    builder._post_lifecycle_validation(basic_project)
 
 
-@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
-def test_build_entrypoint_default_nonexec(basic_project, monkeypatch):
-    """The entrypoint is not specified and the file for its default is not executable."""
+def test_build_postlifecycle_validation_entrypoint_missing(basic_project):
+    """The entrypoint is not specified and there is no file for its default."""
     host_base = get_host_as_base()
     charmcraft_yaml_content = f"""\
        type: charm
@@ -1592,108 +1663,47 @@ def test_build_entrypoint_default_nonexec(basic_project, monkeypatch):
        parts:
          charm:
             charm-requirements: ["reqs.txt"]
+            charm-entrypoint: missing.py
        """
     (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
     config = load(basic_project)
-    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
-
-    # remove exec properties from the file
-    entrypoint = basic_project / "src" / "charm.py"
-    entrypoint.chmod(0o666)
 
     builder = get_builder(config, entrypoint=None, force=True)
+    builder._handle_deprecated_cli_arguments()  # load from config
     with pytest.raises(CraftError) as cm:
-        builder.run([0])
-    assert str(cm.value) == f"Charm entry point must be executable: {str(entrypoint)!r}"
-
-
-def test_build_entrypoint_from_parts_missing(basic_project, monkeypatch):
-    """The specified entrypoint does not point to an existing file."""
-    host_base = get_host_as_base()
-    charmcraft_yaml_content = f"""\
-        type: charm
-        bases:
-          - build-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-            run-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-        parts:
-          charm:
-            charm-entrypoint: my_entrypoint.py
-        """
-    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
-    config = load(basic_project)
-    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
-
-    builder = get_builder(config, entrypoint=None)
-    entrypoint = basic_project / "my_entrypoint.py"
-    with pytest.raises(CraftError) as cm:
-        builder.run([0])
+        builder._post_lifecycle_validation(basic_project)
+    entrypoint = basic_project / "missing.py"
     assert str(cm.value) == f"Charm entry point was not found: {str(entrypoint)!r}"
 
 
-def test_build_entrypoint_from_parts_outside(basic_project, monkeypatch):
-    """The specified entrypoint not points to a file outside the project."""
-    host_base = get_host_as_base()
-    outside = basic_project.parent
-    charmcraft_yaml_content = f"""\
-        type: charm
-        bases:
-          - build-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-            run-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-        parts:
-          charm:
-            charm-entrypoint: ../my_entrypoint.py
-        """
-    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
-    config = load(basic_project)
-    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
-
-    # file exists and it's fine
-    entrypoint = outside / "my_entrypoint.py"
-    entrypoint.touch(mode=0o755)
-
-    builder = get_builder(config, entrypoint=None)
-    with pytest.raises(CraftError) as cm:
-        builder.run([0])
-    assert str(cm.value) == (
-        f"Charm entry point must be inside the project: {str(entrypoint.resolve())!r}"
-    )
-
-
 @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
-def test_build_entrypoint_from_parts_nonexec(basic_project, monkeypatch):
-    """The specified entrypoint not points to a non-executable file."""
+def test_build_postlifecycle_validation_entrypoint_nonexec(basic_project):
+    """The entrypoint is not specified and the file for its default is not executable."""
     host_base = get_host_as_base()
     charmcraft_yaml_content = f"""\
-        type: charm
-        bases:
-          - build-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-            run-on:
-              - name: {host_base.name!r}
-                channel: {host_base.channel!r}
-        parts:
-          charm:
-            charm-entrypoint: entrypoint.py
-        """
+       type: charm
+       bases:
+         - build-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+           run-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+       parts:
+         charm:
+            charm-requirements: ["reqs.txt"]
+       """
     (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
     config = load(basic_project)
-    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
 
-    entrypoint = basic_project / "entrypoint.py"
-    entrypoint.touch(mode=0o666)
+    # remove exec properties from the file
+    entrypoint = basic_project / "src" / "charm.py"
+    entrypoint.chmod(0o666)
 
-    builder = get_builder(config, entrypoint=None)
+    builder = get_builder(config, entrypoint=None, force=True)
+    builder._handle_deprecated_cli_arguments()  # load from config
     with pytest.raises(CraftError) as cm:
-        builder.run([0])
+        builder._post_lifecycle_validation(basic_project)
     assert str(cm.value) == f"Charm entry point must be executable: {str(entrypoint)!r}"