Skip to content

Commit 8e8d067

Browse files
author
Mike Grima
committedJun 7, 2023
This fixes nested OU bug
- Fixes #24 - Corrects an issue with nested OU names not being resolved - Updated docs to reflect
1 parent fa73c55 commit 8e8d067

File tree

9 files changed

+523
-362
lines changed

9 files changed

+523
-362
lines changed
 

‎deploy/starfleet-execution-role-org-management.yml

+5-15
Original file line numberDiff line numberDiff line change
@@ -124,24 +124,14 @@ Resources:
124124
aws:PrincipalOrgId:
125125
- !Ref OrganizationId
126126
Policies:
127-
- PolicyName: AuthorizeOrganizationsList
127+
- PolicyName: OrganizationsReadOnly
128128
PolicyDocument:
129129
Version: '2012-10-17'
130130
Statement:
131-
- Sid: AuthorizeOrgListRestricted
132-
Effect: Allow
131+
- Effect: Allow
133132
Action:
134-
- organizations:ListOrganizationalUnitsForParent
135-
- organizations:ListParents
136-
- organizations:ListTagsForResource
137-
Resource:
138-
- !Sub 'arn:${Partition}:organizations::${AWS::AccountId}:account/${OrganizationId}/*'
139-
- !Sub 'arn:${Partition}:organizations::${AWS::AccountId}:ou/${OrganizationId}/ou-*'
140-
- !Sub 'arn:${Partition}:organizations::${AWS::AccountId}:root/${OrganizationId}/r-*'
141-
- Sid: AuthorizeOrgListAccounts
142-
Effect: Allow
143-
Action:
144-
- organizations:ListAccounts
133+
- organizations:Describe*
134+
- organizations:List*
145135
Resource:
146136
- '*'
147137

@@ -212,7 +202,7 @@ Outputs:
212202
Description: The 12-digit AWS account ID of the Starfleet management account.
213203
Value: !Ref StarfleetAccountId
214204
StarfleetAccountIndexRoleArn:
215-
Description: >-
205+
Description: >-
216206
The Amazon Resource Name (ARN) of the Starfleet Account Index Generator
217207
execution role.
218208
Value: !GetAtt AccountIndexExecutionRole.Arn

‎mkdocs/images/StarfleetArchitecture.svg

+1-1
Loading

‎mkdocs/installation/IAM.md

+7-2
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,20 @@ Resources:
108108
If you do decide to go with the CloudFormation StackSets route, you need to keep in mind that StackSets will _NOT_ deploy to the Organization Root account. If you do choose to use StackSets, you will need to _manually_ create an IAM role in the organization root account that has the exact same permissions as what is documented in the StackSet YAML above.
109109
110110
!!! Note
111-
If you leverage the `AccountIndexGeneratorShip` worker ship for your AWS account inventory (recommended), you will need to make sure that the Starfleet IAM roles in the Organization Root has the following permissions:
111+
If you leverage the `AccountIndexGeneratorShip` worker ship for your AWS account inventory (recommended), you will need to make sure that the Starfleet IAM roles in the Organization Root account has the following permissions (in addition to the permissions you grant to the other worker roles):
112112

