Skip to content

Commit

Permalink
Merge pull request #759 from dgraeber/fix/bounderyperms
Browse files Browse the repository at this point in the history
correct the reference to account for boundary policies
  • Loading branch information
dgraeber authored Dec 10, 2024
2 parents ca53f44 + f8ee029 commit 344ec66
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion seedfarmer/models/manifests/_deployment_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 1 addition & 1 deletion seedfarmer/models/transfer/_module_deploy_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 4 additions & 1 deletion test/unit-test/mock_data/mock_deployment_manifest_huge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion test/unit-test/test_commands_deployment.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/unit-test/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 344ec66

Please sign in to comment.