From 8b62c7f143297a138d5f704b3da3cadf9a4faefb Mon Sep 17 00:00:00 2001 From: Russell Date: Wed, 10 Nov 2021 11:25:03 -0500 Subject: [PATCH] change deployment subscription checks (#98) --- .../pipeline_config/pipeline.py | 247 ++++++++++++------ .../pipeline/test_frames_puff_map.py | 26 +- .../pipeline_config/pipeline/test_pipeline.py | 199 +++++++++----- 3 files changed, 316 insertions(+), 156 deletions(-) diff --git a/foodx_devops_tools/pipeline_config/pipeline.py b/foodx_devops_tools/pipeline_config/pipeline.py index 3464132..c341446 100644 --- a/foodx_devops_tools/pipeline_config/pipeline.py +++ b/foodx_devops_tools/pipeline_config/pipeline.py @@ -168,76 +168,171 @@ def check_clients(cls: pydantic.BaseModel, loaded_data: dict) -> dict: raise PipelineConfigurationError(message) return loaded_data - @pydantic.root_validator() - def check_deployments(cls: pydantic.BaseModel, loaded_data: dict) -> dict: - """Cross-check loaded deployment data.""" - deployment_names = set( - loaded_data["deployments"].deployment_tuples.keys() - ) + @staticmethod + def __check_deployment_tuple( + deployment_tuple: str, loaded_data: dict + ) -> None: + """Check for a valid deployment tuple in deployments.""" this_re = re.compile(DEPLOYMENT_NAME_REGEX) - for this_name in deployment_names: - result = this_re.match(this_name) + result = this_re.match(deployment_tuple) - if not result: - message = "Bad deployment tuple, {0}".format(this_name) - log.error("{0}, {1}".format(message, DEPLOYMENT_NAME_REGEX)) - raise PipelineConfigurationError(message) + if not result: + message = "Bad deployment tuple, {0}".format(deployment_tuple) + log.error("{0}, {1}".format(message, DEPLOYMENT_NAME_REGEX)) + raise PipelineConfigurationError(message) - this_client = result.group("client") - if this_client not in loaded_data["clients"]: - message = "Bad client in deployment tuple, {0}".format( - this_client + this_client = result.group("client") + if this_client not in loaded_data["clients"]: + message = "Bad client in deployment tuple, {0}".format(this_client) + log.error( + "{0}, {1}, {2}".format( + message, deployment_tuple, str(loaded_data["clients"]) ) - log.error( - "{0}, {1}, {2}".format( - message, this_name, str(loaded_data["clients"]) - ) + ) + raise PipelineConfigurationError(message) + + this_release_state = result.group("release_state") + if this_release_state not in loaded_data["release_states"]: + message = "Bad release state in deployment tuple, {0}".format( + this_release_state + ) + log.error( + "{0}, {1}, {2}".format( + message, + deployment_tuple, + str(loaded_data["release_states"]), ) - raise PipelineConfigurationError(message) + ) + raise PipelineConfigurationError(message) - this_release_state = result.group("release_state") - if this_release_state not in loaded_data["release_states"]: - message = "Bad release state in deployment tuple, {0}".format( - this_release_state + this_system = result.group("system") + if this_system not in loaded_data["systems"]: + message = "Bad system in deployment tuple, {0}".format(this_system) + log.error( + "{0}, {1}, {2}".format( + message, deployment_tuple, str(loaded_data["systems"]) ) - log.error( - "{0}, {1}, {2}".format( - message, this_name, str(loaded_data["release_states"]) + ) + raise PipelineConfigurationError(message) + + @staticmethod + def __check_subscription_in_puff_map( + deployment_tuple: str, subscription_name: str, puff_map_data: PuffMap + ) -> None: + puff_map_subscriptions: typing.Set[str] = set() + for _, frame_data in puff_map_data.frames.items(): + for _, application_data in frame_data.applications.items(): + for ( + _, + subscription_data, + ) in application_data.arm_parameters_files.items(): + puff_map_subscriptions = puff_map_subscriptions.union( + set(subscription_data.keys()) ) + + if subscription_name not in puff_map_subscriptions: + # a deployment subscription must be defined in puff_map.yml + message = ( + "Deployment subscription not defined in puff map, " + "{0}, {1}".format(deployment_tuple, subscription_name) + ) + log.error("{0}".format(message)) + raise PipelineConfigurationError(message) + + @staticmethod + def __report_deployment_subscription_error( + deployment_tuple: str, + subscription_name: str, + subscriptions: set, + category: str, + ) -> None: + message = ( + "Deployment subscription not defined in " + "{0}, {1}, {2}".format( + category, deployment_tuple, subscription_name + ) + ) + log.error( + "{0}, {1}".format( + message, + str(subscriptions), + ) + ) + raise PipelineConfigurationError(message) + + @staticmethod + def __check_deployment_subscriptions( + deployment_tuple: str, loaded_data: dict + ) -> None: + """Check for valid subscriptions in deployments.""" + deployment_subscriptions = set( + loaded_data["deployments"] + .deployment_tuples[deployment_tuple] + .subscriptions.keys() + ) + for this_deployment_subscription in deployment_subscriptions: + if this_deployment_subscription not in loaded_data["subscriptions"]: + # a deployment subscription must be defined in subscriptions.yml + PipelineConfiguration.__report_deployment_subscription_error( + deployment_tuple, + this_deployment_subscription, + set(loaded_data["subscriptions"].keys()), + "subscriptions", ) - raise PipelineConfigurationError(message) - this_system = result.group("system") - if this_system not in loaded_data["systems"]: - message = "Bad system in deployment tuple, {0}".format( - this_system + if ( + ("service_principals" in loaded_data) + and (loaded_data["service_principals"]) + and ( + this_deployment_subscription + not in loaded_data["service_principals"] ) - log.error( - "{0}, {1}, {2}".format( - message, this_name, str(loaded_data["systems"]) - ) + ): + # a deployment subscription must be defined in + # service_principals.vault + PipelineConfiguration.__report_deployment_subscription_error( + deployment_tuple, + this_deployment_subscription, + set(loaded_data["service_principals"].keys()), + "service principals", ) - raise PipelineConfigurationError(message) - this_subscriptions = ( - loaded_data["deployments"] - .deployment_tuples[this_name] - .subscriptions + if ( + ("static_secrets" in loaded_data) + and (loaded_data["static_secrets"]) + and ( + this_deployment_subscription + not in loaded_data["static_secrets"] + ) + ): + # a deployment subscription must be defined in static_secrets + PipelineConfiguration.__report_deployment_subscription_error( + deployment_tuple, + this_deployment_subscription, + set(loaded_data["static_secrets"].keys()), + "static secrets", + ) + + PipelineConfiguration.__check_subscription_in_puff_map( + deployment_tuple, + this_deployment_subscription, + loaded_data["puff_map"], ) - for subscription_name in this_subscriptions.keys(): - if subscription_name not in loaded_data["subscriptions"]: - message = "Bad subscription in deployment, {0}".format( - this_name - ) - log.error( - "{0}, {1}, {2}, {3}".format( - message, - this_name, - str(this_subscriptions.keys()), - str(loaded_data["subscriptions"]), - ) - ) - raise PipelineConfigurationError(message) + + @pydantic.root_validator() + def check_deployments(cls: pydantic.BaseModel, loaded_data: dict) -> dict: + """Cross-check loaded deployment data.""" + deployment_names = set( + loaded_data["deployments"].deployment_tuples.keys() + ) + for this_name in deployment_names: + typing.cast("PipelineConfiguration", cls).__check_deployment_tuple( + this_name, loaded_data + ) + typing.cast( + "PipelineConfiguration", cls + ).__check_deployment_subscriptions(this_name, loaded_data) + return loaded_data @pydantic.root_validator() @@ -246,32 +341,35 @@ def check_service_principals( ) -> dict: """Validate service principal data.""" if loaded_data["service_principals"]: - bad_subscriptions = [ - "{0}".format(name) - for name in loaded_data["service_principals"].keys() - if name not in loaded_data["subscriptions"].keys() - ] - if any(bad_subscriptions): - message = "Bad subscription(s) in service_principals" - log.error("{0}, {1}".format(message, str(bad_subscriptions))) - raise PipelineConfigurationError(message) + PipelineConfiguration.__check_subscriptions_in_subscriptions( + loaded_data, "service_principals" + ) return loaded_data + @staticmethod + def __check_subscriptions_in_subscriptions( + loaded_data: dict, data_name: str + ) -> None: + """Check that subscriptions names are defined in subscriptions.""" + bad_subscriptions = [ + "{0}".format(name) + for name in loaded_data[data_name].keys() + if name not in loaded_data["subscriptions"].keys() + ] + if any(bad_subscriptions): + message = f"{data_name} subscriptions not defined in subscriptions" + log.error("{0}, {1}".format(message, str(bad_subscriptions))) + raise PipelineConfigurationError(message) + @pydantic.root_validator() def check_static_secrets( cls: pydantic.BaseModel, loaded_data: dict ) -> dict: """Validate static secret data.""" if loaded_data["static_secrets"]: - bad_subscriptions = [ - "{0}".format(name) - for name in loaded_data["static_secrets"].keys() - if name not in loaded_data["subscriptions"].keys() - ] - if any(bad_subscriptions): - message = "Bad subscription(s) in static_secrets" - log.error("{0}, {1}".format(message, str(bad_subscriptions))) - raise PipelineConfigurationError(message) + PipelineConfiguration.__check_subscriptions_in_subscriptions( + loaded_data, "static_secrets" + ) return loaded_data @pydantic.root_validator() @@ -401,4 +499,5 @@ def check_frames_puff_map( ) ) raise PipelineConfigurationError(message) + return loaded_data diff --git a/tests/ci/unit_tests/pipeline_config/pipeline/test_frames_puff_map.py b/tests/ci/unit_tests/pipeline_config/pipeline/test_frames_puff_map.py index d8d1189..f2a4c82 100644 --- a/tests/ci/unit_tests/pipeline_config/pipeline/test_frames_puff_map.py +++ b/tests/ci/unit_tests/pipeline_config/pipeline/test_frames_puff_map.py @@ -13,6 +13,7 @@ FramesDefinition, PipelineConfiguration, PuffMapGeneratedFiles, + SubscriptionsDefinition, ) from foodx_devops_tools.pipeline_config.exceptions import ( PipelineConfigurationError, @@ -54,7 +55,7 @@ def test_mismatched_frames_raises1(mock_loads, mock_results): "a1": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a1stp1": "some/path/puff1.json" } } @@ -67,7 +68,7 @@ def test_mismatched_frames_raises1(mock_loads, mock_results): "a2": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a2stp1": "some/path/puff1.json" } } @@ -134,7 +135,7 @@ def test_mismatched_frames_raises2(mock_loads, mock_results): "a1": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a1stp1": "some/path/puff1.json" } } @@ -187,7 +188,7 @@ def test_mismatched_applications_raises1(mock_loads, mock_results): "a1": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a1stp1": "some/path/puff1.json" } } @@ -196,7 +197,7 @@ def test_mismatched_applications_raises1(mock_loads, mock_results): "a2": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a2stp1": "some/path/puff1.json" } } @@ -259,7 +260,7 @@ def test_mismatched_applications_raises2(mock_loads, mock_results): "a1": { "arm_parameters_files": { "r1": { - "sub1": { + "sys1_c1_r1a": { "a1stp1": "some/path/puff1.json" } } @@ -313,7 +314,7 @@ def test_bad_puff_map_release_state_raises(mock_loads, mock_results): "a1": { "arm_parameters_files": { "bad_state": { - "sub1": { + "sys1_c1_r1a": { "a1stp1": "some/path/puff1.json" } } @@ -367,11 +368,14 @@ def test_bad_puff_map_subscription_raises(mock_loads, mock_results): "r1": { "bad_sub": { "a1stp1": "some/path/puff1.json" - } - } - } + }, + "sys1_c1_r1a": { + "a1stp1": "some/path/puff1.json" + }, + }, + }, }, - } + }, }, }, }, diff --git a/tests/ci/unit_tests/pipeline_config/pipeline/test_pipeline.py b/tests/ci/unit_tests/pipeline_config/pipeline/test_pipeline.py index c3e0896..852bacb 100644 --- a/tests/ci/unit_tests/pipeline_config/pipeline/test_pipeline.py +++ b/tests/ci/unit_tests/pipeline_config/pipeline/test_pipeline.py @@ -20,6 +20,7 @@ DeploymentsDefinition, PipelineConfiguration, PipelineConfigurationPaths, + PuffMapGeneratedFiles, ServicePrincipals, StaticSecrets, SubscriptionsDefinition, @@ -174,35 +175,6 @@ def test_bad_deployment_systems_raises(self, mock_loads, mock_results): ): PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) - def test_bad_deployment_subscription_raises(self, mock_loads, mock_results): - mock_results.deployments = DeploymentsDefinition.parse_obj( - { - "deployments": { - "deployment_tuples": { - "sys1-c1-r1": { - "subscriptions": { - "bad_name": { - "locations": [ - {"primary": "l1"}, - {"primary": "l2"}, - ], - "root_fqdn": "some.where", - } - }, - }, - }, - "url_endpoints": ["a", "p"], - } - } - ).deployments - mock_loads(mock_results) - - with pytest.raises( - PipelineConfigurationError, - match=r"Bad subscription in deployment", - ): - PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) - def test_bad_client_release_states_raises(self, mock_loads, mock_results): mock_results.clients = ClientsDefinition.parse_obj( { @@ -252,47 +224,6 @@ def test_bad_subscription_tenant_raises(self, mock_loads, mock_results): ): PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) - def test_bad_service_principals_subscription_raises( - self, mock_loads, mock_results - ): - mock_results.service_principals = ServicePrincipals.parse_obj( - { - "service_principals": { - "bad_name": { - "id": "12345", - "secret": "verysecret", - "name": "some_name", - }, - }, - } - ).service_principals - mock_loads(mock_results) - - with pytest.raises( - PipelineConfigurationError, - match=r"Bad subscription\(s\) in service_principals", - ): - PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) - - def test_bad_secrets_subscription_raises(self, mock_loads, mock_results): - mock_results.static_secrets = StaticSecrets.parse_obj( - { - "static_secrets": { - "bad_name": { - "k1": "k1v", - "k2": "k2v", - }, - }, - } - ).static_secrets - mock_loads(mock_results) - - with pytest.raises( - PipelineConfigurationError, - match=r"Bad subscription\(s\) in static_secrets", - ): - PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) - def test_load_files(self): with split_directories(CLEAN_SPLIT.copy()) as ( client_path, @@ -339,4 +270,130 @@ def test_none_token_raises(self): PipelineConfiguration.from_files(this_paths, None) def test_load_dict(self): - under_test = PipelineConfiguration.parse_obj(MOCK_RESULTS.copy()) + PipelineConfiguration.parse_obj(MOCK_RESULTS.copy()) + + +class TestDeploymentSubscriptions: + def test_deployment_subscription_not_in_subscriptions_raises( + self, mock_loads, mock_results + ): + """Deployment declares a subscription not present in subscriptions.""" + mock_results.deployments = DeploymentsDefinition.parse_obj( + { + "deployments": { + "deployment_tuples": { + "sys1-c1-r1": { + "subscriptions": { + "bad_name": { + "locations": [ + {"primary": "l1"}, + {"primary": "l2"}, + ], + "root_fqdn": "some.where", + } + }, + }, + }, + "url_endpoints": ["a", "p"], + } + } + ).deployments + mock_loads(mock_results) + + with pytest.raises( + PipelineConfigurationError, + match=r"Deployment subscription not defined in subscriptions", + ): + PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) + + def test_deployment_subscription_not_in_service_principals_raises( + self, mock_loads, mock_results + ): + """Deployment declares a subscription not present in service + principals.""" + mock_results.service_principals = ServicePrincipals.parse_obj( + { + "service_principals": { + "inactive_subscription": { + "id": "12345", + "secret": "verysecret", + "name": "sp_name", + }, + }, + } + ).service_principals + mock_loads(mock_results) + + with pytest.raises( + PipelineConfigurationError, + match=r"Deployment subscription not defined in service principals", + ): + PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) + + def test_deployment_subscription_not_in_static_secrets_raises( + self, mock_loads, mock_results + ): + """Deployment declares a subscription not present in static secrets.""" + mock_results.static_secrets = StaticSecrets.parse_obj( + { + "static_secrets": { + "inactive_subscription": {"k1": "k1v"}, + }, + } + ).static_secrets + mock_loads(mock_results) + + with pytest.raises( + PipelineConfigurationError, + match=r"Deployment subscription not defined in static secrets", + ): + PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET) + + def test_deployment_subscription_not_in_puff_map_raises( + self, mock_loads, mock_results + ): + """Deployment declares a subscription not present in puff map.""" + mock_results.subscriptions = SubscriptionsDefinition.parse_obj( + { + "subscriptions": { + "sys1_c1_r1a": { + "ado_service_connection": "some-name", + "azure_id": "abc123", + "tenant": "t1", + }, + "inactive_subscription": { + "ado_service_connection": "some-name", + "azure_id": "abc123", + "tenant": "t1", + }, + }, + } + ).subscriptions + mock_results.puff_map = PuffMapGeneratedFiles.parse_obj( + { + "puff_map": { + "frames": { + "f1": { + "applications": { + "a1": { + "arm_parameters_files": { + "r1": { + "inactive_subscription": { + "a1l1": "some/puff_map/path", + }, + }, + }, + }, + }, + }, + }, + }, + } + ).puff_map + mock_loads(mock_results) + + with pytest.raises( + PipelineConfigurationError, + match=r"Deployment subscription not defined in puff map", + ): + PipelineConfiguration.from_files(MOCK_PATHS, MOCK_SECRET)