113113
```json
114114
{
115115
"Version": "2012-10-17",
116116
"Statement": [
117117
{
118118
"Effect": "Allow",
119-
"Action": "organizations:List*",
119+
"Action": [
120+
[
121+
"organizations:Describe*",
122+
"organizations:List*"
123+
]
124+
]
120125
"Resource": "*"
121126
}
122127
]

‎pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ authors = [
88
{name = "Gemini", email = "careers@gemini.com"},
99
{name = "Mike Grima", email = "michael.grima@gemini.com"},
1010
]
11-
version = "0.0.1"
11+
version = "1.0.0"
1212
dynamic = ["dependencies"]
1313

1414
[tool.setuptools]

‎src/starfleet/worker_ships/plugins/account_index_generator/ship.py

+19-10
Original file line numberDiff line numberDiff line change
@@ -60,24 +60,28 @@ def execute(self, commit: bool = False) -> None:
6060
config = STARFLEET_CONFIGURATION.config[self.worker_ship_name]
6161
LOGGER.info(f"[📡] Reaching out to the Orgs API to get the list of AWS accounts in account: {config['OrgAccountId']}...")
6262
all_accounts = list_accounts( # pylint: disable=no-value-for-parameter
63-
account_number=config["OrgAccountId"], assume_role=config["OrgAccountAssumeRole"]
63+
account_number=config["OrgAccountId"], assume_role=config["OrgAccountAssumeRole"], region="us-east-1"
6464
)
6565
account_map = get_account_map(all_accounts) # Reformat the data
6666

67-
# Fetch the list of org OUs:
68-
LOGGER.info("[📡] Reaching out to the Orgs API to get the list of all OUs in the org...")
67+
# Fetch the list of org OUs for root:
68+
LOGGER.info("[📡] Reaching out to the Orgs API to get the list of all OUs under the ROOT OU...")
6969
all_ous = list_organizational_units_for_parent( # pylint: disable=no-value-for-parameter
70-
ParentId=config["OrgRootId"], account_number=config["OrgAccountId"], assume_role=config["OrgAccountAssumeRole"]
70+
ParentId=config["OrgRootId"], account_number=config["OrgAccountId"], assume_role=config["OrgAccountAssumeRole"], region="us-east-1"
7171
)
72-
ou_map = {ou["Id"]: ou["Name"] for ou in all_ous} # noqa # Reformat the data into a nice map
73-
ou_map[config["OrgRootId"]] = "ROOT" # Add the root in
72+
73+
# The resolved parents map: This is used to provide us with the parents for each account eventually pointing to ROOT.
74+
# At this point, we know that the top level OUs map to root, so we build that mapping list now so we don't have to resolve this later with lots of
75+
# unnecessary recursive API calls:
76+
root = {"Id": config["OrgRootId"], "Name": "ROOT", "Type": "ROOT"}
77+
resolved_parent_map = {ou["Id"]: [{"Id": ou["Id"], "Name": ou["Name"], "Type": "ORGANIZATIONAL_UNIT"}, root] for ou in all_ous}
78+
resolved_parent_map[config["OrgRootId"]] = [root] # Any account that has a parent of ROOT will just have ROOT as the parent.
7479

7580
# Fetch the tags and enabled regions:
7681
LOGGER.info("[🚚] Fetching tags and enabled regions for each account...")
7782
fetch_additional_details(
7883
account_map,
79-
ou_map,
80-
config["OrgRootId"],
84+
resolved_parent_map,
8185
config["OrgAccountId"],
8286
config["OrgAccountAssumeRole"],
8387
config["DescribeRegionsAssumeRole"],
@@ -93,13 +97,18 @@ def execute(self, commit: bool = False) -> None:
9397
Bucket=self.payload["account_inventory_bucket"],
9498
Key=self.payload["inventory_object_prefix"],
9599
ACL="bucket-owner-full-control",
96-
Body=json.dumps(dump_accounts, indent=4),
100+
Body=json.dumps(dump_accounts, indent=4, sort_keys=True),
97101
ContentType="application/json",
98102
)
99103

100104
# If we are not committing, then just output the raw data out:
101105
else:
102-
LOGGER.info(f"[🍣] Raw Inventory:\n{json.dumps(account_map, indent=4)}")
106+
LOGGER.info(f"[🍣] Raw Inventory:\n{json.dumps(account_map, sort_keys=True, indent=4)}")
107+
108+
# If you need to update the index JSON for unit tests, then run the tests and uncomment the code below. That will generate a new index JSON.
109+
# Simply copy and paste this `generatedIndex.json` file to `tests/starfleet_included_plugins/account_index_generator/generatedIndex.json`:
110+
# with open("generatedIndex.json", "w") as file:
111+
# file.write(json.dumps({"accounts": account_map, "generated": datetime.utcnow().replace(tzinfo=None, microsecond=0).isoformat() + "Z"}, indent=4, sort_keys=True))
103112

104113

105114
@click.group()

‎src/starfleet/worker_ships/plugins/account_index_generator/utils.py

+76-23
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ def list_accounts(client: BaseClient, **kwargs) -> Dict[str, Any]:
3636
@paginated("OrganizationalUnits", request_pagination_marker="NextToken", response_pagination_marker="NextToken")
3737
def list_organizational_units_for_parent(client: BaseClient, **kwargs) -> Dict[str, Any]:
3838
"""This will list all the OUs for the given parent -- calls are wrapped by CloudAux"""
39-
return client.list_organizational_units_for_parent(**kwargs) # pragma: no cover
39+
return client.list_organizational_units_for_parent(**kwargs)
40+
41+
42+
@sts_conn("organizations")
43+
def describe_organizational_unit(client: BaseClient, **kwargs) -> Dict[str, Any]:
44+
"""This will describe the OU given the OU ID. This is used to extract the name of the OU -- calls are wrapped by CloudAux"""
45+
return client.describe_organizational_unit(**kwargs)
4046

4147

4248
@paginated("Tags", request_pagination_marker="NextToken", response_pagination_marker="NextToken")
@@ -45,9 +51,10 @@ def list_tags_for_resource(client: BaseClient, **kwargs) -> Dict[str, Any]:
4551
return client.list_tags_for_resource(**kwargs)
4652

4753

54+
@sts_conn("organizations")
4855
@paginated("Parents", request_pagination_marker="NextToken", response_pagination_marker="NextToken")
4956
def list_parents(client: BaseClient, **kwargs) -> Dict[str, Any]:
50-
"""This will query organizations to list all the parent OUs and Root for a given account -- calls are wrapped by CloudAux"""
57+
"""This will query organizations to list the immediate parent a given account or OU -- calls are wrapped by CloudAux"""
5158
return client.list_parents(**kwargs)
5259

5360

@@ -65,8 +72,8 @@ def get_account_map(account_list: List[Dict[str, Any]]) -> Dict[str, Dict[str, A
6572

6673

6774
@retry(tries=3, jitter=(0, 3), delay=1, backoff=2, max_delay=3, logger=LOGGER)
68-
def fetch_tags_and_parents(account_id: str, creds: Dict[str, str], deployment_region: str) -> Tuple[str, Dict[str, str], List[Dict[str, str]]]:
69-
"""Fetch the tags and parent OUs for each individual account"""
75+
def fetch_tags_and_parent(account_id: str, creds: Dict[str, str], deployment_region: str) -> Tuple[str, Dict[str, str], List[Dict[str, str]]]:
76+
"""Fetch the tags and the immediate parent OU for each individual account"""
7077
LOGGER.debug(f"[🏷️] Fetching tags for account id: {account_id}...")
7178
client = boto3.client(
7279
"organizations",
@@ -79,9 +86,9 @@ def fetch_tags_and_parents(account_id: str, creds: Dict[str, str], deployment_re
7986
extracted_tags = {tag["Key"]: tag["Value"] for tag in returned_tags}
8087

8188
LOGGER.debug(f"[👪] Fetching parents for account id: {account_id}...")
82-
returned_parents = list_parents(client, ChildId=account_id)
89+
returned_parent = list_parents(force_client=client, ChildId=account_id) # pylint: disable=no-value-for-parameter
8390

84-
return account_id, extracted_tags, returned_parents
91+
return account_id, extracted_tags, returned_parent
8592

8693

8794
@retry(tries=3, jitter=(0, 3), delay=1, backoff=2, max_delay=3, logger=LOGGER)
@@ -107,6 +114,54 @@ def fetch_regions(account_id: str, assume_role: str, deployment_region: str) ->
107114
return account_id, enabled_regions
108115

109116

117+
@retry(tries=3, jitter=(0, 3), delay=1, backoff=2, max_delay=3, logger=LOGGER)
118+
def resolve_parents(ou_dict: Dict[str, Any], resolved_parents: Dict[str, List[Dict[str, str]]], org_account_id: str, assume_role: str) -> List[Dict[str, Any]]:
119+
"""
120+
This will resolve the parents for a given account's parent OU. the `resolved_parents` map is a dictionary that contains a list of the parents for the given OU ID passed in.
121+
If we have an OU ID that is not resolved, then we need to build the parent tree for it. This involves recursively calling the list_parents call so that we can
122+
resolve all parents up to one that we have already resolved parents for.
123+
124+
Before this function runs, the `resolved_parents` parameter should at a minimum have the ROOT resolved. It should also have all the ROOT's immediate children OUs resolved
125+
as well.
126+
127+
This function is required to address issue #24. Orgs does not provide an API to just get the Org OU tree. When this worker ship first runs, we list the OUs that are under
128+
the ROOT, but that doesn't list all the nested OUs. This addresses that by fetching and updating the map as missing OUs are encountered. Very annoying but is required. :/
129+
"""
130+
ou_id = ou_dict["Id"]
131+
if parents := resolved_parents.get(ou_id):
132+
return parents
133+
134+
# We have not yet resolved the parent OUs and we need to resolve it:
135+
LOGGER.debug(f"[🌲] Need to query the Orgs API to fetch the parents for OU ID: {ou_dict['Id']}...")
136+
137+
# We also need to get the name of this OU because we haven't seen it yet:
138+
ou_response = describe_organizational_unit( # pylint: disable=no-value-for-parameter
139+
OrganizationalUnitId=ou_dict["Id"], account_number=org_account_id, assume_role=assume_role, region="us-east-1"
140+
)["OrganizationalUnit"]
141+
resolved_ou = {"Name": ou_response["Name"], "Id": ou_id, "Type": "ORGANIZATIONAL_UNIT"}
142+
LOGGER.debug(f"[🆔] OU ID: {ou_id} has name: {ou_response['Name']}.")
143+
144+
# Append this OU as the first parent in the list:
145+
resolved_parents[ou_id] = [resolved_ou]
146+
147+
# Unfortunately, when we list_parents, we only get back the immediate parent. This sucks. So we need to have the following logic:
148+
# 1. Get the immediate parent that is returned (it should only be 1) -- but it's a list that's returned.
149+
# 2. Call this function again (recurse) with the parent provided. Keep going until we find a parent we know about (we should always map to ROOT at a minimum).
150+
# 3. As we continue, we are traversing the Org tree. As we go along we continue to update the parents and grandparents parents
151+
# 4. After the tree traversal is completed, we update the current OU list with the list of resolved parents.
152+
# 5. We now have the full parents list for an Account with the passed in OU ID. In future calls regarding this OU ID, it will reside in the resolved_parents map
153+
# and will simply be returned without additional API calls necessary!
154+
LOGGER.debug(f"[👪] Fetching parents for OU ID: {ou_id}/{resolved_ou['Name']}...")
155+
returned_parents = list_parents( # pylint: disable=no-value-for-parameter
156+
ChildId=ou_id, account_number=org_account_id, assume_role=assume_role, region="us-east-1"
157+
)
158+
# It only returns exactly 1 parent back out. Now, resolve the parent's parents...
159+
if returned_parents:
160+
resolved_parents[ou_id] += resolve_parents(returned_parents[0], resolved_parents, org_account_id, assume_role)
161+
162+
return resolved_parents[ou_id]
163+
164+
110165
TagAndParentResults = List[Tuple[str, Dict[str, str], List[Dict[str, str]]]]
111166
RegionsResults = List[Tuple[str, List[str]]]
112167

@@ -128,7 +183,7 @@ async def fetch_additional_async(
128183
regions_tasks = []
129184

130185
for account_id in all_accounts.keys():
131-
tag_and_parent_tasks.append(loop.run_in_executor(executor, fetch_tags_and_parents, account_id, creds, deployment_region))
186+
tag_and_parent_tasks.append(loop.run_in_executor(executor, fetch_tags_and_parent, account_id, creds, deployment_region))
132187
regions_tasks.append(loop.run_in_executor(executor, fetch_regions, account_id, region_role, deployment_region))
133188

134189
try:
@@ -143,14 +198,21 @@ async def fetch_additional_async(
143198

144199

145200
def fetch_additional_details(
146-
all_accounts: Dict[str, Any], all_ous: Dict[str, str], root_id: str, org_id: str, org_role: str, region_role: str, deployment_region: str
201+
all_accounts: Dict[str, Any],
202+
resolved_parent_map: Dict[str, List[Dict[str, str]]],
203+
org_account_id: str,
204+
org_role: str,
205+
region_role: str,
206+
deployment_region: str,
147207
) -> None:
148-
"""This is the function that will go out and fetch all the additional details asynchronously with multiple spun processes. This wraps everything
149-
in the event loop."""
208+
"""
209+
This is the function that will go out and fetch all the additional details asynchronously with multiple spun processes. This wraps everything
210+
in the event loop.
211+
"""
150212
loop = asyncio.new_event_loop()
151213
try:
152214
tag_and_parent_results, region_results = loop.run_until_complete(
153-
fetch_additional_async(loop, all_accounts, org_id, org_role, region_role, deployment_region)
215+
fetch_additional_async(loop, all_accounts, org_account_id, org_role, region_role, deployment_region)
154216
)
155217
finally:
156218
loop.close()
@@ -159,20 +221,11 @@ def fetch_additional_details(
159221
for result in tag_and_parent_results:
160222
all_accounts[result[0]]["Tags"] = result[1]
161223

162-
parent_ous = []
163-
has_root = False # If the Root wasn't added in, then we will append it so that there is a full chain up to the Root.
164-
for parent in result[2]:
165-
if parent["Type"] == "ROOT":
166-
has_root = True
167-
168-
parent_ous.append({"Id": parent["Id"], "Type": parent["Type"], "Name": all_ous[parent["Id"]]})
169-
170-
# If Root wasn't there, then append it:
171-
if not has_root:
172-
parent_ous.append({"Id": root_id, "Type": "ROOT", "Name": "ROOT"})
173-
224+
# Resolve all the parents:
225+
parent_ous = resolve_parents(result[2][0], resolved_parent_map, org_account_id, org_role)
174226
all_accounts[result[0]]["Parents"] = parent_ous
175227

228+
# Merge in the regions:
176229
for result in region_results:
177230
all_accounts[result[0]]["Regions"] = result[1]
178231

‎tests/starfleet_included_plugins/account_index_generator/conftest.py

+50-21
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
# pylint: disable=redefined-outer-name,unused-argument,duplicate-code
1111
import json
1212
from datetime import datetime
13-
from typing import Any, Dict, Generator, List, Optional
13+
from typing import Any, Dict, Generator, Optional
1414
from unittest import mock
1515
from unittest.mock import MagicMock
1616

@@ -103,35 +103,69 @@ def account_generator(paginator: Optional[str] = None) -> Dict[str, Any]:
103103

104104

105105
@pytest.fixture
106-
def mock_list_account() -> MagicMock:
107-
"""This will mock out the Organizations list_account call so that it will return the generated account list."""
106+
def mock_boto3_sts_orgs_calls() -> Generator[MagicMock, None, None]:
107+
"""This will mock out the Organizations STS related calls so that it will return the proper responses back out"""
108108

109109
def list_accounts(**kwargs) -> Dict[str, Any]:
110110
"""This is the mocked out list_accounts call."""
111111
return account_generator(paginator=kwargs.pop("NextToken", None))
112112

113+
unit_map = {
114+
"ou-1234-5678910": "SomeOU",
115+
"ou-1234-5678911": "SomeNestedOU",
116+
"ou-1234-5678912": "SomeNestedNestedOU",
117+
}
118+
119+
def list_organizational_units_for_parent(*args, **kwargs) -> Dict[str, Any]: # noqa
120+
"""This is the mocked out function that will just list all the OUs that are not Root back out."""
121+
return {
122+
"OrganizationalUnits": [{"Id": "ou-1234-5678910", "Arn": "arn:aws:organizations::000000000020:ou/o-000000000020/ou-1234-5678910", "Name": "SomeOU"}]
123+
}
124+
125+
def fetch_parent_ous_in_resolve(*args, **kwargs) -> Dict[str, Any]: # noqa
126+
"""This is the mocked out function that will return the parent OUs in the resolve function. This is only ever called"""
127+
if kwargs["ChildId"] == "ou-1234-5678912":
128+
return {"Parents": [{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT"}]}
129+
130+
return {"Parents": [{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"}]}
131+
132+
def describe_organizational_unit(*args, **kwargs) -> Dict[str, Any]: # noqa
133+
"""The mocked out call to describing organizational units"""
134+
return {
135+
"OrganizationalUnit": {
136+
"Id": kwargs["OrganizationalUnitId"],
137+
"Arn": f"arn:aws:organizations::000000000020:ou/o-000000000020/{kwargs['OrganizationalUnitId']}",
138+
"Name": unit_map[kwargs["OrganizationalUnitId"]],
139+
}
140+
}
141+
113142
# Make the "returned" magic mock. This is the mocked out boto3 client, which is returned when the boto3_cached_conn function is instantiated
114143
mocked_boto_client = MagicMock()
115144
mocked_boto_client.list_accounts = MagicMock(side_effect=list_accounts)
145+
mocked_boto_client.list_organizational_units_for_parent = MagicMock(side_effect=list_organizational_units_for_parent)
146+
mocked_boto_client.list_parents = MagicMock(side_effect=fetch_parent_ous_in_resolve)
147+
mocked_boto_client.describe_organizational_unit = MagicMock(side_effect=describe_organizational_unit)
116148

117149
# This is the mocked boto3_cached_conn function itself. It needs to have a return_value to the mocked out client above:
118150
with mock.patch("cloudaux.aws.sts.boto3_cached_conn", return_value=mocked_boto_client) as mocked_cache_conn:
119151
yield mocked_cache_conn
120152

121153

122154
@pytest.fixture
123-
def account_map(mock_list_account: MagicMock) -> Dict[str, Any]:
155+
def account_map(mock_boto3_sts_orgs_calls: MagicMock) -> Dict[str, Any]:
124156
"""The account list from the mocked call."""
125157
from starfleet.worker_ships.plugins.account_index_generator.utils import list_accounts, get_account_map
126158

127159
return get_account_map(list_accounts(account_number="000000000020", assume_role="testing")) # pylint: disable=no-value-for-parameter
128160

129161

130162
@pytest.fixture
131-
def mock_direct_boto_clients() -> MagicMock:
163+
def mock_direct_boto_clients() -> Generator[MagicMock, None, None]:
132164
"""
133165
Mocks out the direct boto3 client creator. The mocked boto3 object has a new client() function that will return the normal boto3 client (gets mocked with moto)
134-
for all services *except* organizations. For organizations, we're making a MagicMock that mocks out the `list_tags_for_resource` call.
166+
for all services *except* organizations. For organizations, we're making a MagicMock that mocks out the `list_tags_for_resource` and `list_parents` call.
167+
168+
Accounts 10 and 11 have nested OU residency.
135169
"""
136170
old_client = boto3.client
137171

@@ -145,6 +179,16 @@ def fetch_parent_ous(*args, **kwargs) -> Dict[str, Any]: # noqa
145179
if kwargs["ChildId"] == "000000000020":
146180
return {"Parents": [{"Id": "r-123456", "Type": "ROOT"}]}
147181

182+
# If account 10 is passed in (000000000010), then return the parent OU (ou-1234-5678912) - Account 10 will live in a nested OU of a nested OU
183+
# (SomeOU --> SomeNestedOU --> SomeNestedNestedOU --> Account10)
184+
if kwargs["ChildId"] == "000000000010":
185+
return {"Parents": [{"Id": "ou-1234-5678912", "Type": "ORGANIZATIONAL_UNIT"}]}
186+
187+
# If account 11 is passed in (000000000011), then return the parent OU (ou-1234-5678911) - Account 11 will live in a nested OU of a nested OU
188+
# (SomeOU --> SomeNestedOU --> Account11)
189+
if kwargs["ChildId"] == "000000000011":
190+
return {"Parents": [{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT"}]}
191+
148192
return {"Parents": [{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"}]}
149193

150194
class MockedBoto3:
@@ -169,21 +213,6 @@ def client(self, service, **kwargs):
169213
yield mocked_boto
170214

171215

172-
@pytest.fixture
173-
def mock_list_parent_ous() -> Generator[None, None, None]:
174-
"""
175-
This very specifically mocks out the boto3 call for listing the parent OUs. Not using Moto for this. The original function is wrapped by CloudAux's STS wrapper,
176-
and it's easier to just mock out what we need vs. test the boto3 stuff itself.
177-
"""
178-
179-
def mocked_func(*args, **kwargs) -> List[Dict[str, Any]]: # noqa
180-
"""This is the mocked out function that will just list all the OUs that are not Root back out."""
181-
return [{"Id": "ou-1234-5678910", "Arn": "arn:aws:organizations::000000000020:ou/o-000000000020/ou-1234-5678910", "Name": "SomeOU"}]
182-
183-
with mock.patch("starfleet.worker_ships.plugins.account_index_generator.ship.list_organizational_units_for_parent", mocked_func):
184-
yield
185-
186-
187216
@pytest.fixture
188217
def lambda_payload(good_payload: Dict[str, Any]) -> Dict[str, Any]:
189218
"""This is the payload from SQS encoded in the Body of the dictionary. Not including the rest of the SQS details."""

‎tests/starfleet_included_plugins/account_index_generator/generatedIndex.json

+294-279
Large diffs are not rendered by default.

‎tests/starfleet_included_plugins/account_index_generator/test_account_indexer_ship.py

+70-10
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def test_payload_schema(good_payload: Dict[str, Any]) -> None:
5555
AccountIndexGeneratorShipPayloadTemplate().load(good_payload)
5656

5757

58-
def test_list_org_accounts(mock_list_account: MagicMock) -> None:
58+
def test_list_org_accounts(mock_boto3_sts_orgs_calls: MagicMock) -> None:
5959
"""This mostly tests that our code will pull out the proper (mocked) results from the AWS API. This also ensures that our fixture is working properly."""
6060
from starfleet.worker_ships.plugins.account_index_generator.utils import list_accounts
6161

@@ -92,9 +92,12 @@ def test_fetch_additional_details(
9292
"""This is a test to ensure that we can fetch all the additional details about"""
9393
from starfleet.worker_ships.plugins.account_index_generator.utils import fetch_additional_details
9494

95-
ous = {"ou-1234-5678910": "SomeOU", "r-123456": "ROOT"}
95+
resolved_parent_map = {
96+
"ou-1234-5678910": [{"Name": "SomeOU", "Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"}, {"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"}],
97+
"r-123456": [{"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"}],
98+
}
9699

97-
fetch_additional_details(account_map, ous, "r-123456", "000000000020", "testing", "testing", "us-east-2")
100+
fetch_additional_details(account_map, resolved_parent_map, "000000000020", "testing", "testing", "us-east-2")
98101

99102
# Verify that everything is there:
100103
tag_value = {f"Key{x}": f"Value{x}" for x in range(1, 4)}
@@ -106,13 +109,50 @@ def test_fetch_additional_details(
106109
assert account["Regions"] == regions
107110

108111
# And the parent OUs:
109-
if account["Id"] != "000000000020":
112+
if account["Id"] not in {"000000000020", "000000000010", "000000000011"}:
110113
assert account["Parents"] == [
111114
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
112115
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
113116
]
114-
else:
117+
elif account["Id"] == "000000000020":
118+
assert account["Parents"] == [{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"}]
119+
elif account["Id"] == "000000000010": # Account 10
120+
assert account["Parents"] == [
121+
{"Id": "ou-1234-5678912", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedNestedOU"},
122+
{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedOU"},
123+
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
124+
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
125+
]
126+
elif account["Id"] == "000000000011": # Account 11
127+
assert account["Parents"] == [
128+
{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedOU"},
129+
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
130+
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
131+
]
132+
133+
# Let's try this again with the resolved parent map having been fully resolved. We should get the proper parent OUs back out:
134+
fetch_additional_details(account_map, resolved_parent_map, "000000000020", "testing", "testing", "us-east-2")
135+
for account in account_map.values():
136+
if account["Id"] not in {"000000000020", "000000000010", "000000000011"}:
137+
assert account["Parents"] == [
138+
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
139+
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
140+
]
141+
elif account["Id"] == "000000000020":
115142
assert account["Parents"] == [{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"}]
143+
elif account["Id"] == "000000000010": # Account 10
144+
assert account["Parents"] == [
145+
{"Id": "ou-1234-5678912", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedNestedOU"},
146+
{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedOU"},
147+
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
148+
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
149+
]
150+
elif account["Id"] == "000000000011": # Account 11
151+
assert account["Parents"] == [
152+
{"Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeNestedOU"},
153+
{"Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT", "Name": "SomeOU"},
154+
{"Id": "r-123456", "Type": "ROOT", "Name": "ROOT"},
155+
]
116156

117157

118158
def test_async_exceptions(aws_sts: BaseClient) -> None:
@@ -125,9 +165,16 @@ def raise_exception(*args, **kwargs) -> None: # noqa
125165

126166
# Just mock with lambda functions. These can't be pickled and cause an exception.
127167
with pytest.raises(AccountIndexerProcessError) as exc:
128-
with mock.patch("starfleet.worker_ships.plugins.account_index_generator.utils.fetch_tags_and_parents", raise_exception):
168+
with mock.patch("starfleet.worker_ships.plugins.account_index_generator.utils.fetch_tags_and_parent", raise_exception):
129169
with mock.patch("starfleet.worker_ships.plugins.account_index_generator.utils.fetch_regions", raise_exception):
130-
fetch_additional_details({"000000000000": {}}, {"r-123456": "ROOT"}, "r-123456", "000000000020", "testing", "testing", "us-east-2")
170+
fetch_additional_details(
171+
{"000000000000": {}},
172+
{"r-123456": [{"Type": "ROOT", "Name": "ROOT", "Id": "r-123456"}]},
173+
"000000000020",
174+
"testing",
175+
"testing",
176+
"us-east-2",
177+
)
131178

132179
assert "Fetching tags and regions" in str(exc.value)
133180

@@ -141,7 +188,6 @@ def test_full_run(
141188
aws_s3: BaseClient,
142189
aws_sts: BaseClient,
143190
inventory_bucket: str,
144-
mock_list_parent_ous: None,
145191
account_map: Dict[str, Any],
146192
mock_direct_boto_clients: MagicMock,
147193
lambda_payload: Dict[str, Any],
@@ -152,8 +198,22 @@ def test_full_run(
152198
from starfleet.worker_ships.plugins.account_index_generator.utils import fetch_additional_details
153199

154200
# We need to get the proper account map so we can verify that we generated the proper thing:
155-
ou_map = {"ou-1234-5678910": "SomeOU", "r-123456": "ROOT"}
156-
fetch_additional_details(account_map, ou_map, "r-123456", "000000000020", "testing", "testing", "us-east-2")
201+
resolved_parent_map = {
202+
"ou-1234-5678910": [{"Name": "SomeOU", "Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"}, {"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"}],
203+
"ou-1234-5678911": [
204+
{"Name": "SomeNestedOU", "Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT"},
205+
{"Name": "SomeOU", "Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"},
206+
{"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"},
207+
],
208+
"ou-1234-5678912": [
209+
{"Name": "SomeNestedNestedOU", "Id": "ou-1234-5678912", "Type": "ORGANIZATIONAL_UNIT"},
210+
{"Name": "SomeNestedOU", "Id": "ou-1234-5678911", "Type": "ORGANIZATIONAL_UNIT"},
211+
{"Name": "SomeOU", "Id": "ou-1234-5678910", "Type": "ORGANIZATIONAL_UNIT"},
212+
{"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"},
213+
],
214+
"r-123456": [{"Name": "ROOT", "Id": "r-123456", "Type": "ROOT"}],
215+
}
216+
fetch_additional_details(account_map, resolved_parent_map, "000000000020", "testing", "testing", "us-east-2")
157217

158218
with mock.patch("starfleet.worker_ships.plugins.account_index_generator.ship.LOGGER") as mocked_logger:
159219
if cli:

0 commit comments

Comments
 (0)
Please sign in to comment.