From c1b2c78db2c4156e44d501b33f36330034d37b00 Mon Sep 17 00:00:00 2001 From: Brian Pandola Date: Fri, 9 Oct 2020 05:55:48 -0700 Subject: [PATCH] Fix `TagFilter` implementation in `tag:GetResources` (#3366) The `tag_filter` method has been re-arranged to mimic the actual AWS behavior: Return `True` if *any* tag matches a filter and *all* filters are matched. Python's closures are late-binding, so we have to modify the lambdas accordingly! Closes #2814 --- moto/resourcegroupstaggingapi/models.py | 21 ++++--- .../test_resourcegroupstaggingapi.py | 59 +++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/moto/resourcegroupstaggingapi/models.py b/moto/resourcegroupstaggingapi/models.py index 4cdf73cc7..bd63847a0 100644 --- a/moto/resourcegroupstaggingapi/models.py +++ b/moto/resourcegroupstaggingapi/models.py @@ -113,32 +113,35 @@ class ResourceGroupsTaggingAPIBackend(BaseBackend): # https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html # TODO move these to their respective backends - filters = [lambda t, v: True] + filters = [] for tag_filter_dict in tag_filters: values = tag_filter_dict.get("Values", []) if len(values) == 0: # Check key matches - filters.append(lambda t, v: t == tag_filter_dict["Key"]) + filters.append(lambda t, v, key=tag_filter_dict["Key"]: t == key) elif len(values) == 1: # Check its exactly the same as key, value filters.append( - lambda t, v: t == tag_filter_dict["Key"] and v == values[0] + lambda t, v, key=tag_filter_dict["Key"], value=values[0]: t == key + and v == value ) else: # Check key matches and value is one of the provided values - filters.append(lambda t, v: t == tag_filter_dict["Key"] and v in values) + filters.append( + lambda t, v, key=tag_filter_dict["Key"], vl=values: t == key + and v in vl + ) def tag_filter(tag_list): result = [] if tag_filters: - for tag in tag_list: + for f in filters: temp_result = [] - for f in filters: + for tag in tag_list: f_result = f(tag["Key"], tag["Value"]) temp_result.append(f_result) - result.append(all(temp_result)) - - return any(result) + result.append(any(temp_result)) + return all(result) else: return True diff --git a/tests/test_resourcegroupstaggingapi/test_resourcegroupstaggingapi.py b/tests/test_resourcegroupstaggingapi/test_resourcegroupstaggingapi.py index c14636fff..154744a14 100644 --- a/tests/test_resourcegroupstaggingapi/test_resourcegroupstaggingapi.py +++ b/tests/test_resourcegroupstaggingapi/test_resourcegroupstaggingapi.py @@ -293,3 +293,62 @@ def test_get_resources_s3(): response_keys.remove(resource["Tags"][0]["Key"]) response_keys.should.have.length_of(0) + + +@mock_ec2 +@mock_resourcegroupstaggingapi +def test_multiple_tag_filters(): + client = boto3.client("ec2", region_name="eu-central-1") + + resp = client.run_instances( + ImageId="ami-123", + MinCount=1, + MaxCount=1, + InstanceType="t2.micro", + TagSpecifications=[ + { + "ResourceType": "instance", + "Tags": [ + {"Key": "MY_TAG1", "Value": "MY_UNIQUE_VALUE"}, + {"Key": "MY_TAG2", "Value": "MY_SHARED_VALUE"}, + ], + }, + { + "ResourceType": "instance", + "Tags": [{"Key": "MY_TAG3", "Value": "MY_VALUE3"}], + }, + ], + ) + instance_1_id = resp["Instances"][0]["InstanceId"] + + resp = client.run_instances( + ImageId="ami-456", + MinCount=1, + MaxCount=1, + InstanceType="t2.micro", + TagSpecifications=[ + { + "ResourceType": "instance", + "Tags": [ + {"Key": "MY_TAG1", "Value": "MY_ALT_UNIQUE_VALUE"}, + {"Key": "MY_TAG2", "Value": "MY_SHARED_VALUE"}, + ], + }, + { + "ResourceType": "instance", + "Tags": [{"Key": "MY_ALT_TAG3", "Value": "MY_VALUE3"}], + }, + ], + ) + instance_2_id = resp["Instances"][0]["InstanceId"] + + rtapi = boto3.client("resourcegroupstaggingapi", region_name="eu-central-1") + results = rtapi.get_resources( + TagFilters=[ + {"Key": "MY_TAG1", "Values": ["MY_UNIQUE_VALUE"]}, + {"Key": "MY_TAG2", "Values": ["MY_SHARED_VALUE"]}, + ] + ).get("ResourceTagMappingList", []) + results.should.have.length_of(1) + instance_1_id.should.be.within(results[0]["ResourceARN"]) + instance_2_id.shouldnt.be.within(results[0]["ResourceARN"])