Merge pull request #372 from opensafely-core/validate-version

Validate the form of the image versions.
opensafely-core · Jan 21, 2025 · c9fbb17 · c9fbb17
2 parents 4bda291 + 086ffb6
commit c9fbb17
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 6 deletions.
diff --git a/pipeline/models.py b/pipeline/models.py
@@ -214,6 +214,10 @@ def build(
 
         return action
 
+    # Valid image versions. `dev` is for local testing
+    # Note: at some point, we probably want to disallow latest.
+    VERSION_REGEX = re.compile(r"^((v[\d.]+)|dev|latest)$")
+
     @classmethod
     def parse_run_string(cls, action_id: str, run: str) -> Command:
         if run == "":
@@ -224,9 +228,11 @@ def parse_run_string(cls, action_id: str, run: str) -> Command:
         parts = shlex.split(run)
 
         name, _, version = parts[0].partition(":")
-        if not version:
+
+        vmatch = cls.VERSION_REGEX.match(version)
+        if not vmatch:
             raise ValidationError(
-                f"{name} must have a version specified (e.g. {name}:0.5.2)",
+                f"Action command {name} must have a version specified in the form :vN (e.g. {name}:v2)",
             )
 
         return Command(raw=run)

diff --git a/tests/test_models.py b/tests/test_models.py
@@ -21,12 +21,23 @@ def test_success():
     Pipeline.build(**data)
 
 
-def test_action_has_a_version():
+@pytest.mark.parametrize(
+    "action",
+    [
+        "test",
+        "test:",
+        "test:v",
+        "test:other",
+        "test:vnotdigits",
+        "test:v1x1",
+    ],
+)
+def test_action_handles_invalid_version(action):
     data = {
         "version": 1,
         "actions": {
             "generate_cohort": {
-                "run": "test foo",
+                "run": action,
                 "outputs": {
                     "highly_sensitive": {"cohort": "output/input.csv"},
                 },
@@ -39,6 +50,36 @@ def test_action_has_a_version():
         Pipeline.build(**data)
 
 
+@pytest.mark.parametrize(
+    "action",
+    [
+        "test:v1",
+        "test:v2",
+        "test:v1.2",
+        "test:v1.2.3",
+        "test:dev",
+        "test:latest",
+    ],
+)
+def test_action_handles_valid_version(action):
+    data = {
+        "version": 1,
+        "actions": {
+            "generate_cohort": {
+                "run": action,
+                "outputs": {
+                    "highly_sensitive": {"cohort": "output/input.csv"},
+                },
+            }
+        },
+    }
+
+    run = Pipeline.build(**data).actions["generate_cohort"].run
+    n, _, v = action.partition(":")
+    assert run.name == n
+    assert run.version == v
+
+
 def test_action_cohortextractor_multiple_outputs_with_output_flag():
     data = {
         "version": 1,
@@ -337,13 +378,13 @@ def test_pipeline_with_duplicated_action_run_commands():
         "version": 1,
         "actions": {
             "action1": {
-                "run": "test:lastest",
+                "run": "test:latest",
                 "outputs": {
                     "moderately_sensitive": {"cohort": "output.csv"},
                 },
             },
             "action2": {
-                "run": "test:lastest",
+                "run": "test:latest",
                 "outputs": {
                     "moderately_sensitive": {"cohort": "output.csv"},
                 },