diff --git a/.gitignore b/.gitignore index cb0388c093..3001631ecb 100644 --- a/.gitignore +++ b/.gitignore @@ -48,7 +48,6 @@ 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/cmds/spark_run.py b/paasta_tools/cli/cmds/spark_run.py index 9638cd0467..568cbd45ba 100644 --- a/paasta_tools/cli/cmds/spark_run.py +++ b/paasta_tools/cli/cmds/spark_run.py @@ -1203,7 +1203,7 @@ def paasta_spark_run(args): document = POD_TEMPLATE.format( spark_pod_label=limit_size_with_hash(f"exec-{app_base_name}"), ) - parsed_pod_template = yaml.safe_load(document) + parsed_pod_template = yaml.load(document) with open(pod_template_path, "w") as f: yaml.dump(parsed_pod_template, f) diff --git a/paasta_tools/cli/schemas/kubernetes_schema.json b/paasta_tools/cli/schemas/kubernetes_schema.json index dc9eef5473..6b088c5d93 100644 --- a/paasta_tools/cli/schemas/kubernetes_schema.json +++ b/paasta_tools/cli/schemas/kubernetes_schema.json @@ -557,7 +557,6 @@ }, "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 9134bfc934..1e0b872baf 100644 --- a/paasta_tools/cli/schemas/tron_schema.json +++ b/paasta_tools/cli/schemas/tron_schema.json @@ -236,51 +236,6 @@ }, "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/secret_tools.py b/paasta_tools/secret_tools.py index bfbefd00e6..4dc35f5cf9 100644 --- a/paasta_tools/secret_tools.py +++ b/paasta_tools/secret_tools.py @@ -42,14 +42,6 @@ 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 2f40a4e650..9115f8711e 100644 --- a/paasta_tools/tron_tools.py +++ b/paasta_tools/tron_tools.py @@ -52,7 +52,6 @@ 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, @@ -69,7 +68,6 @@ ) 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 @@ -415,40 +413,10 @@ def get_job_name(self): def get_action_name(self): return self.action - def get_secret_volumes(self) -> List[TronSecretVolume]: # type: ignore - """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_name=secret_volume["secret_name"], - container_path=secret_volume["container_path"], - default_mode=secret_volume["default_mode"], - items=secret_volume["items"], - ) - 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) @@ -901,7 +869,6 @@ 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 29cc57e59b..1c33ecf139 100644 --- a/paasta_tools/utils.py +++ b/paasta_tools/utils.py @@ -273,10 +273,6 @@ 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/requirements.txt b/requirements.txt index 30e16b3f68..89da4ec80b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -79,7 +79,7 @@ python-iptables==1.0.1 python-utils==2.0.1 pytimeparse==1.1.5 pytz==2016.10 -pyyaml==6.0.1 +pyyaml==5.4.1 repoze.lru==0.6 requests==2.25.0 requests-cache==0.6.3 diff --git a/tests/test_tron_tools.py b/tests/test_tron_tools.py index bdfaafe2b0..cfbc62b494 100644 --- a/tests/test_tron_tools.py +++ b/tests/test_tron_tools.py @@ -1,7 +1,5 @@ import datetime import hashlib -import os -import tempfile import mock import pytest @@ -9,7 +7,6 @@ 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 @@ -145,63 +142,6 @@ 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" @@ -847,14 +787,6 @@ 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"} ], @@ -903,15 +835,6 @@ 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"} ], @@ -952,14 +875,6 @@ 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"} ], @@ -1088,15 +1003,6 @@ 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"} ], @@ -1188,7 +1094,6 @@ 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", } @@ -1235,14 +1140,6 @@ 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"} ], @@ -1281,10 +1178,6 @@ 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) @@ -1325,15 +1218,6 @@ 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"} @@ -1364,14 +1248,6 @@ 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"} ], @@ -1397,6 +1273,7 @@ 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"], @@ -1407,15 +1284,6 @@ 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"} ], @@ -1562,7 +1430,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() == "35972651618a848ac6bf7947245dbaea" + assert hasher.hexdigest() == "f740410f7ae2794f9924121c1115e15d" def test_override_default_pool_override(self, tmpdir): soa_dir = tmpdir.mkdir("test_create_complete_config_soa")