diff --git a/moto/ec2/models.py b/moto/ec2/models.py index a6611e34a..03b690113 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -2035,7 +2035,7 @@ class SecurityRule(object): self.id = random_security_group_rule_id() self.ip_protocol = str(ip_protocol) self.ip_ranges = ip_ranges or [] - self.source_groups = source_groups + self.source_groups = source_groups or [] self.prefix_list_ids = prefix_list_ids or [] self.from_port = self.to_port = None @@ -2057,8 +2057,10 @@ class SecurityRule(object): "ALL": "-1", "icMp": "icmp", "1": "icmp", + "icmp": "icmp", } - self.ip_protocol = ip_protocol_keywords.get(self.ip_protocol) + proto = ip_protocol_keywords.get(self.ip_protocol.lower()) + self.ip_protocol = proto if proto else self.ip_protocol @property def owner_id(self): @@ -2067,11 +2069,27 @@ class SecurityRule(object): def __eq__(self, other): if self.ip_protocol != other.ip_protocol: return False - if self.ip_ranges != other.ip_ranges: + ip_ranges = list( + [item for item in self.ip_ranges if item not in other.ip_ranges] + + [item for item in other.ip_ranges if item not in self.ip_ranges] + ) + if ip_ranges: return False - if self.source_groups != other.source_groups: + source_groups = list( + [item for item in self.source_groups if item not in other.source_groups] + + [item for item in other.source_groups if item not in self.source_groups] + ) + if source_groups: return False - if self.prefix_list_ids != other.prefix_list_ids: + prefix_list_ids = list( + [item for item in self.prefix_list_ids if item not in other.prefix_list_ids] + + [ + item + for item in other.prefix_list_ids + if item not in self.prefix_list_ids + ] + ) + if prefix_list_ids: return False if self.ip_protocol != "-1": if self.from_port != other.from_port: @@ -2091,9 +2109,7 @@ class SecurityGroup(TaggedEC2Resource, CloudFormationModel): self.name = name self.description = description self.ingress_rules = [] - self.egress_rules = [ - SecurityRule("-1", None, None, [{"CidrIp": "0.0.0.0/0"}], []), - ] + self.egress_rules = [] self.enis = {} self.vpc_id = vpc_id self.owner_id = ACCOUNT_ID @@ -2102,6 +2118,10 @@ class SecurityGroup(TaggedEC2Resource, CloudFormationModel): # Append default IPv6 egress rule for VPCs with IPv6 support if vpc_id: vpc = self.ec2_backend.vpcs.get(vpc_id) + if vpc: + self.egress_rules.append( + SecurityRule("-1", None, None, [{"CidrIp": "0.0.0.0/0"}], []) + ) if vpc and len(vpc.get_cidr_block_association_set(ipv6=True)) > 0: self.egress_rules.append( SecurityRule("-1", None, None, [{"CidrIpv6": "::/0"}], []) @@ -2137,6 +2157,12 @@ class SecurityGroup(TaggedEC2Resource, CloudFormationModel): for ingress_rule in properties.get("SecurityGroupIngress", []): source_group_id = ingress_rule.get("SourceSecurityGroupId",) + source_group_name = ingress_rule.get("SourceSecurityGroupName",) + source_group = {} + if source_group_id: + source_group["GroupId"] = source_group_id + if source_group_name: + source_group["GroupName"] = source_group_name ec2_backend.authorize_security_group_ingress( group_name_or_id=security_group.id, @@ -2144,7 +2170,7 @@ class SecurityGroup(TaggedEC2Resource, CloudFormationModel): from_port=ingress_rule["FromPort"], to_port=ingress_rule["ToPort"], ip_ranges=ingress_rule.get("CidrIp"), - source_group_ids=[source_group_id] if source_group_id else [], + source_groups=[source_group] if source_group else [], vpc_id=vpc_id, ) @@ -2331,9 +2357,15 @@ class SecurityGroupBackend(object): return group def get_security_group_from_name(self, name, vpc_id=None): - for group_id, group in self.groups[vpc_id].items(): - if group.name == name: - return group + if vpc_id: + for group_id, group in self.groups[vpc_id].items(): + if group.name == name: + return group + else: + for vpc_id in self.groups: + for group_id, group in self.groups[vpc_id].items(): + if group.name == name: + return group def get_security_group_by_name_or_id(self, group_name_or_id, vpc_id): # try searching by id, fallbacks to name search @@ -2349,8 +2381,7 @@ class SecurityGroupBackend(object): from_port, to_port, ip_ranges, - source_group_names=None, - source_group_ids=None, + source_groups=None, prefix_list_ids=None, vpc_id=None, ): @@ -2379,36 +2410,50 @@ class SecurityGroupBackend(object): raise InvalidCIDRSubnetError(cidr=cidr) self._verify_group_will_respect_rule_count_limit( - group, - group.get_number_of_ingress_rules(), - ip_ranges, - source_group_names, - source_group_ids, + group, group.get_number_of_ingress_rules(), ip_ranges, source_groups, ) - source_group_names = source_group_names if source_group_names else [] - source_group_ids = source_group_ids if source_group_ids else [] - - source_groups = [] - for source_group_name in source_group_names: - source_group = self.get_security_group_from_name(source_group_name, vpc_id) - if source_group: - source_groups.append(source_group) - else: - raise InvalidSecurityGroupNotFoundError(source_group_name) - - # for VPCs - for source_group_id in source_group_ids: - source_group = self.get_security_group_from_id(source_group_id) - if source_group: - source_groups.append(source_group) - else: - raise InvalidSecurityGroupNotFoundError(source_group_id) + _source_groups = self._add_source_group(source_groups, vpc_id) security_rule = SecurityRule( - ip_protocol, from_port, to_port, ip_ranges, source_groups, prefix_list_ids + ip_protocol, from_port, to_port, ip_ranges, _source_groups, prefix_list_ids ) - group.add_ingress_rule(security_rule) + + if security_rule in group.ingress_rules: + raise InvalidPermissionDuplicateError() + # To match drift property of the security rules. + # If no rule found then add security_rule as a new rule + for rule in group.ingress_rules: + if ( + security_rule.from_port == rule.from_port + and security_rule.to_port == rule.to_port + and security_rule.ip_protocol == rule.ip_protocol + ): + rule.ip_ranges.extend( + [ + item + for item in security_rule.ip_ranges + if item not in rule.ip_ranges + ] + ) + rule.source_groups.extend( + [ + item + for item in security_rule.source_groups + if item not in rule.source_groups + ] + ) + rule.prefix_list_ids.extend( + [ + item + for item in security_rule.prefix_list_ids + if item not in rule.prefix_list_ids + ] + ) + security_rule = rule + break + else: + group.add_ingress_rule(security_rule) return security_rule, group def revoke_security_group_ingress( @@ -2418,30 +2463,54 @@ class SecurityGroupBackend(object): from_port, to_port, ip_ranges, - source_group_names=None, - source_group_ids=None, + source_groups=None, prefix_list_ids=None, vpc_id=None, ): group = self.get_security_group_by_name_or_id(group_name_or_id, vpc_id) - source_groups = [] - for source_group_name in source_group_names: - source_group = self.get_security_group_from_name(source_group_name, vpc_id) - if source_group: - source_groups.append(source_group) - - for source_group_id in source_group_ids: - source_group = self.get_security_group_from_id(source_group_id) - if source_group: - source_groups.append(source_group) + _source_groups = self._add_source_group(source_groups, vpc_id) security_rule = SecurityRule( - ip_protocol, from_port, to_port, ip_ranges, source_groups, prefix_list_ids + ip_protocol, from_port, to_port, ip_ranges, _source_groups, prefix_list_ids ) + + # To match drift property of the security rules. + for rule in group.ingress_rules: + if ( + security_rule.from_port == rule.from_port + and security_rule.to_port == rule.to_port + and security_rule.ip_protocol == rule.ip_protocol + ): + security_rule = copy.deepcopy(rule) + security_rule.ip_ranges.extend( + [item for item in ip_ranges if item not in rule.ip_ranges] + ) + security_rule.source_groups.extend( + [item for item in _source_groups if item not in rule.source_groups] + ) + security_rule.prefix_list_ids.extend( + [ + item + for item in prefix_list_ids + if item not in rule.prefix_list_ids + ] + ) + break + if security_rule in group.ingress_rules: - group.ingress_rules.remove(security_rule) + rule = group.ingress_rules[group.ingress_rules.index(security_rule)] + self._remove_items_from_rule( + ip_ranges, _source_groups, prefix_list_ids, rule + ) + + if ( + not rule.prefix_list_ids + and not rule.source_groups + and not rule.ip_ranges + ): + group.ingress_rules.remove(rule) self.sg_old_ingress_ruls[group.id] = group.ingress_rules.copy() return security_rule raise InvalidPermissionNotFoundError() @@ -2453,8 +2522,7 @@ class SecurityGroupBackend(object): from_port, to_port, ip_ranges, - source_group_names=None, - source_group_ids=None, + source_groups=None, prefix_list_ids=None, vpc_id=None, ): @@ -2486,37 +2554,52 @@ class SecurityGroupBackend(object): group, group.get_number_of_egress_rules(), ip_ranges, - source_group_names, - source_group_ids, + source_groups, egress=True, ) - source_group_names = source_group_names if source_group_names else [] - source_group_ids = source_group_ids if source_group_ids else [] - - source_groups = [] - for source_group_name in source_group_names: - source_group = self.get_security_group_from_name(source_group_name, vpc_id) - if source_group: - source_groups.append(source_group) - - # for VPCs - for source_group_id in source_group_ids: - source_group = self.get_security_group_from_id(source_group_id) - if source_group: - source_groups.append(source_group) - - if group.vpc_id: - vpc = self.vpcs.get(group.vpc_id) - if vpc and not len(vpc.get_cidr_block_association_set(ipv6=True)) > 0: - for item in ip_ranges.copy(): - if "CidrIpv6" in item: - ip_ranges.remove(item) + _source_groups = self._add_source_group(source_groups, vpc_id) security_rule = SecurityRule( - ip_protocol, from_port, to_port, ip_ranges, source_groups, prefix_list_ids + ip_protocol, from_port, to_port, ip_ranges, _source_groups, prefix_list_ids ) - group.add_egress_rule(security_rule) + + if security_rule in group.egress_rules: + raise InvalidPermissionDuplicateError() + # To match drift property of the security rules. + # If no rule found then add security_rule as a new rule + for rule in group.egress_rules: + if ( + security_rule.from_port == rule.from_port + and security_rule.to_port == rule.to_port + and security_rule.ip_protocol == rule.ip_protocol + ): + rule.ip_ranges.extend( + [ + item + for item in security_rule.ip_ranges + if item not in rule.ip_ranges + ] + ) + rule.source_groups.extend( + [ + item + for item in security_rule.source_groups + if item not in rule.source_groups + ] + ) + rule.prefix_list_ids.extend( + [ + item + for item in security_rule.prefix_list_ids + if item not in rule.prefix_list_ids + ] + ) + security_rule = rule + break + else: + group.add_egress_rule(security_rule) + return security_rule, group def revoke_security_group_egress( @@ -2526,24 +2609,14 @@ class SecurityGroupBackend(object): from_port, to_port, ip_ranges, - source_group_names=None, - source_group_ids=None, + source_groups=None, prefix_list_ids=None, vpc_id=None, ): group = self.get_security_group_by_name_or_id(group_name_or_id, vpc_id) - source_groups = [] - for source_group_name in source_group_names: - source_group = self.get_security_group_from_name(source_group_name, vpc_id) - if source_group: - source_groups.append(source_group) - - for source_group_id in source_group_ids: - source_group = self.get_security_group_from_id(source_group_id) - if source_group: - source_groups.append(source_group) + _source_groups = self._add_source_group(source_groups, vpc_id) # I don't believe this is required after changing the default egress rule # to be {'CidrIp': '0.0.0.0/0'} instead of just '0.0.0.0/0' @@ -2560,31 +2633,100 @@ class SecurityGroupBackend(object): ip_ranges.remove(item) security_rule = SecurityRule( - ip_protocol, from_port, to_port, ip_ranges, source_groups, prefix_list_ids + ip_protocol, from_port, to_port, ip_ranges, _source_groups, prefix_list_ids ) + + # To match drift property of the security rules. + # If no rule found then add security_rule as a new rule + for rule in group.egress_rules: + if ( + security_rule.from_port == rule.from_port + and security_rule.to_port == rule.to_port + and security_rule.ip_protocol == rule.ip_protocol + ): + security_rule = copy.deepcopy(rule) + security_rule.ip_ranges.extend( + [item for item in ip_ranges if item not in rule.ip_ranges] + ) + security_rule.source_groups.extend( + [item for item in _source_groups if item not in rule.source_groups] + ) + security_rule.prefix_list_ids.extend( + [ + item + for item in prefix_list_ids + if item not in rule.prefix_list_ids + ] + ) + break + if security_rule in group.egress_rules: - group.egress_rules.remove(security_rule) + rule = group.egress_rules[group.egress_rules.index(security_rule)] + self._remove_items_from_rule( + ip_ranges, _source_groups, prefix_list_ids, rule + ) + if ( + not rule.prefix_list_ids + and not rule.source_groups + and not rule.ip_ranges + ): + group.egress_rules.remove(rule) self.sg_old_egress_ruls[group.id] = group.egress_rules.copy() return security_rule raise InvalidPermissionNotFoundError() + def _remove_items_from_rule(self, ip_ranges, _source_groups, prefix_list_ids, rule): + for item in ip_ranges: + if item not in rule.ip_ranges: + raise InvalidPermissionNotFoundError() + else: + rule.ip_ranges.remove(item) + + for item in _source_groups: + if item not in rule.source_groups: + raise InvalidPermissionNotFoundError() + else: + rule.source_groups.remove(item) + + for item in prefix_list_ids: + if item not in rule.prefix_list_ids: + raise InvalidPermissionNotFoundError() + else: + rule.prefix_list_ids.remove(item) + pass + + def _add_source_group(self, source_groups, vpc_id): + _source_groups = [] + for item in source_groups: + if "OwnerId" not in item: + item["OwnerId"] = ACCOUNT_ID + # for VPCs + if "GroupId" in item: + if not self.get_security_group_from_id(item.get("GroupId")): + raise InvalidSecurityGroupNotFoundError(item.get("GroupId")) + if "GroupName" in item: + source_group = self.get_security_group_from_name( + item.get("GroupName"), vpc_id + ) + if not source_group: + raise InvalidSecurityGroupNotFoundError(item.get("GroupName")) + else: + item["GroupId"] = source_group.id + + if vpc_id: + item["VpcId"] = vpc_id + _source_groups.append(item) + return _source_groups + def _verify_group_will_respect_rule_count_limit( - self, - group, - current_rule_nb, - ip_ranges, - source_group_names=None, - source_group_ids=None, - egress=False, + self, group, current_rule_nb, ip_ranges, source_groups=None, egress=False, ): max_nb_rules = 50 if group.vpc_id else 100 future_group_nb_rules = current_rule_nb if ip_ranges: future_group_nb_rules += len(ip_ranges) - if source_group_ids: - future_group_nb_rules += len(source_group_ids) - if source_group_names: - future_group_nb_rules += len(source_group_names) + if source_groups: + future_group_nb_rules += len(source_groups) if future_group_nb_rules > max_nb_rules: if group and not egress: group.ingress_rules = self.sg_old_ingress_ruls[group.id] @@ -2636,14 +2778,11 @@ class SecurityGroupIngress(CloudFormationModel): ) assert ip_protocol + source_group = {} if source_security_group_id: - source_security_group_ids = [source_security_group_id] - else: - source_security_group_ids = None + source_group["GroupId"] = source_security_group_id if source_security_group_name: - source_security_group_names = [source_security_group_name] - else: - source_security_group_names = None + source_group["GroupName"] = source_security_group_name if cidr_ip: ip_ranges = [{"CidrIp": cidr_ip, "Description": cidr_desc}] else: @@ -2664,8 +2803,7 @@ class SecurityGroupIngress(CloudFormationModel): from_port=from_port, to_port=to_port, ip_ranges=ip_ranges, - source_group_ids=source_security_group_ids, - source_group_names=source_security_group_names, + source_groups=[source_group] if source_group else [], ) return cls(security_group, properties) diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index cc60201cd..089ea0ff9 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -42,14 +42,19 @@ def parse_sg_attributes_from_dict(sg_attributes): ip_ranges.append({"CidrIpv6": cidr_ipv6}) source_groups = [] - source_group_ids = [] groups_tree = sg_attributes.get("Groups") or {} for group_idx in sorted(groups_tree.keys()): group_dict = groups_tree[group_idx] + source_group = {} if "GroupId" in group_dict: - source_group_ids.append(group_dict["GroupId"][0]) - elif "GroupName" in group_dict: - source_groups.append(group_dict["GroupName"][0]) + source_group["GroupId"] = group_dict["GroupId"][0] + if "GroupName" in group_dict: + source_group["GroupName"] = group_dict["GroupName"][0] + if "Description" in group_dict: + source_group["Description"] = group_dict["Description"][0] + if "OwnerId" in group_dict: + source_group["OwnerId"] = group_dict["OwnerId"][0] + source_groups.append(source_group) prefix_list_ids = [] pl_tree = sg_attributes.get("PrefixListIds") or {} @@ -68,7 +73,6 @@ def parse_sg_attributes_from_dict(sg_attributes): to_port, ip_ranges, source_groups, - source_group_ids, prefix_list_ids, ) @@ -97,7 +101,6 @@ class SecurityGroups(BaseResponse): to_port, ip_ranges, source_groups, - source_group_ids, prefix_list_ids, ) = parse_sg_attributes_from_dict(querytree) @@ -108,7 +111,6 @@ class SecurityGroups(BaseResponse): to_port, ip_ranges, source_groups, - source_group_ids, prefix_list_ids, ) @@ -122,7 +124,6 @@ class SecurityGroups(BaseResponse): to_port, ip_ranges, source_groups, - source_group_ids, prefix_list_ids, ) = parse_sg_attributes_from_dict(ip_permission) @@ -133,7 +134,6 @@ class SecurityGroups(BaseResponse): to_port, ip_ranges, source_groups, - source_group_ids, prefix_list_ids, ) @@ -223,6 +223,14 @@ CREATE_SECURITY_GROUP_RESPONSE = """ @@ -255,9 +263,18 @@ DESCRIBE_SECURITY_GROUPS_RESPONSE = """{{ source_group.OwnerId }} + {% endif %} + {% if source_group.GroupId and source_group.GroupId != "" %} + {{ source_group.GroupId }} + {% endif %} + {% if source_group.GroupName and source_group.GroupName != "" %} + {{ source_group.GroupName }} + {% endif %} + {% if source_group.Description and source_group.Description != "" %} + {{ source_group.Description }} + {% endif %} {% endfor %} @@ -289,9 +306,11 @@ DESCRIBE_SECURITY_GROUPS_RESPONSE = """{{ source_group.OwnerId }} + {% endif %} + {% if source_group.GroupId and source_group.GroupId != "" %} + {{ source_group.GroupId }} + {% endif %} + {% if source_group.GroupName and source_group.GroupName != "" %} + {{ source_group.GroupName }} + {% endif %} + {% if source_group.Description and source_group.Description != "" %} + {{ source_group.Description }} + {% endif %} {% endfor %} @@ -340,14 +368,14 @@ DESCRIBE_SECURITY_GROUPS_RESPONSE = """ {% endfor %} + {% for item in rule.source_groups %} + + {% if item.Description and item.Description != "" %} + {{ item.Description }} + {% endif %} + {% if rule.from_port is not none %} + {{ rule.from_port }} + {% endif %} + {{ group.id }} + {{ rule.owner_id }} + {{ rule.ip_protocol }} + true + {{ rule.id }} + {% if rule.to_port is not none %} + {{ rule.to_port }} + {% endif %} + + {% if item.OwnerId and item.OwnerId != "" %} + {{ item.OwnerId }} + {% endif %} + {% if item.GroupId and item.GroupId != "" %} + {{ item.GroupId }} + {% endif %} + {% if item.VpcId and item.VpcId != "" %} + {{ item.VpcId }} + {% endif %} + + + {% endfor %} """ @@ -459,6 +516,35 @@ AUTHORIZE_SECURITY_GROUP_EGRESS_RESPONSE = """ {% endfor %} + {% for item in rule.source_groups %} + + {% if item.Description and item.Description != "" %} + {{ item.Description }} + {% endif %} + {% if rule.from_port is not none %} + {{ rule.from_port }} + {% endif %} + {{ group.id }} + {{ rule.owner_id }} + {{ rule.ip_protocol }} + true + {{ rule.id }} + {% if rule.to_port is not none %} + {{ rule.to_port }} + {% endif %} + + {% if item.OwnerId and item.OwnerId != "" %} + {{ item.OwnerId }} + {% endif %} + {% if item.GroupId and item.GroupId != "" %} + {{ item.GroupId }} + {% endif %} + {% if item.VpcId and item.VpcId != "" %} + {{ item.VpcId }} + {% endif %} + + + {% endfor %} """ diff --git a/moto/emr/utils.py b/moto/emr/utils.py index 489f5f89f..e6d9ca7f8 100644 --- a/moto/emr/utils.py +++ b/moto/emr/utils.py @@ -268,7 +268,7 @@ class EmrSecurityGroupManager(object): "ip_protocol": "-1", "ip_ranges": [{"CidrIp": "0.0.0.0/0"}], "to_port": None, - "source_group_ids": [], + "source_groups": [], }, { "group_name_or_id": EmrManagedSecurityGroup.Kind.SLAVE, @@ -276,7 +276,7 @@ class EmrSecurityGroupManager(object): "ip_protocol": "-1", "ip_ranges": [{"CidrIp": "0.0.0.0/0"}], "to_port": None, - "source_group_ids": [], + "source_groups": [], }, { "group_name_or_id": EmrManagedSecurityGroup.Kind.SERVICE, @@ -284,9 +284,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 8443, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, ] @@ -298,9 +298,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 65535, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -309,7 +309,7 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 8443, - "source_group_ids": [EmrManagedSecurityGroup.Kind.SERVICE], + "source_groups": [{"GroupId": EmrManagedSecurityGroup.Kind.SERVICE}], }, { "group_name_or_id": EmrManagedSecurityGroup.Kind.MASTER, @@ -317,9 +317,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "udp", "ip_ranges": [], "to_port": 65535, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -328,9 +328,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "icmp", "ip_ranges": [], "to_port": -1, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -339,9 +339,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 65535, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.SLAVE, - EmrManagedSecurityGroup.Kind.MASTER, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -350,7 +350,7 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 8443, - "source_group_ids": [EmrManagedSecurityGroup.Kind.SERVICE], + "source_groups": [{"GroupId": EmrManagedSecurityGroup.Kind.SERVICE}], }, { "group_name_or_id": EmrManagedSecurityGroup.Kind.SLAVE, @@ -358,9 +358,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "udp", "ip_ranges": [], "to_port": 65535, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -369,9 +369,9 @@ class EmrSecurityGroupManager(object): "ip_protocol": "icmp", "ip_ranges": [], "to_port": -1, - "source_group_ids": [ - EmrManagedSecurityGroup.Kind.MASTER, - EmrManagedSecurityGroup.Kind.SLAVE, + "source_groups": [ + {"GroupId": EmrManagedSecurityGroup.Kind.MASTER}, + {"GroupId": EmrManagedSecurityGroup.Kind.SLAVE}, ], }, { @@ -380,7 +380,7 @@ class EmrSecurityGroupManager(object): "ip_protocol": "tcp", "ip_ranges": [], "to_port": 9443, - "source_group_ids": [EmrManagedSecurityGroup.Kind.MASTER], + "source_groups": [{"GroupId": EmrManagedSecurityGroup.Kind.MASTER}], }, ] @@ -452,7 +452,8 @@ class EmrSecurityGroupManager(object): rendered_rules = copy.deepcopy(rules) for rule in rendered_rules: rule["group_name_or_id"] = managed_groups[rule["group_name_or_id"]].id - rule["source_group_ids"] = [ - managed_groups[group].id for group in rule["source_group_ids"] + rule["source_groups"] = [ + {"GroupId": managed_groups[group.get("GroupId")].id} + for group in rule["source_groups"] ] return rendered_rules diff --git a/tests/terraform-tests.failures.txt b/tests/terraform-tests.failures.txt index e784b8ceb..6003d6ac2 100644 --- a/tests/terraform-tests.failures.txt +++ b/tests/terraform-tests.failures.txt @@ -3,4 +3,5 @@ TestAccAWSEc2TransitGatewayPeeringAttachmentAccepter TestAccAWSEc2TransitGatewayRouteTableAssociation TestAccAWSEc2TransitGatewayVpcAttachment TestAccAWSFms -TestAccAWSIAMRolePolicy \ No newline at end of file +TestAccAWSIAMRolePolicy +TestAccAWSSecurityGroup_forceRevokeRules_ \ No newline at end of file diff --git a/tests/terraform-tests.success.txt b/tests/terraform-tests.success.txt index be35edeb4..1578b48fa 100644 --- a/tests/terraform-tests.success.txt +++ b/tests/terraform-tests.success.txt @@ -94,37 +94,4 @@ TestAccAWSRouteTable_basic TestAccAWSSsmDocumentDataSource TestAccAwsEc2ManagedPrefixList TestAccAWSEgressOnlyInternetGateway -TestAccAWSSecurityGroup_allowAll -TestAccAWSSecurityGroup_sourceSecurityGroup -TestAccAWSSecurityGroup_IPRangeAndSecurityGroupWithSameRules -TestAccAWSSecurityGroup_IPRangesWithSameRules -TestAccAWSSecurityGroup_basic -TestAccAWSSecurityGroup_egressConfigMode -TestAccAWSSecurityGroup_ingressConfigMode -TestAccAWSSecurityGroup_ipv6 -TestAccAWSSecurityGroup_Name_Generated -TestAccAWSSecurityGroup_Name_TerraformPrefix -TestAccAWSSecurityGroup_NamePrefix -TestAccAWSSecurityGroup_NamePrefix_TerraformPrefix -TestAccAWSSecurityGroup_self -TestAccAWSSecurityGroup_vpc -TestAccAWSSecurityGroup_vpcNegOneIngress -TestAccAWSSecurityGroup_vpcProtoNumIngress -TestAccAWSSecurityGroup_multiIngress -TestAccAWSSecurityGroup_ruleDescription -TestAccAWSSecurityGroup_defaultEgressVPC -TestAccAWSSecurityGroup_defaultEgressClassic -TestAccAWSSecurityGroup_driftComplex -TestAccAWSSecurityGroup_invalidCIDRBlock -TestAccAWSSecurityGroup_tags -TestAccAWSSecurityGroup_CIDRandGroups -TestAccAWSSecurityGroup_ingressWithCidrAndSGsVPC -TestAccAWSSecurityGroup_ipv4andipv6Egress -TestAccAWSSecurityGroup_failWithDiffMismatch -TestAccAWSSecurityGroup_ruleLimitExceededAppend -TestAccAWSSecurityGroupRule_Ingress_VPC -TestAccAWSSecurityGroupRule_MultipleRuleSearching_AllProtocolCrash -TestAccAWSSecurityGroup_rulesDropOnError -TestAccAWSSecurityGroup_ruleLimitExceededAllNew -TestAccAWSSecurityGroup_ruleLimitExceededPrepend -TestAccAWSSecurityGroup_ruleLimitCidrBlockExceededAppend +TestAccAWSSecurityGroup_ diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 65e959b08..ff4dc8984 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -242,8 +242,10 @@ def test_authorize_ip_range_and_revoke(): security_group.rules.should.have.length_of(0) # Test for egress as well + vpc_conn = boto.connect_vpc() + vpc = vpc_conn.create_vpc("10.0.0.0/16") egress_security_group = conn.create_security_group( - "testegress", "testegress", vpc_id="vpc-3432589" + "testegress", "testegress", vpc_id=vpc.id ) with pytest.raises(EC2ResponseError) as ex: @@ -308,7 +310,11 @@ def test_authorize_ip_range_and_revoke(): cidr_ip="123.123.123.123/32", ) - egress_security_group = conn.get_all_security_groups()[0] + egress_security_group = [ + group + for group in conn.get_all_security_groups() + if group.id == egress_security_group.id + ][0] # There is still the default outbound rule egress_security_group.rules_egress.should.have.length_of(1) @@ -576,9 +582,7 @@ def test_sec_group_rule_limit(): group_id=sg.id, ip_protocol="-1", src_group_id=other_sg.id ) # fill the rules up the limit - # remember that by default, when created a sec group contains 1 egress rule - # so our other_sg rule + 98 CIDR IP rules + 1 by default == 100 the limit - for i in range(1, 99): + for i in range(1, 100): ec2_conn.authorize_security_group_egress( group_id=sg.id, ip_protocol="-1", cidr_ip="{0}.0.0.0/0".format(i) ) @@ -902,26 +906,80 @@ def test_authorize_and_revoke_in_bulk(): "PrefixListIds": [], }, ] - expected_ip_permissions = copy.deepcopy(ip_permissions) - expected_ip_permissions[1]["UserIdGroupPairs"][0]["GroupName"] = "sg02" - expected_ip_permissions[2]["UserIdGroupPairs"][0]["GroupId"] = sg03.id - expected_ip_permissions[3]["UserIdGroupPairs"][0]["GroupId"] = sg04.id - expected_ip_permissions[4]["UserIdGroupPairs"][0]["GroupName"] = "sg04" - sg01.authorize_ingress(IpPermissions=ip_permissions) - sg01.ip_permissions.should.have.length_of(5) + org_ip_permissions = copy.deepcopy(ip_permissions) + + for rule in ip_permissions.copy(): + for other_rule in ip_permissions.copy(): + if ( + rule is not other_rule + and rule.get("IpProtocol") == other_rule.get("IpProtocol") + and rule.get("FromPort") == other_rule.get("FromPort") + and rule.get("ToPort") == other_rule.get("ToPort") + ): + if rule in ip_permissions: + ip_permissions.remove(rule) + if other_rule in ip_permissions: + ip_permissions.remove(other_rule) + + rule["UserIdGroupPairs"].extend( + [ + item + for item in other_rule["UserIdGroupPairs"] + if item not in rule["UserIdGroupPairs"] + ] + ) + rule["IpRanges"].extend( + [ + item + for item in other_rule["IpRanges"] + if item not in rule["IpRanges"] + ] + ) + rule["Ipv6Ranges"].extend( + [ + item + for item in other_rule["Ipv6Ranges"] + if item not in rule["Ipv6Ranges"] + ] + ) + rule["PrefixListIds"].extend( + [ + item + for item in other_rule["PrefixListIds"] + if item not in rule["PrefixListIds"] + ] + ) + if rule not in ip_permissions: + ip_permissions.append(json.loads(json.dumps(rule, sort_keys=True))) + + expected_ip_permissions = copy.deepcopy(ip_permissions) + expected_ip_permissions[1]["UserIdGroupPairs"][0]["GroupId"] = sg04.id + expected_ip_permissions[3]["UserIdGroupPairs"][0]["GroupId"] = sg03.id + expected_ip_permissions = json.dumps(expected_ip_permissions, sort_keys=True) + + sg01.authorize_ingress(IpPermissions=org_ip_permissions) + # Due to drift property of the Security Group, + # rules with same Ip protocol, FromPort and ToPort will be merged together + sg01.ip_permissions.should.have.length_of(4) + sorted_sg01_ip_permissions = json.dumps(sg01.ip_permissions, sort_keys=True) for ip_permission in expected_ip_permissions: - sg01.ip_permissions.should.contain(ip_permission) + sorted_sg01_ip_permissions.should.contain(ip_permission) sg01.revoke_ingress(IpPermissions=ip_permissions) sg01.ip_permissions.should.be.empty for ip_permission in expected_ip_permissions: sg01.ip_permissions.shouldnt.contain(ip_permission) - sg01.authorize_egress(IpPermissions=ip_permissions) - sg01.ip_permissions_egress.should.have.length_of(6) + sg01.authorize_egress(IpPermissions=org_ip_permissions) + # Due to drift property of the Security Group, + # rules with same Ip protocol, FromPort and ToPort will be merged together + sg01.ip_permissions_egress.should.have.length_of(5) + sorted_sg01_ip_permissions_egress = json.dumps( + sg01.ip_permissions_egress, sort_keys=True + ) for ip_permission in expected_ip_permissions: - sg01.ip_permissions_egress.should.contain(ip_permission) + sorted_sg01_ip_permissions_egress.should.contain(ip_permission) sg01.revoke_egress(IpPermissions=ip_permissions) sg01.ip_permissions_egress.should.have.length_of(1) @@ -983,7 +1041,10 @@ def test_get_all_security_groups_filter_with_same_vpc_id(): @mock_ec2 def test_revoke_security_group_egress(): ec2 = boto3.resource("ec2", "us-east-1") - sg = ec2.create_security_group(Description="Test SG", GroupName="test-sg") + vpc = ec2.create_vpc(CidrBlock="10.0.0.0/16") + sg = ec2.create_security_group( + Description="Test SG", GroupName="test-sg", VpcId=vpc.id + ) sg.ip_permissions_egress.should.equal( [ @@ -1047,7 +1108,7 @@ def test_security_group_rules_added_via_the_backend_can_be_revoked_via_the_api() "ip_protocol": "udp", "ip_ranges": [], "to_port": 65535, - "source_group_ids": [sg.id], + "source_groups": [{"GroupId": sg.id}], } ec2_backend.authorize_security_group_ingress(**rule_ingress) rule_egress = { @@ -1056,7 +1117,7 @@ def test_security_group_rules_added_via_the_backend_can_be_revoked_via_the_api() "ip_protocol": "tcp", "ip_ranges": [], "to_port": 8443, - "source_group_ids": [sg.id], + "source_groups": [{"GroupId": sg.id}], } ec2_backend.authorize_security_group_egress(**rule_egress) # Both rules (plus the default egress) should now be present.