From f7094ef9aec2129cbd2b522c69e057fe2e49128c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 07:36:13 +0000 Subject: [PATCH 1/7] Initial plan From 53f315f2c282048b61cbeb750b3207a38cb885a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 07:44:36 +0000 Subject: [PATCH 2/7] fix: replace --param CLI args with Porter parameter set file to avoid ARG_MAX limits When many workspaces (~25+) are deployed, the aggregated base64-encoded parameter values passed as --param CLI arguments to Porter exceed the Linux execve limits (ARG_MAX ~2MiB, MAX_ARG_STRLEN 128KiB per arg), causing OSError: [Errno 7] Argument list too long. Fix by writing all parameters to a temporary JSON parameter set file (/tmp/tre-params-{installation_id}.json) and passing it via a single --parameter-set argument instead. The temp file is cleaned up after the Porter command completes. Fixes #4903 Agent-Logs-Url: https://github.com/microsoft/AzureTRE/sessions/12b6c77f-6b2b-4b0a-bc96-76ba95ef0ade Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> --- CHANGELOG.md | 3 + resource_processor/helpers/commands.py | 29 ++++- resource_processor/tests_rp/test_commands.py | 127 +++++++++++++------ resource_processor/vmss_porter/runner.py | 18 ++- 4 files changed, 129 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ab4bcb537..5a234f22ce 100644 --- a/CHANGELOG.md +++ b/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 replacing `--param` CLI arguments with a temporary Porter parameter set file ([#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)) diff --git a/resource_processor/helpers/commands.py b/resource_processor/helpers/commands.py index 35047d309a..6dbd77c942 100644 --- a/resource_processor/helpers/commands.py +++ b/resource_processor/helpers/commands.py @@ -2,6 +2,7 @@ import json import base64 import logging +import os from urllib.parse import urlparse from shared.logging import logger, shell_output_logger @@ -78,7 +79,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,10 +118,29 @@ 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'] + # Write parameters to a temporary parameter set file to avoid ARG_MAX / MAX_ARG_STRLEN limits + # when many workspaces are deployed and parameter values (e.g. base64-encoded rule_collections) + # exceed the Linux execve limits. + param_set_file = None + if param_set_entries: + param_set_file = f"/tmp/tre-params-{installation_id}.json" + param_set = { + "schemaType": "ParameterSet", + "schemaVersion": "1.0.1", + "name": f"tre-params-{installation_id}", + "namespace": "", + "parameters": param_set_entries + } + with open(param_set_file, "w") as f: + json.dump(param_set, f) + command = ["porter"] if custom_action: command.extend(["invoke", "--action"]) @@ -131,7 +151,8 @@ async def build_porter_command(config, msg_body, custom_action=False): "--reference", f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}" ]) - command.extend(porter_parameters) + if param_set_file: + command.extend(["--parameter-set", param_set_file]) command.append("--force") command.extend(["--credential-set", "arm_auth"]) command.extend(["--credential-set", "aad_auth"]) @@ -139,7 +160,7 @@ async def build_porter_command(config, msg_body, custom_action=False): if msg_body['action'] == 'upgrade': command.append("--force-upgrade") - return [command] + return ([command], param_set_file) async def build_porter_command_for_outputs(msg_body): diff --git a/resource_processor/tests_rp/test_commands.py b/resource_processor/tests_rp/test_commands.py index 2caab5c0bd..e89cc17684 100644 --- a/resource_processor/tests_rp/test_commands.py +++ b/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,30 @@ 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" - ]] + commands, param_set_file = await build_porter_command(config, msg_body) + try: + assert commands == [[ + "porter", "install", "guid", + "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", + "--parameter-set", param_set_file, + "--force", + "--credential-set", "arm_auth", + "--credential-set", "aad_auth" + ]] - command = await build_porter_command(config, msg_body) - assert command == expected_command + assert param_set_file is not None + assert os.path.exists(param_set_file) + + with open(param_set_file) as f: + param_set = json.load(f) + + assert param_set["schemaType"] == "ParameterSet" + assert param_set["name"] == "tre-params-guid" + assert len(param_set["parameters"]) == 1 + assert param_set["parameters"][0] == {"name": "param1", "source": {"value": "value1"}} + finally: + if param_set_file and os.path.exists(param_set_file): + os.unlink(param_set_file) @pytest.mark.asyncio @@ -78,18 +92,23 @@ 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" - ]] - - command = await build_porter_command(config, msg_body) - assert command == expected_command + commands, param_set_file = await build_porter_command(config, msg_body) + try: + assert commands == [[ + "porter", "upgrade", "guid", + "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", + "--parameter-set", param_set_file, + "--force", + "--credential-set", "arm_auth", + "--credential-set", "aad_auth", + "--force-upgrade" + ]] + + assert param_set_file is not None + assert os.path.exists(param_set_file) + finally: + if param_set_file and os.path.exists(param_set_file): + os.unlink(param_set_file) @pytest.mark.asyncio @@ -106,6 +125,25 @@ 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 returns no --parameter-set when there are no parameters.""" + 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 = await build_porter_command(config, msg_body) + + assert param_set_file is None + assert commands == [[ + "porter", "install", "guid", + "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", + "--force", + "--credential-set", "arm_auth", + "--credential-set", "aad_auth" + ]] + + @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 +165,38 @@ 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 = await build_porter_command(config, msg_body) + + try: + command_args = commands[0] - # Verify the command contains properly encoded complex parameters - command_args = command[0] + # Command should have --parameter-set instead of --param + assert "--parameter-set" in command_args + assert "--param" not in command_args - # 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 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) - # 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) + params_by_name = {p["name"]: p["source"]["value"] for p in param_set["parameters"]} - 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 "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" - # Verify the dict and list are base64 encoded - import base64 + # Verify the dict and list are base64 encoded + import base64 - dict_encoded = base64.b64encode(json.dumps(dict_value).encode("ascii")).decode("ascii") - list_encoded = base64.b64encode(json.dumps(list_value).encode("ascii")).decode("ascii") + 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 == f"dict_param={dict_encoded}" - assert list_param == f"list_param={list_encoded}" + assert params_by_name["dict_param"] == dict_encoded + assert params_by_name["list_param"] == list_encoded + finally: + if param_set_file and os.path.exists(param_set_file): + os.unlink(param_set_file) @pytest.mark.asyncio diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index 120ececba0..8a364e8397 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -3,6 +3,7 @@ from multiprocessing import Process import json import asyncio +import os import sys from helpers.commands import azure_acr_login_command, azure_login_command, build_porter_command, build_porter_command_for_outputs, apply_porter_credentials_sets_command, run_command_helper from shared.config import get_config @@ -178,12 +179,19 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf # Build and run porter command (flagging if its a built-in action or custom so we can adapt porter command appropriately) is_custom_action = action not in ["install", "upgrade", "uninstall"] - porter_command = await build_porter_command(config, msg_body, is_custom_action) + porter_command, param_set_file = await build_porter_command(config, msg_body, is_custom_action) logger.debug("Starting to run porter execution command...") returncode, _, err = await run_porter(porter_command, config) logger.debug("Finished running porter execution command.") + # Clean up the temporary parameter set file now that the porter command has completed + if param_set_file: + try: + os.unlink(param_set_file) + except OSError: + pass + action_completed_without_error = False if returncode == 0: @@ -203,8 +211,14 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf if "upgrade" == action and ("could not find installation" in err or "The installation cannot be upgraded, because it is not installed." in err): logger.warning("Upgrade failed, attempting install...") msg_body['action'] = "install" - porter_command = await build_porter_command(config, msg_body, False) + porter_command, param_set_file = await build_porter_command(config, msg_body, False) returncode, _, err = await run_porter(porter_command, config) + # Clean up the temporary parameter set file for the fallback install command + if param_set_file: + try: + os.unlink(param_set_file) + except OSError: + pass if returncode == 0: action_completed_without_error = True From 029877acdcfd834107b1b0289cf5453e77718857 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 08:11:17 +0000 Subject: [PATCH 3/7] fix: address review feedback - use tempfile, porter parameters apply/delete, try/finally cleanup - Replace import os with import tempfile in commands.py (os was unused there) - Use tempfile.NamedTemporaryFile for secure temp file creation (0o600 permissions, non-predictable path) instead of open() to /tmp/ - Switch to porter parameters apply workflow: include `porter parameters apply ` as the first command in the returned list, then pass `--parameter-set ` (set name, not file path) to the main porter command - Wrap porter execution in try/finally in runner.py to guarantee cleanup even on exceptions or cancellation; cleanup now also runs `porter parameters delete ` to remove the registered parameter set from Porter's store - Update all tests to expect the new [apply_cmd, main_cmd] structure and --parameter-set with the set name Agent-Logs-Url: https://github.com/microsoft/AzureTRE/sessions/69f73131-900a-4ce5-9632-109910a3401e Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> --- resource_processor/helpers/commands.py | 22 +++++++--- resource_processor/tests_rp/test_commands.py | 45 ++++++++++++-------- resource_processor/vmss_porter/runner.py | 37 +++++++++------- 3 files changed, 65 insertions(+), 39 deletions(-) diff --git a/resource_processor/helpers/commands.py b/resource_processor/helpers/commands.py index 6dbd77c942..78d4704785 100644 --- a/resource_processor/helpers/commands.py +++ b/resource_processor/helpers/commands.py @@ -2,7 +2,7 @@ import json import base64 import logging -import os +import tempfile from urllib.parse import urlparse from shared.logging import logger, shell_output_logger @@ -124,22 +124,28 @@ async def build_porter_command(config, msg_body, custom_action=False): }) installation_id = msg_body['id'] + param_set_name = f"tre-params-{installation_id}" # Write parameters to a temporary parameter set file to avoid ARG_MAX / MAX_ARG_STRLEN limits # when many workspaces are deployed and parameter values (e.g. base64-encoded rule_collections) - # exceed the Linux execve limits. + # exceed the Linux execve limits. Use tempfile for secure file creation (mode 0o600, no + # predictable path) then apply the set to Porter's store via `porter parameters apply`. param_set_file = None if param_set_entries: - param_set_file = f"/tmp/tre-params-{installation_id}.json" param_set = { "schemaType": "ParameterSet", "schemaVersion": "1.0.1", - "name": f"tre-params-{installation_id}", + "name": param_set_name, "namespace": "", "parameters": param_set_entries } - with open(param_set_file, "w") as f: + with tempfile.NamedTemporaryFile(mode='w', suffix='.json', delete=False) as f: json.dump(param_set, f) + param_set_file = f.name + + commands = [] + if param_set_file: + commands.append(["porter", "parameters", "apply", param_set_file]) command = ["porter"] if custom_action: @@ -152,7 +158,7 @@ async def build_porter_command(config, msg_body, custom_action=False): f"{config['registry_server']}/{msg_body['name']}:v{msg_body['version']}" ]) if param_set_file: - command.extend(["--parameter-set", 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"]) @@ -160,7 +166,9 @@ async def build_porter_command(config, msg_body, custom_action=False): if msg_body['action'] == 'upgrade': command.append("--force-upgrade") - return ([command], param_set_file) + commands.append(command) + + return (commands, param_set_file) async def build_porter_command_for_outputs(msg_body): diff --git a/resource_processor/tests_rp/test_commands.py b/resource_processor/tests_rp/test_commands.py index e89cc17684..677b51ddc3 100644 --- a/resource_processor/tests_rp/test_commands.py +++ b/resource_processor/tests_rp/test_commands.py @@ -61,17 +61,21 @@ async def test_build_porter_command(mock_get_porter_parameter_keys): commands, param_set_file = await build_porter_command(config, msg_body) try: - assert commands == [[ + assert param_set_file is not None + assert os.path.exists(param_set_file) + + # First command applies the parameter set to Porter's store + assert commands[0] == ["porter", "parameters", "apply", param_set_file] + + # Second command is the main porter install using the parameter set by name + assert commands[1] == [ "porter", "install", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", - "--parameter-set", param_set_file, + "--parameter-set", "tre-params-guid", "--force", "--credential-set", "arm_auth", "--credential-set", "aad_auth" - ]] - - assert param_set_file is not None - assert os.path.exists(param_set_file) + ] with open(param_set_file) as f: param_set = json.load(f) @@ -94,18 +98,22 @@ async def test_build_porter_command_for_upgrade(mock_get_porter_parameter_keys): commands, param_set_file = await build_porter_command(config, msg_body) try: - assert commands == [[ + assert param_set_file is not None + assert os.path.exists(param_set_file) + + # First command applies the parameter set to Porter's store + assert commands[0] == ["porter", "parameters", "apply", param_set_file] + + # Second command is the main porter upgrade using the parameter set by name + assert commands[1] == [ "porter", "upgrade", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", - "--parameter-set", param_set_file, + "--parameter-set", "tre-params-guid", "--force", "--credential-set", "arm_auth", "--credential-set", "aad_auth", "--force-upgrade" - ]] - - assert param_set_file is not None - assert os.path.exists(param_set_file) + ] finally: if param_set_file and os.path.exists(param_set_file): os.unlink(param_set_file) @@ -168,11 +176,14 @@ async def test_build_porter_command_with_complex_parameters(mock_get_porter_para commands, param_set_file = await build_porter_command(config, msg_body) try: - command_args = commands[0] - - # Command should have --parameter-set instead of --param - assert "--parameter-set" in command_args - assert "--param" not in command_args + # First command is the apply command + assert commands[0] == ["porter", "parameters", "apply", param_set_file] + + # Main porter command should reference the parameter set by name + main_command = commands[1] + assert "--parameter-set" in main_command + assert "tre-params-guid" in main_command + assert "--param" not in main_command # Verify the param set file contains the correct parameters assert param_set_file is not None diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index 8a364e8397..4258bfbf73 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -182,16 +182,19 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf porter_command, param_set_file = await build_porter_command(config, msg_body, is_custom_action) logger.debug("Starting to run porter execution command...") - returncode, _, err = await run_porter(porter_command, config) + try: + returncode, _, err = await run_porter(porter_command, config) + finally: + # Clean up the temporary parameter set file and remove it from Porter's store + if param_set_file: + param_set_name = f"tre-params-{installation_id}" + await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") + try: + os.unlink(param_set_file) + except OSError: + pass logger.debug("Finished running porter execution command.") - # Clean up the temporary parameter set file now that the porter command has completed - if param_set_file: - try: - os.unlink(param_set_file) - except OSError: - pass - action_completed_without_error = False if returncode == 0: @@ -212,13 +215,17 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf logger.warning("Upgrade failed, attempting install...") msg_body['action'] = "install" porter_command, param_set_file = await build_porter_command(config, msg_body, False) - returncode, _, err = await run_porter(porter_command, config) - # Clean up the temporary parameter set file for the fallback install command - if param_set_file: - try: - os.unlink(param_set_file) - except OSError: - pass + try: + returncode, _, err = await run_porter(porter_command, config) + finally: + # Clean up the temporary parameter set file for the fallback install command + if param_set_file: + param_set_name = f"tre-params-{installation_id}" + await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") + try: + os.unlink(param_set_file) + except OSError: + pass if returncode == 0: action_completed_without_error = True From 2ee02db46e16a85a618d879c1484650b9a359104 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 08:31:47 +0000 Subject: [PATCH 4/7] refactor: return param_set_name from build_porter_command and extract cleanup helper - build_porter_command now returns a 3-tuple (commands, param_set_file, param_set_name) so callers get the exact set name without re-deriving it - Extract _cleanup_param_set() async helper in runner.py that runs `porter parameters delete` + os.unlink, eliminating the duplicated cleanup blocks in invoke_porter_action - Update both call sites in invoke_porter_action to unpack the 3-tuple and call the shared helper - Update all tests to unpack the 3-tuple and assert param_set_name Agent-Logs-Url: https://github.com/microsoft/AzureTRE/sessions/6280ded8-8f2a-40f6-8c42-2199c1374dbc Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> --- resource_processor/helpers/commands.py | 2 +- resource_processor/tests_rp/test_commands.py | 11 +++++--- resource_processor/vmss_porter/runner.py | 27 ++++++++++---------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/resource_processor/helpers/commands.py b/resource_processor/helpers/commands.py index 78d4704785..19140e9fb6 100644 --- a/resource_processor/helpers/commands.py +++ b/resource_processor/helpers/commands.py @@ -168,7 +168,7 @@ async def build_porter_command(config, msg_body, custom_action=False): commands.append(command) - return (commands, param_set_file) + return (commands, param_set_file, param_set_name) async def build_porter_command_for_outputs(msg_body): diff --git a/resource_processor/tests_rp/test_commands.py b/resource_processor/tests_rp/test_commands.py index 677b51ddc3..7b35ccd250 100644 --- a/resource_processor/tests_rp/test_commands.py +++ b/resource_processor/tests_rp/test_commands.py @@ -59,9 +59,10 @@ 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"] - commands, param_set_file = await build_porter_command(config, msg_body) + commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) try: assert param_set_file is not None + assert param_set_name == "tre-params-guid" assert os.path.exists(param_set_file) # First command applies the parameter set to Porter's store @@ -96,9 +97,10 @@ 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"] - commands, param_set_file = await build_porter_command(config, msg_body) + commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) try: assert param_set_file is not None + assert param_set_name == "tre-params-guid" assert os.path.exists(param_set_file) # First command applies the parameter set to Porter's store @@ -140,9 +142,10 @@ async def test_build_porter_command_no_parameters(mock_get_porter_parameter_keys msg_body = {"id": "guid", "action": "install", "name": "mybundle", "version": "1.0.0", "parameters": {}} mock_get_porter_parameter_keys.return_value = [] - commands, param_set_file = await build_porter_command(config, msg_body) + commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) assert param_set_file is None + assert param_set_name == "tre-params-guid" assert commands == [[ "porter", "install", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", @@ -173,7 +176,7 @@ 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"] - commands, param_set_file = await build_porter_command(config, msg_body) + commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) try: # First command is the apply command diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index 4258bfbf73..9dddcecc37 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -162,6 +162,15 @@ def service_bus_message_generator(sb_message: dict, status: str, deployment_mess return resource_request_message +async def _cleanup_param_set(param_set_name: str, param_set_file: str, config: dict): + """Remove a Porter parameter set from its local store and delete the temp file.""" + await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") + try: + os.unlink(param_set_file) + except OSError: + pass + + async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, config: dict) -> bool: """ Handle resource message by invoking specified porter action (i.e. install, uninstall) @@ -179,7 +188,7 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf # Build and run porter command (flagging if its a built-in action or custom so we can adapt porter command appropriately) is_custom_action = action not in ["install", "upgrade", "uninstall"] - porter_command, param_set_file = await build_porter_command(config, msg_body, is_custom_action) + porter_command, param_set_file, param_set_name = await build_porter_command(config, msg_body, is_custom_action) logger.debug("Starting to run porter execution command...") try: @@ -187,12 +196,7 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf finally: # Clean up the temporary parameter set file and remove it from Porter's store if param_set_file: - param_set_name = f"tre-params-{installation_id}" - await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") - try: - os.unlink(param_set_file) - except OSError: - pass + await _cleanup_param_set(param_set_name, param_set_file, config) logger.debug("Finished running porter execution command.") action_completed_without_error = False @@ -214,18 +218,13 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf if "upgrade" == action and ("could not find installation" in err or "The installation cannot be upgraded, because it is not installed." in err): logger.warning("Upgrade failed, attempting install...") msg_body['action'] = "install" - porter_command, param_set_file = await build_porter_command(config, msg_body, False) + porter_command, param_set_file, param_set_name = await build_porter_command(config, msg_body, False) try: returncode, _, err = await run_porter(porter_command, config) finally: # Clean up the temporary parameter set file for the fallback install command if param_set_file: - param_set_name = f"tre-params-{installation_id}" - await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") - try: - os.unlink(param_set_file) - except OSError: - pass + await _cleanup_param_set(param_set_name, param_set_file, config) if returncode == 0: action_completed_without_error = True From 12e741d128ef7eefc2c09ac6c176374a9f864093 Mon Sep 17 00:00:00 2001 From: Marcus Robinson Date: Wed, 20 May 2026 16:05:09 +0100 Subject: [PATCH 5/7] Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- resource_processor/vmss_porter/runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index 9dddcecc37..8638a42ec3 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -167,8 +167,8 @@ async def _cleanup_param_set(param_set_name: str, param_set_file: str, config: d await run_command_helper(["porter", "parameters", "delete", param_set_name], config, "Delete parameter set") try: os.unlink(param_set_file) - except OSError: - pass + except OSError as e: + logger.debug(f"Best-effort cleanup: could not delete temp parameter set file '{param_set_file}': {e}") async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, config: dict) -> bool: From e7c1eed995a1ed51776f11415f8eb59729c590ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 15:58:18 +0000 Subject: [PATCH 6/7] chore: remove verbose comments and bump resource_processor version to 0.13.4 Agent-Logs-Url: https://github.com/microsoft/AzureTRE/sessions/e5ce1228-b98a-4813-8d53-cef74ea8ab5b Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> --- resource_processor/_version.py | 2 +- resource_processor/helpers/commands.py | 4 ---- resource_processor/vmss_porter/runner.py | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/resource_processor/_version.py b/resource_processor/_version.py index 26c36ca79a..f2b93b13de 100644 --- a/resource_processor/_version.py +++ b/resource_processor/_version.py @@ -1 +1 @@ -__version__ = "0.13.3" +__version__ = "0.13.4" diff --git a/resource_processor/helpers/commands.py b/resource_processor/helpers/commands.py index 19140e9fb6..d8a6fae3c8 100644 --- a/resource_processor/helpers/commands.py +++ b/resource_processor/helpers/commands.py @@ -126,10 +126,6 @@ async def build_porter_command(config, msg_body, custom_action=False): installation_id = msg_body['id'] param_set_name = f"tre-params-{installation_id}" - # Write parameters to a temporary parameter set file to avoid ARG_MAX / MAX_ARG_STRLEN limits - # when many workspaces are deployed and parameter values (e.g. base64-encoded rule_collections) - # exceed the Linux execve limits. Use tempfile for secure file creation (mode 0o600, no - # predictable path) then apply the set to Porter's store via `porter parameters apply`. param_set_file = None if param_set_entries: param_set = { diff --git a/resource_processor/vmss_porter/runner.py b/resource_processor/vmss_porter/runner.py index 8638a42ec3..3c8b9e4507 100644 --- a/resource_processor/vmss_porter/runner.py +++ b/resource_processor/vmss_porter/runner.py @@ -194,7 +194,6 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf try: returncode, _, err = await run_porter(porter_command, config) finally: - # Clean up the temporary parameter set file and remove it from Porter's store if param_set_file: await _cleanup_param_set(param_set_name, param_set_file, config) logger.debug("Finished running porter execution command.") @@ -222,7 +221,6 @@ async def invoke_porter_action(msg_body: dict, sb_client: ServiceBusClient, conf try: returncode, _, err = await run_porter(porter_command, config) finally: - # Clean up the temporary parameter set file for the fallback install command if param_set_file: await _cleanup_param_set(param_set_name, param_set_file, config) if returncode == 0: From 62afd6b807b0e631116ebced62ba49aaaf9ac9b0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 21 May 2026 11:32:40 +0000 Subject: [PATCH 7/7] fix: make Porter parameter set name unique per run using UUID suffix Agent-Logs-Url: https://github.com/microsoft/AzureTRE/sessions/c0be6c59-94cb-4a09-8225-b1726b0f6c58 Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> --- resource_processor/helpers/commands.py | 3 ++- resource_processor/tests_rp/test_commands.py | 16 +++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/resource_processor/helpers/commands.py b/resource_processor/helpers/commands.py index d8a6fae3c8..20c67c3160 100644 --- a/resource_processor/helpers/commands.py +++ b/resource_processor/helpers/commands.py @@ -3,6 +3,7 @@ import base64 import logging import tempfile +import uuid from urllib.parse import urlparse from shared.logging import logger, shell_output_logger @@ -124,7 +125,7 @@ async def build_porter_command(config, msg_body, custom_action=False): }) installation_id = msg_body['id'] - param_set_name = f"tre-params-{installation_id}" + param_set_name = f"tre-params-{installation_id}-{uuid.uuid4().hex[:8]}" param_set_file = None if param_set_entries: diff --git a/resource_processor/tests_rp/test_commands.py b/resource_processor/tests_rp/test_commands.py index 7b35ccd250..aa88f910d4 100644 --- a/resource_processor/tests_rp/test_commands.py +++ b/resource_processor/tests_rp/test_commands.py @@ -62,7 +62,8 @@ async def test_build_porter_command(mock_get_porter_parameter_keys): commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) try: assert param_set_file is not None - assert param_set_name == "tre-params-guid" + 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) # First command applies the parameter set to Porter's store @@ -72,7 +73,7 @@ async def test_build_porter_command(mock_get_porter_parameter_keys): assert commands[1] == [ "porter", "install", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", - "--parameter-set", "tre-params-guid", + "--parameter-set", param_set_name, "--force", "--credential-set", "arm_auth", "--credential-set", "aad_auth" @@ -82,7 +83,7 @@ async def test_build_porter_command(mock_get_porter_parameter_keys): param_set = json.load(f) assert param_set["schemaType"] == "ParameterSet" - assert param_set["name"] == "tre-params-guid" + assert param_set["name"] == param_set_name assert len(param_set["parameters"]) == 1 assert param_set["parameters"][0] == {"name": "param1", "source": {"value": "value1"}} finally: @@ -100,7 +101,7 @@ async def test_build_porter_command_for_upgrade(mock_get_porter_parameter_keys): commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) try: assert param_set_file is not None - assert param_set_name == "tre-params-guid" + assert param_set_name.startswith("tre-params-guid-") assert os.path.exists(param_set_file) # First command applies the parameter set to Porter's store @@ -110,7 +111,7 @@ async def test_build_porter_command_for_upgrade(mock_get_porter_parameter_keys): assert commands[1] == [ "porter", "upgrade", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", - "--parameter-set", "tre-params-guid", + "--parameter-set", param_set_name, "--force", "--credential-set", "arm_auth", "--credential-set", "aad_auth", @@ -145,7 +146,7 @@ async def test_build_porter_command_no_parameters(mock_get_porter_parameter_keys commands, param_set_file, param_set_name = await build_porter_command(config, msg_body) assert param_set_file is None - assert param_set_name == "tre-params-guid" + assert param_set_name.startswith("tre-params-guid-") assert commands == [[ "porter", "install", "guid", "--reference", "myregistry.azurecr.io/mybundle:v1.0.0", @@ -185,7 +186,8 @@ async def test_build_porter_command_with_complex_parameters(mock_get_porter_para # Main porter command should reference the parameter set by name main_command = commands[1] assert "--parameter-set" in main_command - assert "tre-params-guid" in main_command + assert param_set_name in main_command + assert param_set_name.startswith("tre-params-guid-") assert "--param" not in main_command # Verify the param set file contains the correct parameters