Skip to content

Commit

Permalink
address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nick123pig committed Sep 21, 2020
1 parent 7bc5b5c commit ff84b63
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 90 deletions.
15 changes: 7 additions & 8 deletions CONFIG_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,14 @@ An example of the above is implemented for S3. You can see that by looking at:
1. `moto/s3/config.py`
1. `moto/config/models.py`

As well as the corresponding unit tests in:
### Testing
For each resource type, you will need to test write tests for a few separate areas:

1. `tests/s3/test_s3.py`
1. `tests/config/test_config.py`
- Test the backend queries to ensure discovered resources come back (ie for `IAM::Policy`, write `tests.tests_iam.test_policy_list_config_discovered_resources`). For writing these tests, you must not make use of `boto` to create resources. You will need to use the backend model methods to provision the resources. This is to make tests compatible with the moto server. You must make tests for the resource type to test listing and object fetching.

Note for unit testing, you will want to add a test to ensure that you can query all the resources effectively. For testing this feature,
the unit tests for the `ConfigQueryModel` will not make use of `boto` to create resources, such as S3 buckets. You will need to use the
backend model methods to provision the resources. This is to make tests compatible with the moto server. You should absolutely make tests
in the resource type to test listing and object fetching.
- Test the config dict for all scenarios (ie for `IAM::Policy`, write `tests.tests_iam.test_policy_config_dict`). For writing this test, you'll need to create resources in the same way as the first test (without using `boto`), in every meaningful configuration that would produce a different config dict. Then, query the backend and ensure each of the dicts are as you expect.

- Test that everything works end to end with the `boto` clients. (ie for `IAM::Policy`, write `tests.tests_iam.test_policy_config_client`). The main two items to test will be the `boto.client('config').list_discovered_resources()`, `boto.client('config').list_aggregate_discovered_resources()`, `moto.client('config').batch_get_resource_config()`, and `moto.client('config').batch_aggregate_get_resource_config()`. This test doesn't have to be super thorough, but it basically tests that the front end and backend logic all works together and returns correct resources. Beware the aggregate methods all have capital first letters (ie `Limit`), while non-aggregate methods have lowercase first letters (ie `limit`)

### Listing
S3 is currently the model implementation, but it also odd in that S3 is a global resource type with regional resource residency.
Expand Down Expand Up @@ -117,4 +116,4 @@ return for it.
When implementing resource config fetching, you will need to return at a minimum `None` if the resource is not found, or a `dict` that looks
like what AWS Config would return.

It's recommended to read the comment for the `ConfigQueryModel` 's `get_config_resource` function in [base class here](moto/core/models.py).
It's recommended to read the comment for the `ConfigQueryModel` 's `get_config_resource` function in [base class here](moto/core/models.py).
26 changes: 20 additions & 6 deletions moto/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
convert_flask_to_responses_response,
)


ACCOUNT_ID = os.environ.get("MOTO_ACCOUNT_ID", "123456789012")
CONFIG_BACKEND_DELIM = "\x1e" # Record Seperator "RS" ASCII Character


