Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/5.0.2 #760

Merged
merged 14 commits into from
Dec 10, 2024
Merged
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a Ch
- Update session manager to pass toolchain role region to sts

### Fixes
- allow nested modules in archives pulled over HTTPS (ref issue/749)
- 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
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ markers = [
"mgmt_metadata_support: marks all `mgmt_metadata_support` tests",
"mgmt_build_info: marks all `mgmt_build_info` tests",
"mgmt_git_support: marks all `mgmt_git_support` tests",
"mgmt_git_release: marks all `mgmt_git_release` tests",
"mgmt_archive_support: marks all `mgmt_archive_support` tests",
"service: marks all `services` tests",
"projectpolicy: marks all `projectpolicy` tests",
Expand Down
38 changes: 32 additions & 6 deletions seedfarmer/mgmt/archive_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os.path
import pathlib
import re
import shutil
import tarfile
from typing import Optional, Tuple
from urllib.parse import parse_qs, urlparse
Expand Down Expand Up @@ -71,13 +72,31 @@ def _download_archive(archive_url: str, secret_name: Optional[str]) -> Response:
def _extract_archive(archive_name: str, extracted_dir_path: str) -> str:
if archive_name.endswith(".tar.gz"):
with tarfile.open(archive_name, "r:gz") as tar_file:
embedded_dir = os.path.commonprefix(tar_file.getnames())
tar_file.extractall(extracted_dir_path)
all_members = tar_file.getmembers()
top_level_dirs = set(member.name.split("/")[0] for member in all_members if "/" in member.name)
if len(top_level_dirs) > 1:
raise InvalidConfigurationError(
f"the archive {archive_name} can only have one directory at the root and no files"
)
elif len(top_level_dirs) == 1:
embedded_dir = top_level_dirs.pop()
else:
embedded_dir = ""
tar_file.extractall(extracted_dir_path, members=all_members)
else:
with ZipFile(archive_name, "r") as zip_file:
embedded_dir = os.path.commonprefix(zip_file.namelist())
zip_file.extractall(extracted_dir_path)

all_files = zip_file.namelist()
top_level_dirs = set(name.split("/")[0] for name in all_files if "/" in name)
if len(top_level_dirs) > 1:
raise InvalidConfigurationError(
f"the archive {archive_name} can only have one directory at the root and no files"
)
elif len(top_level_dirs) == 1:
embedded_dir = top_level_dirs.pop()
else:
embedded_dir = ""
for file in all_files:
zip_file.extract(file, path=extracted_dir_path)
return embedded_dir


Expand All @@ -89,10 +108,17 @@ def _process_archive(archive_name: str, response: Response, extracted_dir: str)
archive_file.write(response.content)

extracted_dir_path = os.path.join(parent_dir, extracted_dir)
_ = _extract_archive(archive_name, extracted_dir_path)
embedded_dir = _extract_archive(archive_name, extracted_dir_path)

os.remove(archive_name)

if embedded_dir:
file_names = os.listdir(os.path.join(extracted_dir_path, embedded_dir))
for file_name in file_names:
shutil.move(os.path.join(extracted_dir_path, embedded_dir, file_name), extracted_dir_path)

os.rmdir(os.path.join(extracted_dir_path, embedded_dir))

return extracted_dir_path


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
Loading
Loading