From 62af053f2a6327d95391152fd3b52e15f379e86d Mon Sep 17 00:00:00 2001 From: Derek Graeber Date: Mon, 9 Dec 2024 13:44:53 -0500 Subject: [PATCH 1/3] correct the reference to account for boundary policies --- CHANGELOG.md | 1 + seedfarmer/models/manifests/_deployment_manifest.py | 2 +- seedfarmer/models/transfer/_module_deploy_object.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba95b4..f1df67c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a Ch ### Fixes - correct archive extraction when there is only one nested path (issue 749) +- force proper resolution of boundary permissions ## v5.0.0 (2024-08-16) diff --git a/seedfarmer/models/manifests/_deployment_manifest.py b/seedfarmer/models/manifests/_deployment_manifest.py index bf6e2ce..c70af62 100644 --- a/seedfarmer/models/manifests/_deployment_manifest.py +++ b/seedfarmer/models/manifests/_deployment_manifest.py @@ -448,7 +448,7 @@ def get_region_seedfarmer_bucket( def get_permission_boundary_arn(self, target_account: str, target_region: str) -> Optional[str]: permissions_boundary_name = self.get_parameter_value( "permissionsBoundaryName", - account_alias=target_account, + account_id=target_account, region=target_region, ) return ( diff --git a/seedfarmer/models/transfer/_module_deploy_object.py b/seedfarmer/models/transfer/_module_deploy_object.py index c030c95..a77d0f5 100644 --- a/seedfarmer/models/transfer/_module_deploy_object.py +++ b/seedfarmer/models/transfer/_module_deploy_object.py @@ -26,7 +26,7 @@ def __init__(self, **kwargs: Any) -> None: _module = cast(ModuleManifest, self.deployment_manifest.get_module(self.group_name, self.module_name)) pba = self.deployment_manifest.get_permission_boundary_arn( - target_account=cast(str, _module.target_account), + target_account=cast(str, _module.get_target_account_id()), target_region=cast(str, _module.target_region), ) codebuild_image = self.deployment_manifest.get_region_codebuild_image( From 7e2586eb5217c6cc93a1ed91b888a5c70ce53f03 Mon Sep 17 00:00:00 2001 From: kukushking Date: Mon, 9 Dec 2024 19:11:34 +0000 Subject: [PATCH 2/3] add test case --- test/unit-test/test_models.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit-test/test_models.py b/test/unit-test/test_models.py index 19b4f2d..b7e9b74 100644 --- a/test/unit-test/test_models.py +++ b/test/unit-test/test_models.py @@ -121,6 +121,12 @@ def test_deployment_manifest_get_parameter_with_defaults(): with pytest.raises(seedfarmer.errors.InvalidManifestError): manifest.get_target_account_mapping() + assert manifest.get_parameter_value("permissionsBoundaryName") == "policyName" + assert ( + manifest.get_permission_boundary_arn(target_account="000000000000", target_region="us-west-2") + == "arn:aws:iam::000000000000:policy/policyName" + ) + @pytest.mark.models @pytest.mark.models_deployment_manifest From f8ee0294a88769ba966c59d296b35be2e1de77bf Mon Sep 17 00:00:00 2001 From: kukushking Date: Mon, 9 Dec 2024 19:28:21 +0000 Subject: [PATCH 3/3] more test cases --- .../mock_data/mock_deployment_manifest_huge.py | 5 ++++- test/unit-test/test_commands_deployment.py | 13 ++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/test/unit-test/mock_data/mock_deployment_manifest_huge.py b/test/unit-test/mock_data/mock_deployment_manifest_huge.py index 5e92a5a..2d65bb0 100644 --- a/test/unit-test/mock_data/mock_deployment_manifest_huge.py +++ b/test/unit-test/mock_data/mock_deployment_manifest_huge.py @@ -241,7 +241,10 @@ "alias": "primary", "account_id": "123456789012", "default": True, - "parameters_global": {"dockerCredentialsSecret": "aws-addf-docker-credentials"}, + "parameters_global": { + "dockerCredentialsSecret": "aws-addf-docker-credentials", + "permissionsBoundaryName": "boundary", + }, "region_mappings": [ { "region": "us-east-1", diff --git a/test/unit-test/test_commands_deployment.py b/test/unit-test/test_commands_deployment.py index c8a2ae9..a66cad8 100644 --- a/test/unit-test/test_commands_deployment.py +++ b/test/unit-test/test_commands_deployment.py @@ -1,5 +1,6 @@ import json import os +from unittest.mock import ANY import mock_data.mock_deployment_manifest_for_destroy as mock_deployment_manifest_for_destroy import mock_data.mock_deployment_manifest_huge as mock_deployment_manifest_huge @@ -293,7 +294,9 @@ def test_create_module_deployment_role(session_manager, mocker): "seedfarmer.commands._deployment_commands.get_generic_module_deployment_role_name", return_value="generic-module-deployment-role", ) - mocker.patch("seedfarmer.commands._deployment_commands.create_module_deployment_role", return_value=None) + mock_create_module_deployment_role = mocker.patch( + "seedfarmer.commands._deployment_commands.create_module_deployment_role", return_value=None + ) dep = DeploymentManifest(**mock_deployment_manifest_huge.deployment_manifest) dep.validate_and_set_module_defaults() @@ -304,6 +307,14 @@ def test_create_module_deployment_role(session_manager, mocker): deployment_manifest=dep, ) + mock_create_module_deployment_role.assert_called_once_with( + role_name="generic-module-deployment-role", + deployment_name="mlops", + permissions_boundary_arn="arn:aws:iam::123456789012:policy/boundary", + docker_credentials_secret=None, + session=ANY, + ) + @pytest.mark.commands @pytest.mark.commands_deployment