class BaseMockAWS(object):
Expand Down Expand Up @@ -768,23 +768,37 @@ def get_config_resource(

def aggregate_regions(self, path, backend_region, resource_region):
"""
Returns a list of "region\1eresourcename" strings
This method will is called for both aggregated and non-aggregated calls for config resources.
It will figure out how to return the full list of resources for a given regional backend and append them to a final list.
It produces a list of both the region and the resource name with a delimiter character (CONFIG_BACKEND_DELIM, ASCII Record separator, \x1e).
IE: "us-east-1\x1ei-1234567800"
Each config-enabled resource has a method named `list_config_service_resources` which has to parse the delimiter
...
:param path: - A dict accessor string applied to the backend that locates the resource.
:param backend_region:
:param resource_region:
:return: - Returns a list of "region\x1eresourcename" strings
"""

filter_region = backend_region or resource_region
if filter_region:
filter_resources = list(self.backends[filter_region].__dict__[path].keys())
return map(
lambda resource: "{}\1e{}".format(filter_region, resource),
filter_resources,
return list(
map(
lambda resource: "{}{}{}".format(
filter_region, CONFIG_BACKEND_DELIM, resource
),
filter_resources,
)
)

# If we don't have a filter region
ret = []
for region in self.backends:
this_region_resources = list(self.backends[region].__dict__[path].keys())
for resource in this_region_resources:
ret.append("{}\1e{}".format(region, resource))
ret.append("{}{}{}".format(region, CONFIG_BACKEND_DELIM, resource))
return ret


Expand Down
63 changes: 37 additions & 26 deletions moto/iam/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from moto.core.models import ConfigQueryModel
from moto.iam import iam_backends

CONFIG_BACKEND_DELIM = "\x1e" # Record Seperator "RS" ASCII Character


class RoleConfigQuery(ConfigQueryModel):
def list_config_service_resources(
Expand All @@ -15,26 +17,25 @@ def list_config_service_resources(
backend_region=None,
resource_region=None,
):
# For aggregation -- did we get both a resource ID and a resource name?
if resource_ids and resource_name:
# If the values are different, then return an empty list:
if resource_name not in resource_ids:
return [], None
# IAM roles are "global" and aren't assigned into any availability zone
# The resource ID is a AWS-assigned random string like "AROA0BSVNSZKXVHS00SBJ"
# The resource name is a user-assigned string like "MyDevelopmentAdminRole"

role_list = self.aggregate_regions("roles", backend_region, resource_region)
# Grab roles from backend
role_list = self.aggregate_regions("roles", "global", None)

if not role_list:
return [], None

# Pagination logic:
# Pagination logic
sorted_roles = sorted(role_list)
new_token = None

# Get the start:
if not next_token:
start = 0
else:
# "Tokens" are region + \00 + resource ID.
# "Tokens" are region + \x1e + resource ID.
if next_token not in sorted_roles:
raise InvalidNextTokenException()

Expand All @@ -46,13 +47,16 @@ def list_config_service_resources(
if len(sorted_roles) > (start + limit):
new_token = sorted_roles[start + limit]

# Each element is a string of "region\x1eresource_id"
return (
[
{
"type": "AWS::IAM::Role",
"id": role.split("\1e")[1],
"name": role.split("\1e")[1],
"region": role.split("\1e")[0],
"id": role.split(CONFIG_BACKEND_DELIM)[1],
"name": self.backends["global"]
.roles[role.split(CONFIG_BACKEND_DELIM)[1]]
.name,
"region": role.split(CONFIG_BACKEND_DELIM)[0],
}
for role in role_list
],
Expand All @@ -71,7 +75,7 @@ def get_config_resource(
if resource_name and role.name != resource_name:
return

# Format the bucket to the AWS Config format:
# Format the role to the AWS Config format:
config_data = role.to_config_dict()

# The 'configuration' field is also a JSON string:
Expand All @@ -95,16 +99,19 @@ def list_config_service_resources(
backend_region=None,
resource_region=None,
):
# For aggregation -- did we get both a resource ID and a resource name?
if resource_ids and resource_name:
# If the values are different, then return an empty list:
if resource_name not in resource_ids:
return [], None

# We don't want to include AWS Managed Policies
# IAM policies are "global" and aren't assigned into any availability zone
# The resource ID is a AWS-assigned random string like "ANPA0BSVNSZK00SJSPVUJ"
# The resource name is a user-assigned string like "my-development-policy"

# We don't want to include AWS Managed Policies. This technically needs to
# respect the configuration recorder's 'includeGlobalResourceTypes' setting,
# but it's default set be default, and moto's config doesn't yet support
# custom configuration recorders, we'll just behave as default.
policy_list = filter(
lambda policy: not policy.split("\1e")[1].startswith("arn:aws:iam::aws"),
self.aggregate_regions("managed_policies", backend_region, resource_region),
lambda policy: not policy.split(CONFIG_BACKEND_DELIM)[1].startswith(
"arn:aws:iam::aws"
),
self.aggregate_regions("managed_policies", "global", None),
)

if not policy_list:
Expand All @@ -118,7 +125,7 @@ def list_config_service_resources(
if not next_token:
start = 0
else:
# "Tokens" are region + \00 + resource ID.
# "Tokens" are region + \x1e + resource ID.
if next_token not in sorted_policies:
raise InvalidNextTokenException()

Expand All @@ -134,9 +141,13 @@ def list_config_service_resources(
[
{
"type": "AWS::IAM::Policy",
"id": policy.split("\1e")[1],
"name": policy.split("\1e")[1],
"region": policy.split("\1e")[0],
"id": self.backends["global"]
.managed_policies[policy.split(CONFIG_BACKEND_DELIM)[1]]
.id,
"name": self.backends["global"]
.managed_policies[policy.split(CONFIG_BACKEND_DELIM)[1]]
.name,
"region": policy.split(CONFIG_BACKEND_DELIM)[0],
}
for policy in policy_list
],
Expand All @@ -155,7 +166,7 @@ def get_config_resource(
if resource_name and policy.name != resource_name:
return

# Format the bucket to the AWS Config format:
# Format the policy to the AWS Config format:
config_data = policy.to_config_dict()

# The 'configuration' field is also a JSON string:
Expand Down
Loading

0 comments on commit ff84b63

Please sign in to comment.