Fix argument list too long and Porter installation document handling for parameter sets

CHANGELOG.md

@@ -5,6 +5,9 @@ ENHANCEMENTS:
 * Specify default_outbound_access_enabled = false setting for all subnets ([#4757](https://github.com/microsoft/AzureTRE/pull/4757))
 * Pin all GitHub Actions workflow steps to full commit SHAs to prevent supply chain attacks plus update to latest releases ([#4886](https://github.com/microsoft/AzureTRE/pull/4886))
 
+BUG FIXES:
+* Fix `OSError: [Errno 7] Argument list too long` when deploying many workspaces by switching to `porter installation apply` with a temporary parameter set file; the first run after upgrade of each existing installation also clears legacy installation-resource parameter overrides ([#4903](https://github.com/microsoft/AzureTRE/issues/4903))
+
 ## (0.28.0) (March 2, 2026)
 **BREAKING CHANGES**
 * Sonatype Nexus shared service now requires explicit EULA acceptance (`accept_nexus_eula: true`) when deploying. This ensures compliance with Sonatype Nexus Community Edition licensing. ([#4842](https://github.com/microsoft/AzureTRE/issues/4842))

resource_processor/_version.py

@@ -1 +1 @@
-__version__ = "0.13.3"
+__version__ = "0.13.5"

resource_processor/helpers/commands.py

@@ -2,6 +2,8 @@
 import json
 import base64
 import logging
+import tempfile
+import uuid
 from urllib.parse import urlparse
 
 from shared.logging import logger, shell_output_logger
@@ -78,7 +80,7 @@ def azure_acr_login_command(config):
 
 async def build_porter_command(config, msg_body, custom_action=False):
     porter_parameter_keys = await get_porter_parameter_keys(config, msg_body)
-    porter_parameters = []
+    param_set_entries = []
 
     if porter_parameter_keys is None:
         logger.warning("Unknown porter parameters - explain probably failed.")
@@ -117,29 +119,77 @@ async def build_porter_command(config, msg_body, custom_action=False):
                     val_base64_bytes = base64.b64encode(val_bytes)
                     parameter_value = val_base64_bytes.decode("ascii")
 
-                porter_parameters.extend(["--param", f"{parameter_name}={parameter_value}"])
+                param_set_entries.append({
+                    "name": parameter_name,
+                    "source": {"value": str(parameter_value)}
+                })
 
     installation_id = msg_body['id']
+    param_set_name = f"tre-params-{installation_id}-{uuid.uuid4().hex[:8]}"
+
+    param_set_file = None
+    if param_set_entries:
+        param_set = {
+            "schemaType": "ParameterSet",
+            "schemaVersion": "1.0.1",
+            "name": param_set_name,
+            "namespace": "",
+            "parameters": param_set_entries
+        }
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.json', delete=False) as f:
+            json.dump(param_set, f)
+            param_set_file = f.name
+
+    installation_file = None
+    if not custom_action and msg_body['action'] in ("install", "upgrade"):
+        installation = {
+            "schemaType": "Installation",
+            "schemaVersion": "1.0.2",
+            "name": installation_id,
+            "bundle": {
+                "repository": f"{config['registry_server']}/{msg_body['name']}",
+                "version": msg_body['version']
+            },
+            "parameters": {},
+            "parameterSets": [param_set_name] if param_set_entries else [],
+            "credentialSets": ["arm_auth", "aad_auth"]
+        }
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.json', delete=False) as f:
+            json.dump(installation, f)
+            installation_file = f.name
 
-    command = ["porter"]
-    if custom_action:
-        command.extend(["invoke", "--action"])
-
-    command.append(msg_body['action'])
-    command.append(installation_id)
-    command.extend([
-        "--reference",
-        f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}"
-    ])
-    command.extend(porter_parameters)
-    command.append("--force")
-    command.extend(["--credential-set", "arm_auth"])
-    command.extend(["--credential-set", "aad_auth"])
-
-    if msg_body['action'] == 'upgrade':
-        command.append("--force-upgrade")
+    commands = []
+    if param_set_file:
+        commands.append(["porter", "parameters", "apply", param_set_file])
 
-    return [command]
+    if custom_action:
+        command = ["porter", "invoke", "--action", msg_body['action'], installation_id]
+        command.extend([
+            "--reference",
+            f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}"
+        ])
+        if param_set_file:
+            command.extend(["--parameter-set", param_set_name])
+        command.append("--force")
+        command.extend(["--credential-set", "arm_auth"])
+        command.extend(["--credential-set", "aad_auth"])
+        commands.append(command)
+    elif installation_file:
+        commands.append(["porter", "installation", "apply", installation_file, "--force"])
+    else:
+        command = ["porter", msg_body['action'], installation_id]
+        command.extend([
+            "--reference",
+            f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}"
+        ])
+        if param_set_file:
+            command.extend(["--parameter-set", param_set_name])
+        command.append("--force")
+        command.extend(["--credential-set", "arm_auth"])
+        command.extend(["--credential-set", "aad_auth"])
+        commands.append(command)
+
+    return (commands, param_set_file, param_set_name, installation_file)
 
 
 async def build_porter_command_for_outputs(msg_body):

resource_processor/tests_rp/test_commands.py

@@ -1,5 +1,6 @@
 import json
 import asyncio
+import os
 import pytest
 from unittest.mock import patch, AsyncMock
 from helpers.commands import azure_login_command, apply_porter_credentials_sets_command, azure_acr_login_command, build_porter_command, build_porter_command_for_outputs, get_porter_parameter_keys, run_command_helper, get_special_porter_param_value
@@ -58,17 +59,44 @@ async def test_build_porter_command(mock_get_porter_parameter_keys):
     msg_body = {"id": "guid", "action": "install", "name": "mybundle", "version": "1.0.0", "parameters": {"param1": "value1"}}
     mock_get_porter_parameter_keys.return_value = ["param1"]
 
-    expected_command = [[
-        "porter", "install", "guid",
-        "--reference", "myregistry.azurecr.io/mybundle:v1.0.0",
-        "--param", "param1=value1",
-        "--force",
-        "--credential-set", "arm_auth",
-        "--credential-set", "aad_auth"
-    ]]
-
-    command = await build_porter_command(config, msg_body)
-    assert command == expected_command
+    commands, param_set_file, param_set_name, installation_file = await build_porter_command(config, msg_body)
+    try:
+        assert param_set_file is not None
+        assert param_set_name.startswith("tre-params-guid-")
+        assert len(param_set_name) == len("tre-params-guid-") + 8
+        assert os.path.exists(param_set_file)
+        assert installation_file is not None
+        assert os.path.exists(installation_file)
+
+        # First command applies the parameter set to Porter's store
+        assert commands[0] == ["porter", "parameters", "apply", param_set_file]
+
+        # Second command is porter installation apply using the installation file
+        assert commands[1] == ["porter", "installation", "apply", installation_file, "--force"]
+
+        with open(param_set_file) as f:
+            param_set = json.load(f)
+
+        assert param_set["schemaType"] == "ParameterSet"
+        assert param_set["name"] == param_set_name
+        assert len(param_set["parameters"]) == 1
+        assert param_set["parameters"][0] == {"name": "param1", "source": {"value": "value1"}}
+
+        with open(installation_file) as f:
+            installation = json.load(f)
+
+        assert installation["schemaType"] == "Installation"
+        assert installation["name"] == "guid"
+        assert installation["parameters"] == {}
+        assert installation["parameterSets"] == [param_set_name]
+        assert installation["credentialSets"] == ["arm_auth", "aad_auth"]
+        assert installation["bundle"]["repository"] == "myregistry.azurecr.io/mybundle"
+        assert installation["bundle"]["version"] == "1.0.0"
+    finally:
+        if param_set_file and os.path.exists(param_set_file):
+            os.unlink(param_set_file)
+        if installation_file and os.path.exists(installation_file):
+            os.unlink(installation_file)
 
 
 @pytest.mark.asyncio
@@ -78,18 +106,30 @@ async def test_build_porter_command_for_upgrade(mock_get_porter_parameter_keys):
     msg_body = {"id": "guid", "action": "upgrade", "name": "mybundle", "version": "1.0.0", "parameters": {"param1": "value1"}}
     mock_get_porter_parameter_keys.return_value = ["param1"]
 
-    expected_command = [[
-        "porter", "upgrade", "guid",
-        "--reference", "myregistry.azurecr.io/mybundle:v1.0.0",
-        "--param", "param1=value1",
-        "--force",
-        "--credential-set", "arm_auth",
-        "--credential-set", "aad_auth",
-        "--force-upgrade"
-    ]]
+    commands, param_set_file, param_set_name, installation_file = await build_porter_command(config, msg_body)
+    try:
+        assert param_set_file is not None
+        assert param_set_name.startswith("tre-params-guid-")
+        assert os.path.exists(param_set_file)
+        assert installation_file is not None
+        assert os.path.exists(installation_file)
 
-    command = await build_porter_command(config, msg_body)
-    assert command == expected_command
+        # First command applies the parameter set to Porter's store
+        assert commands[0] == ["porter", "parameters", "apply", param_set_file]
+
+        # Second command is porter installation apply (not porter upgrade)
+        assert commands[1] == ["porter", "installation", "apply", installation_file, "--force"]
+
+        with open(installation_file) as f:
+            installation = json.load(f)
+
+        assert installation["parameters"] == {}
+        assert installation["parameterSets"] == [param_set_name]
+    finally:
+        if param_set_file and os.path.exists(param_set_file):
+            os.unlink(param_set_file)
+        if installation_file and os.path.exists(installation_file):
+            os.unlink(installation_file)
 
 
 @pytest.mark.asyncio
@@ -106,6 +146,34 @@ async def test_build_porter_command_for_outputs():
     assert command == expected_command
 
 
+@pytest.mark.asyncio
+async def test_build_porter_command_no_parameters(mock_get_porter_parameter_keys):
+    """Test build_porter_command with no parameters: no param set file, but installation file with empty parameterSets."""
+    config = {"registry_server": "myregistry.azurecr.io"}
+    msg_body = {"id": "guid", "action": "install", "name": "mybundle", "version": "1.0.0", "parameters": {}}
+    mock_get_porter_parameter_keys.return_value = []
+
+    commands, param_set_file, param_set_name, installation_file = await build_porter_command(config, msg_body)
+
+    try:
+        assert param_set_file is None
+        assert param_set_name.startswith("tre-params-guid-")
+        assert installation_file is not None
+        assert os.path.exists(installation_file)
+
+        assert commands == [["porter", "installation", "apply", installation_file, "--force"]]
+
+        with open(installation_file) as f:
+            installation = json.load(f)
+
+        assert installation["parameters"] == {}
+        assert installation["parameterSets"] == []
+        assert installation["credentialSets"] == ["arm_auth", "aad_auth"]
+    finally:
+        if installation_file and os.path.exists(installation_file):
+            os.unlink(installation_file)
+
+
 @pytest.mark.asyncio
 async def test_build_porter_command_with_complex_parameters(mock_get_porter_parameter_keys):
     """Test build_porter_command function with complex parameter types (dict, list)."""
@@ -127,33 +195,78 @@ async def test_build_porter_command_with_complex_parameters(mock_get_porter_para
 
     mock_get_porter_parameter_keys.return_value = ["dict_param", "list_param", "string_param"]
 
-    command = await build_porter_command(config, msg_body)
+    commands, param_set_file, param_set_name, installation_file = await build_porter_command(config, msg_body)
+
+    try:
+        # First command is the apply command
+        assert commands[0] == ["porter", "parameters", "apply", param_set_file]
+
+        # Second command is porter installation apply
+        assert commands[1] == ["porter", "installation", "apply", installation_file, "--force"]
+
+        assert param_set_name.startswith("tre-params-guid-")
+
+        # Verify the param set file contains the correct parameters
+        assert param_set_file is not None
+        with open(param_set_file) as f:
+            param_set = json.load(f)
+
+        params_by_name = {p["name"]: p["source"]["value"] for p in param_set["parameters"]}
 
-    # Verify the command contains properly encoded complex parameters
-    command_args = command[0]
+        assert "dict_param" in params_by_name
+        assert "list_param" in params_by_name
+        assert "string_param" in params_by_name
+        assert params_by_name["string_param"] == "simple_value"
 
-    # Find the indices of parameters
-    param_indices = [i for i, arg in enumerate(command_args) if arg == "--param"]
-    param_values = [command_args[i + 1] for i in param_indices]
+        # Verify the dict and list are base64 encoded
+        import base64
 
-    # Check for all parameters
-    dict_param = next((p for p in param_values if p.startswith("dict_param=")), None)
-    list_param = next((p for p in param_values if p.startswith("list_param=")), None)
-    string_param = next((p for p in param_values if p.startswith("string_param=")), None)
+        dict_encoded = base64.b64encode(json.dumps(dict_value).encode("ascii")).decode("ascii")
+        list_encoded = base64.b64encode(json.dumps(list_value).encode("ascii")).decode("ascii")
 
-    assert dict_param is not None
-    assert list_param is not None
-    assert string_param is not None
-    assert string_param == "string_param=simple_value"
+        assert params_by_name["dict_param"] == dict_encoded
+        assert params_by_name["list_param"] == list_encoded
+
+        # Verify the installation file references the parameter set
+        assert installation_file is not None
+        with open(installation_file) as f:
+            installation = json.load(f)
+
+        assert installation["parameters"] == {}
+        assert installation["parameterSets"] == [param_set_name]
+    finally:
+        if param_set_file and os.path.exists(param_set_file):
+            os.unlink(param_set_file)
+        if installation_file and os.path.exists(installation_file):
+            os.unlink(installation_file)
+
+
+@pytest.mark.asyncio
+async def test_build_porter_command_custom_action(mock_get_porter_parameter_keys):
+    """Test that custom actions use porter invoke --action (regression guard)."""
+    config = {"registry_server": "myregistry.azurecr.io"}
+    msg_body = {"id": "guid", "action": "start", "name": "mybundle", "version": "1.0.0", "parameters": {"param1": "value1"}}
+    mock_get_porter_parameter_keys.return_value = ["param1"]
 
-    # Verify the dict and list are base64 encoded
-    import base64
+    commands, param_set_file, param_set_name, installation_file = await build_porter_command(config, msg_body, custom_action=True)
+    try:
+        assert installation_file is None
 
-    dict_encoded = base64.b64encode(json.dumps(dict_value).encode("ascii")).decode("ascii")
-    list_encoded = base64.b64encode(json.dumps(list_value).encode("ascii")).decode("ascii")
+        # First command applies the parameter set
+        assert commands[0] == ["porter", "parameters", "apply", param_set_file]
 
-    assert dict_param == f"dict_param={dict_encoded}"
-    assert list_param == f"list_param={list_encoded}"
+        # Second command uses porter invoke --action (not installation apply)
+        assert commands[1] == [
+            "porter", "invoke", "--action", "start", "guid",
+            "--reference", "myregistry.azurecr.io/mybundle:v1.0.0",
+            "--parameter-set", param_set_name,
+            "--force",
+            "--credential-set", "arm_auth",
+            "--credential-set", "aad_auth"
+        ]
+    finally:
+        if param_set_file and os.path.exists(param_set_file):
+            os.unlink(param_set_file)
 
 
 @pytest.mark.asyncio
@@ -184,7 +297,8 @@ async def test_get_porter_parameter_keys(mock_run_command_helper):
     assert command_args[0] == "porter"
     assert command_args[1] == "explain"
     assert "--reference" in command_args
-    assert f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}" == command_args[3]
+    expected_reference = "{}/{}:v{}".format(config['registry_server'], msg_body['name'], msg_body['version'])
+    assert expected_reference == command_args[3]
 
 
 @pytest.mark.asyncio