Skip to content

Commit 0ff0cee

Browse files
iainelderconnelldave
authored andcommitted
Fix effect on target regions of assuming_session
Change the host account init to pass these tests: * `test_when_no_default_region_then_cove_uses_assuming_session_region` * `test_cove_prefers_assuming_session_region` A side effect of the change is even if the caller doesn't specify a target region then the cove result includes a `Region` key. (PR #79 shows how to avoid that side effect, but it's simpler to accept it.) Set the default region for all tests to allow me to test for that side effect. A real user will always set the region from a profile or an environment variable if one isn't passed explicitly to botocove. The default region forces me to update the `test_regions` module. Without setting a default region, the old test still passed! * Old: `test_when_region_is_unspecified_then_result_has_no_region_key` * New: `test_when_region_is_unspecified_then_result_has_default_region_key` The default region makes a `Region` key appear in tests that check the whole cove output instead of a single feature. Later I'll refactor these tests because it's not clear what feature they cover. The default region is `eu-west-1` and all region arguments use some other value such as `eu-central-1` or `us-east-1`. This convention makes the tests easier to read. Use the `_no_default_region` fixture when you really need to test how cove behaves without the default region. Using the `_org_with_one_member` fixture and the `raise_exception=True` parameter together make the tests easier to write because you can assume the cove output has a single result and no exceptions. If any exception occurs, use `pytest.raises` to detect it. Later I'll refactor more tests to use this calling style. We can use dedicated test modules to cover the behavior of `raise_exception=False` and of an organization with multiple accounts. This paves the way to querying the opt-in regions in #77.
1 parent 10fb482 commit 0ff0cee

9 files changed

+83
-14
lines changed

botocove/cove_host_account.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def __init__(
5959
self.host_account_partition = caller_id["Arn"].split(":")[1]
6060

6161
if regions is None:
62-
self.target_regions = [None]
62+
self.target_regions = [assuming_session.region_name]
6363
else:
6464
self.target_regions = regions
6565

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ flake8-print = ">=5.0.0"
3232
mypy = ">=1.5.1"
3333
pre-commit = ">=3.4.0"
3434
flakeheaven = ">=3.3.0"
35-
moto = {extras = ["organizations", "sts"], version = ">=4.2.5"}
35+
moto = {extras = ["organizations", "sts", "ec2"], version = ">=4.2.5"}
36+
boto3-stubs = {extras = ["ec2"], version = "*"}
3637
pytest-randomly = ">=3.15.0"
3738

3839
# These are the last versions compatible with flake8 4. flakeheaven 3.3.0 is

tests/conftest.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ def _clean_env(monkeypatch: MonkeyPatch) -> None:
2424
monkeypatch.setenv(env_var, "broken_not_real_profile")
2525

2626

27+
@pytest.fixture(autouse=True)
28+
def _default_region(monkeypatch: MonkeyPatch) -> None:
29+
monkeypatch.setenv("AWS_DEFAULT_REGION", "eu-west-1")
30+
31+
32+
@pytest.fixture()
33+
def _no_default_region(monkeypatch: MonkeyPatch) -> None:
34+
monkeypatch.delenv("AWS_DEFAULT_REGION")
35+
36+
2737
@pytest.fixture()
2838
def mock_session() -> Iterator[Session]:
2939
"""Returns a session with mock AWS services."""

tests/test_assuming_session.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import pytest
2+
from boto3.session import Session
3+
from botocore.exceptions import NoRegionError
4+
from moto import mock_ec2
5+
6+
from botocove import cove
7+
8+
# Query the region with different configurations of assuming session and
9+
# environment variables. To make the assertions easier all `cove` calls set
10+
# `raise_exception`. If set the assuming session region and the default region
11+
# are always distinct to be able to assert the source of the query result.
12+
13+
14+
def _query_region(session: Session) -> str:
15+
with mock_ec2():
16+
response = session.client("ec2").describe_availability_zones()
17+
return response["AvailabilityZones"][0]["RegionName"]
18+
19+
20+
@pytest.fixture(autouse=True)
21+
def _org_with_one_member(mock_session: Session) -> None:
22+
org_client = mock_session.client("organizations")
23+
org_client.create_organization(FeatureSet="ALL")
24+
org_client.create_account(Email="account1@aws.com", AccountName="Account 1")
25+
26+
27+
@pytest.mark.usefixtures("_no_default_region")
28+
def test_when_no_assuming_session_and_no_default_region_then_cove_raises_error() -> None: # noqa: 501
29+
with pytest.raises(NoRegionError, match=r"^You must specify a region\.$"):
30+
cove(_query_region, raise_exception=True)()
31+
32+
33+
def test_when_no_assuming_session_then_cove_uses_default_region() -> None:
34+
output = cove(_query_region, raise_exception=True)()
35+
assert output["Results"][0]["Result"] == "eu-west-1"
36+
37+
38+
@pytest.mark.usefixtures("_no_default_region")
39+
def test_when_no_default_region_then_cove_uses_assuming_session_region() -> None:
40+
output = cove(
41+
_query_region,
42+
assuming_session=Session(region_name="eu-central-1"),
43+
raise_exception=True,
44+
)()
45+
assert output["Results"][0]["Result"] == "eu-central-1"
46+
47+
48+
def test_cove_prefers_assuming_session_region() -> None:
49+
output = cove(
50+
_query_region,
51+
assuming_session=Session(region_name="eu-central-1"),
52+
raise_exception=True,
53+
)()
54+
assert output["Results"][0]["Result"] == "eu-central-1"

tests/test_decorator.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ def simple_func(session: CoveSession, arg1: int, arg2: int, arg3: int) -> int:
8686
"Result": 6,
8787
"RoleName": "OrganizationAccountAccessRole",
8888
"RoleSessionName": "OrganizationAccountAccessRole",
89+
"Region": "eu-west-1",
8990
}
9091
]
9192

