diff --git a/.gitignore b/.gitignore index 3001631ecb..cb0388c093 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,7 @@ example_cluster/paasta/docker_registry.json general_itests/fake_etc_paasta/clusters.json pip-wheel-metadata debian/debhelper-build-stamp +unique-run # Coverage artifacts .coverage diff --git a/paasta_tools/cli/schemas/kubernetes_schema.json b/paasta_tools/cli/schemas/kubernetes_schema.json index 8c7ab2c7ba..9a384ed989 100644 --- a/paasta_tools/cli/schemas/kubernetes_schema.json +++ b/paasta_tools/cli/schemas/kubernetes_schema.json @@ -555,6 +555,7 @@ }, "items": { "type": "array", + "maxItems": 1, "items": { "type": "object", "properties": { diff --git a/paasta_tools/cli/schemas/tron_schema.json b/paasta_tools/cli/schemas/tron_schema.json index 1e0b872baf..9134bfc934 100644 --- a/paasta_tools/cli/schemas/tron_schema.json +++ b/paasta_tools/cli/schemas/tron_schema.json @@ -236,6 +236,51 @@ }, "uniqueItems": true }, + "secret_volumes": { + "type": "array", + "items": { + "type": "object", + "properties": { + "container_path": { + "type": "string" + }, + "secret_name": { + "type": "string" + }, + "default_mode": { + "type": "string" + }, + "items": { + "type": "array", + "maxItems": 1, + "items": { + "type": "object", + "properties": { + "key": { + "type": "string" + }, + "path": { + "type": "string" + }, + "mode": { + "type": "string" + } + }, + "required": [ + "key", + "path" + ] + }, + "uniqueItems": true + } + }, + "required": [ + "container_path", + "secret_name" + ] + }, + "uniqueItems": true + }, "cluster": { "type": "string" }, diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index 3cd2e71ba5..3dcd558de0 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -1414,7 +1414,7 @@ def get_pod_volumes( items = None pod_volumes.append( V1Volume( - name=self.get_secret_volume_name(secret_volume), + name=self.get_secret_volume_name(secret_volume=secret_volume), secret=V1SecretVolumeSource( secret_name=get_paasta_secret_name( self.get_namespace(), diff --git a/paasta_tools/secret_tools.py b/paasta_tools/secret_tools.py index 4dc35f5cf9..bfbefd00e6 100644 --- a/paasta_tools/secret_tools.py +++ b/paasta_tools/secret_tools.py @@ -42,6 +42,14 @@ def is_shared_secret(env_var_val: str) -> bool: return env_var_val.startswith("SHARED_") +def is_shared_secret_from_secret_name(soa_dir: str, secret_name: str) -> bool: + """Alternative way of figuring if a secret is shared, directly from the secret_name.""" + secret_path = os.path.join( + soa_dir, SHARED_SECRET_SERVICE, "secrets", f"{secret_name}.json" + ) + return os.path.isfile(secret_path) + + def get_hmac_for_secret( env_var_val: str, service: str, soa_dir: str, secret_environment: str ) -> Optional[str]: diff --git a/paasta_tools/tron_tools.py b/paasta_tools/tron_tools.py index 9115f8711e..406e5591ab 100644 --- a/paasta_tools/tron_tools.py +++ b/paasta_tools/tron_tools.py @@ -52,6 +52,7 @@ from paasta_tools.utils import NoDeploymentsAvailable from paasta_tools.utils import time_cache from paasta_tools.utils import filter_templates_from_config +from paasta_tools.utils import TronSecretVolume from paasta_tools.kubernetes_tools import ( allowlist_denylist_to_requirements, create_or_find_service_account_name, @@ -68,6 +69,7 @@ ) from paasta_tools.secret_tools import is_secret_ref from paasta_tools.secret_tools import is_shared_secret +from paasta_tools.secret_tools import is_shared_secret_from_secret_name from paasta_tools.secret_tools import get_secret_name_from_ref from paasta_tools.kubernetes_tools import get_paasta_secret_name from paasta_tools.secret_tools import SHARED_SECRET_SERVICE @@ -413,10 +415,37 @@ def get_job_name(self): def get_action_name(self): return self.action + def get_secret_volumes(self) -> List[TronSecretVolume]: + """Adds the secret_volume_name to the objet so tron/task_processing can load it downstream without replicating code.""" + secret_volumes = super().get_secret_volumes() + return [ + TronSecretVolume( + secret_volume_name=self.get_secret_volume_name( + secret_volume["secret_name"] + ), + **secret_volume, + ) + for secret_volume in secret_volumes + ] + def get_namespace(self) -> str: """Get namespace from config, default to 'paasta'""" return self.config_dict.get("namespace", KUBERNETES_NAMESPACE) + def get_secret_volume_name(self, secret_name: str) -> str: + service = ( + self.service + if not is_shared_secret_from_secret_name( + soa_dir=self.soa_dir, secret_name=secret_name + ) + else SHARED_SECRET_SERVICE + ) + return get_paasta_secret_name( + self.get_namespace(), + service, + secret_name, + ) + def get_deploy_group(self) -> Optional[str]: return self.config_dict.get("deploy_group", None) @@ -869,6 +898,7 @@ def format_tron_action_dict(action_config: TronActionConfig, use_k8s: bool = Fal "node": action_config.get_node(), "retries": action_config.get_retries(), "retries_delay": action_config.get_retries_delay(), + "secret_volumes": action_config.get_secret_volumes(), "expected_runtime": action_config.get_expected_runtime(), "trigger_downstreams": action_config.get_trigger_downstreams(), "triggered_by": action_config.get_triggered_by(), diff --git a/paasta_tools/utils.py b/paasta_tools/utils.py index aef59020db..da6844394a 100644 --- a/paasta_tools/utils.py +++ b/paasta_tools/utils.py @@ -273,6 +273,10 @@ class SecretVolume(TypedDict, total=False): items: List[SecretVolumeItem] +class TronSecretVolume(SecretVolume, total=False): + secret_volume_name: str + + class MonitoringDict(TypedDict, total=False): alert_after: Union[str, float] check_every: str diff --git a/tests/test_tron_tools.py b/tests/test_tron_tools.py index cfbc62b494..bdfaafe2b0 100644 --- a/tests/test_tron_tools.py +++ b/tests/test_tron_tools.py @@ -1,5 +1,7 @@ import datetime import hashlib +import os +import tempfile import mock import pytest @@ -7,6 +9,7 @@ from paasta_tools import tron_tools from paasta_tools import utils +from paasta_tools.secret_tools import SHARED_SECRET_SERVICE from paasta_tools.tron_tools import MASTER_NAMESPACE from paasta_tools.tron_tools import MESOS_EXECUTOR_NAMES from paasta_tools.utils import CAPS_DROP @@ -142,6 +145,63 @@ def test_get_secret_env(self, action_config, test_env, expected_env): secret_env = action_config.get_secret_env() assert secret_env == expected_env + @pytest.mark.parametrize( + ("test_secret_volumes", "expected_secret_volumes"), + ( + ( + [ + { + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], + [ + { + "secret_volume_name": "tron-secret-my--service-secret1", + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], + ), + ), + ) + def test_get_secret_volumes( + self, action_config, test_secret_volumes, expected_secret_volumes + ): + action_config.config_dict["secret_volumes"] = test_secret_volumes + secret_volumes = action_config.get_secret_volumes() + assert secret_volumes == expected_secret_volumes + + @pytest.mark.parametrize( + ("is_shared, secret_name, expected_secret_volume_name"), + ( + (False, "secret1", "tron-secret-my--service-secret1"), + (True, "secret1", "tron-secret-underscore-shared-secret1"), + ), + ) + def test_get_secret_volume_name( + self, action_config, is_shared, secret_name, expected_secret_volume_name + ): + + with tempfile.TemporaryDirectory() as dir_path: + service = action_config.service if not is_shared else SHARED_SECRET_SERVICE + secret_path = os.path.join( + dir_path, service, "secrets", f"{secret_name}.json" + ) + os.makedirs(os.path.dirname(secret_path), exist_ok=True) + with open(secret_path, "w") as f: + f.write("FOOBAR") + + with mock.patch.object(action_config, "soa_dir", dir_path): + assert ( + action_config.get_secret_volume_name(secret_name) + == expected_secret_volume_name + ) + def test_get_executor_default(self, action_config): assert action_config.get_executor() == "paasta" @@ -787,6 +847,14 @@ def test_format_tron_action_dict_paasta(self): "disk": 42, "pool": "special_pool", "env": {"SHELL": "/bin/bash"}, + "secret_volumes": [ + { + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"containerPath": "/nail/tmp", "hostPath": "/nail/tmp", "mode": "RW"} ], @@ -835,6 +903,15 @@ def test_format_tron_action_dict_paasta(self): "mem": 1200, "disk": 42, "env": mock.ANY, + "secret_volumes": [ + { + "secret_volume_name": "tron-secret-my--service-secret1", + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"container_path": "/nail/tmp", "host_path": "/nail/tmp", "mode": "RW"} ], @@ -875,6 +952,14 @@ def test_format_tron_action_dict_spark(self): "disk": 42, "pool": "special_pool", "env": {"SHELL": "/bin/bash"}, + "secret_volumes": [ + { + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"containerPath": "/nail/tmp", "hostPath": "/nail/tmp", "mode": "RW"} ], @@ -1003,6 +1088,15 @@ def test_format_tron_action_dict_spark(self): "mem": 1200, "disk": 42, "env": mock.ANY, + "secret_volumes": [ + { + "secret_volume_name": "tron-secret-my--service-secret1", + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"container_path": "/nail/tmp", "host_path": "/nail/tmp", "mode": "RW"} ], @@ -1094,6 +1188,7 @@ def test_format_tron_action_dict_paasta_k8s_service_account(self): "env": mock.ANY, "secret_env": {}, "field_selector_env": {"PAASTA_POD_IP": {"field_path": "status.podIP"}}, + "secret_volumes": [], "extra_volumes": [], "service_account_name": "a-magic-sa", } @@ -1140,6 +1235,14 @@ def test_format_tron_action_dict_paasta_k8s( "disk": 42, "pool": "special_pool", "env": {"SHELL": "/bin/bash", "SOME_SECRET": "SECRET(secret_name)"}, + "secret_volumes": [ + { + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"containerPath": "/nail/tmp", "hostPath": "/nail/tmp", "mode": "RW"} ], @@ -1178,6 +1281,10 @@ def test_format_tron_action_dict_paasta_k8s( ), mock.patch( "paasta_tools.tron_tools.load_system_paasta_config", autospec=True, + ), mock.patch( + "paasta_tools.secret_tools.is_shared_secret_from_secret_name", + autospec=True, + return_value=False, ): result = tron_tools.format_tron_action_dict(action_config, use_k8s=True) @@ -1218,6 +1325,15 @@ def test_format_tron_action_dict_paasta_k8s( "key": "secret_name", } }, + "secret_volumes": [ + { + "secret_volume_name": "tron-secret-my--service-secret1", + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "field_selector_env": {"PAASTA_POD_IP": {"field_path": "status.podIP"}}, "extra_volumes": [ {"container_path": "/nail/tmp", "host_path": "/nail/tmp", "mode": "RW"} @@ -1248,6 +1364,14 @@ def test_format_tron_action_dict_paasta_no_branch_dict(self): "disk": 42, "pool": "special_pool", "env": {"SHELL": "/bin/bash"}, + "secret_volumes": [ + { + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"containerPath": "/nail/tmp", "hostPath": "/nail/tmp", "mode": "RW"} ], @@ -1273,7 +1397,6 @@ def test_format_tron_action_dict_paasta_no_branch_dict(self): autospec=True, ): result = tron_tools.format_tron_action_dict(action_config) - assert result == { "command": "echo something", "requires": ["required_action"], @@ -1284,6 +1407,15 @@ def test_format_tron_action_dict_paasta_no_branch_dict(self): "mem": 1200, "disk": 42, "env": mock.ANY, + "secret_volumes": [ + { + "secret_volume_name": "tron-secret-my--service-secret1", + "secret_name": "secret1", + "container_path": "/b/c", + "default_mode": "0644", + "items": [{"key": "secret1", "path": "abc"}], + } + ], "extra_volumes": [ {"container_path": "/nail/tmp", "host_path": "/nail/tmp", "mode": "RW"} ], @@ -1430,7 +1562,7 @@ def test_create_complete_config_e2e(self, tmpdir): # that are not static, this will cause continuous reconfiguration, which # will add significant load to the Tron API, which happened in DAR-1461. # but if this is intended, just change the hash. - assert hasher.hexdigest() == "f740410f7ae2794f9924121c1115e15d" + assert hasher.hexdigest() == "35972651618a848ac6bf7947245dbaea" def test_override_default_pool_override(self, tmpdir): soa_dir = tmpdir.mkdir("test_create_complete_config_soa")