tests/test_decorator_no_args.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ def simple_func(session: CoveSession) -> str:
3535
"RoleName": "OrganizationAccountAccessRole",
3636
"RoleSessionName": "OrganizationAccountAccessRole",
3737
"Result": "hello",
38+
"Region": "eu-west-1",
3839
}
3940
]
4041
assert cove_output["Results"] == expected
@@ -58,6 +59,7 @@ def simple_func(session: CoveSession, output: str) -> str:
5859
"RoleName": "OrganizationAccountAccessRole",
5960
"RoleSessionName": "OrganizationAccountAccessRole",
6061
"Result": "blue",
62+
"Region": "eu-west-1",
6163
}
6264
]
6365
assert cove_output["Results"] == expected
@@ -86,6 +88,7 @@ def simple_func(
8688
"RoleName": "OrganizationAccountAccessRole",
8789
"RoleSessionName": "OrganizationAccountAccessRole",
8890
"Result": ("blue", "circle", "11:11"),
91+
"Region": "eu-west-1",
8992
}
9093
]
9194
assert cove_output == expected

tests/test_exceptions.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ def simple_func(session: CoveSession) -> None:
5656
"Status": "ACTIVE",
5757
"RoleSessionName": "OrganizationAccountAccessRole",
5858
"ExceptionDetails": repr(Exception("oh no")),
59+
"Region": "eu-west-1",
5960
}
6061

6162
# Compare repr of exceptions

tests/test_regions.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from tests.moto_mock_org.moto_models import SmallOrg
66

77

8-
def test_when_region_is_unspecified_then_result_has_no_region_key(
8+
def test_when_region_is_unspecified_then_result_has_default_region_key(
99
mock_small_org: SmallOrg,
1010
) -> None:
1111
@cove()
@@ -15,7 +15,7 @@ def do_nothing(session: Session) -> None:
1515
output = do_nothing()
1616
assert output["Results"]
1717
for result in output["Results"]:
18-
assert "Region" not in result
18+
assert result["Region"] == "eu-west-1"
1919

2020

2121
def test_when_region_is_unspecified_then_output_has_one_result_per_account(
@@ -26,20 +26,16 @@ def do_nothing(session: Session) -> None:
2626
pass
2727

2828
output = do_nothing()
29-
print(output["Results"])
30-
print(len(output["Results"]))
31-
print(_count_member_accounts(mock_session))
32-
print(mock_small_org.all_accounts)
3329
assert len(output["Results"]) == _count_member_accounts(mock_session)
3430

3531

3632
def test_when_region_is_str_then_raises_type_error(mock_small_org: SmallOrg) -> None:
37-
@cove(regions="eu-west-1") # type: ignore[arg-type]
33+
@cove(regions="eu-central-1") # type: ignore[arg-type]
3834
def do_nothing() -> None:
3935
pass
4036

4137
with pytest.raises(
42-
TypeError, match=r"regions must be a list of str\. Got str 'eu-west-1'\."
38+
TypeError, match=r"regions must be a list of str\. Got str 'eu-central-1'\."
4339
):
4440
do_nothing()
4541

@@ -58,28 +54,28 @@ def do_nothing() -> None:
5854
def test_when_any_region_is_passed_then_result_has_region_key(
5955
mock_small_org: SmallOrg,
6056
) -> None:
61-
@cove(regions=["eu-west-1"])
57+
@cove(regions=["eu-central-1"])
6258
def do_nothing(session: Session) -> None:
6359
pass
6460

6561
output = do_nothing()
6662
assert output["Results"]
6763
for result in output["Results"]:
68-
assert result["Region"] == "eu-west-1"
64+
assert result["Region"] == "eu-central-1"
6965

7066

7167
def test_when_two_regions_are_passed_then_output_has_one_result_per_account_per_region(
7268
mock_session: Session, mock_small_org: SmallOrg
7369
) -> None:
74-
@cove(regions=["eu-west-1", "us-east-1"])
70+
@cove(regions=["eu-central-1", "us-east-1"])
7571
def do_nothing(session: Session) -> None:
7672
pass
7773

7874
output = do_nothing()
7975

8076
number_of_member_accounts = _count_member_accounts(mock_session)
8177

82-
for region in ["eu-west-1", "us-east-1"]:
78+
for region in ["eu-central-1", "us-east-1"]:
8379
number_of_results_per_region = sum(
8480
1 for result in output["Results"] if result["Region"] == region
8581
)

tests/test_session.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ def simple_func(session: CoveSession, a_string: str) -> str:
3737
"Result": "test-string",
3838
"RoleName": "OrganizationAccountAccessRole",
3939
"RoleSessionName": "OrganizationAccountAccessRole",
40+
"Region": "eu-west-1",
4041
}
4142
]
4243
assert cove_output["Results"] == expected
@@ -66,6 +67,7 @@ def simple_func(session: CoveSession, a_string: str) -> str:
6667
"RoleName": "OrganizationAccountAccessRole",
6768
"RoleSessionName": "OrganizationAccountAccessRole",
6869
"Policy": session_policy,
70+
"Region": "eu-west-1",
6971
}
7072
]
7173
assert cove_output["Results"] == expected
@@ -97,6 +99,7 @@ def simple_func(session: CoveSession, a_string: str) -> str:
9799
"RoleName": "OrganizationAccountAccessRole",
98100
"RoleSessionName": "OrganizationAccountAccessRole",
99101
"PolicyArns": session_policy_arns,
102+
"Region": "eu-west-1",
100103
}
101104
]
102105
assert cove_output["Results"] == expected

0 commit comments

Comments
 (0)