From a7edd5bc066672c6ab2f8664f42d42692eabe8d6 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Sat, 30 Nov 2024 10:53:44 -0100 Subject: [PATCH] ELBv2: register_targets() now validates that instances are running --- moto/elbv2/exceptions.py | 8 +++ moto/elbv2/models.py | 50 ++++++++------ tests/test_elbv2/test_elbv2_target_health.py | 70 +++++++++++++------- 3 files changed, 84 insertions(+), 44 deletions(-) diff --git a/moto/elbv2/exceptions.py b/moto/elbv2/exceptions.py index cd4cc6e47bab..8ab6d7a778e5 100644 --- a/moto/elbv2/exceptions.py +++ b/moto/elbv2/exceptions.py @@ -94,6 +94,14 @@ def __init__(self) -> None: ) +class TargetNotRunning(ELBClientError): + def __init__(self, instance_id: str) -> None: + super().__init__( + "InvalidTarget", + f"The following targets are not in a running state and cannot be registered: '{instance_id}'", + ) + + class EmptyListenersError(ELBClientError): def __init__(self) -> None: super().__init__("ValidationError", "Listeners cannot be empty") diff --git a/moto/elbv2/models.py b/moto/elbv2/models.py index 4c691c8bc764..10ae12cd9ed8 100644 --- a/moto/elbv2/models.py +++ b/moto/elbv2/models.py @@ -9,7 +9,7 @@ from moto.core.common_models import BaseModel, CloudFormationModel from moto.core.exceptions import RESTError from moto.core.utils import iso_8601_datetime_with_milliseconds -from moto.ec2.models import ec2_backends +from moto.ec2.models import EC2Backend, ec2_backends from moto.ec2.models.subnets import Subnet from moto.moto_api._internal import mock_random from moto.utilities.tagging_service import TaggingService @@ -39,6 +39,7 @@ RuleNotFoundError, SubnetNotFoundError, TargetGroupNotFoundError, + TargetNotRunning, TooManyCertificatesError, TooManyTagsError, ValidationError, @@ -78,7 +79,8 @@ class FakeTargetGroup(CloudFormationModel): def __init__( self, name: str, - arn: str, + account_id: str, + region_name: str, vpc_id: str, protocol: str, port: str, @@ -97,7 +99,11 @@ def __init__( ): # TODO: default values differs when you add Network Load balancer self.name = name - self.arn = arn + self.account_id = account_id + self.region_name = region_name + self.arn = make_arn_for_target_group( + account_id=self.account_id, name=name, region_name=self.region_name + ) self.vpc_id = vpc_id if target_type == "lambda": self.protocol = None @@ -156,6 +162,10 @@ def physical_resource_id(self) -> str: def register(self, targets: List[Dict[str, Any]]) -> None: for target in targets: + if instance := self.ec2_backend.get_instance_by_id(target["id"]): + if instance.state != "running": + raise TargetNotRunning(instance_id=target["id"]) + self.targets[target["id"]] = { "id": target["id"], "port": target.get("port", self.port), @@ -175,7 +185,9 @@ def deregister_terminated_instances(self, instance_ids: List[str]) -> None: t = self.targets.pop(target_id) self.terminated_targets[target_id] = t - def health_for(self, target: Dict[str, Any], ec2_backend: Any) -> FakeHealthStatus: + def health_for( + self, target: Dict[str, Any], ec2_backend: EC2Backend + ) -> FakeHealthStatus: t = self.targets.get(target["id"]) if t is None: port = self.port @@ -220,7 +232,7 @@ def health_for(self, target: Dict[str, Any], ec2_backend: Any) -> FakeHealthStat ) if t["id"].startswith("i-"): # EC2 instance ID instance = ec2_backend.get_instance_by_id(t["id"]) - if instance.state == "stopped": + if instance and instance.state == "stopped": return FakeHealthStatus( t["id"], t["port"], @@ -283,6 +295,10 @@ def create_from_cloudformation_json( # type: ignore[misc] ) return target_group + @property + def ec2_backend(self) -> EC2Backend: + return ec2_backends[self.account_id][self.region_name] + class FakeListener(CloudFormationModel): def __init__( @@ -768,13 +784,7 @@ def __init__(self, region_name: str, account_id: str): self.tagging_service = TaggingService() @property - def ec2_backend(self) -> Any: # type: ignore[misc] - """ - EC2 backend - - :return: EC2 Backend - :rtype: moto.ec2.models.EC2Backend - """ + def ec2_backend(self) -> EC2Backend: return ec2_backends[self.account_id][self.region_name] def create_load_balancer( @@ -798,8 +808,8 @@ def create_load_balancer( if subnet is None: raise SubnetNotFoundError() subnets.append(subnet) - for subnet in subnet_mappings or []: - subnet_id = subnet["SubnetId"] + for subnet_mapping in subnet_mappings or []: + subnet_id = subnet_mapping["SubnetId"] subnet = self.ec2_backend.get_subnet(subnet_id) if subnet is None: raise SubnetNotFoundError() @@ -1166,7 +1176,7 @@ def create_target_group(self, name: str, **kwargs: Any) -> FakeTargetGroup: from moto.ec2.exceptions import InvalidVPCIdError try: - self.ec2_backend.get_vpc(kwargs.get("vpc_id")) + self.ec2_backend.get_vpc(kwargs.get("vpc_id") or "") except InvalidVPCIdError: raise ValidationError( f"The VPC ID '{kwargs.get('vpc_id')}' is not found" @@ -1283,11 +1293,13 @@ def create_target_group(self, name: str, **kwargs: Any) -> FakeTargetGroup: f"Health check timeout '{healthcheck_timeout_seconds}' must be smaller than the interval '{healthcheck_interval_seconds}'" ) - arn = make_arn_for_target_group( - account_id=self.account_id, name=name, region_name=self.region_name - ) tags = kwargs.pop("tags", None) - target_group = FakeTargetGroup(name, arn, **kwargs) + target_group = FakeTargetGroup( + name=name, + account_id=self.account_id, + region_name=self.region_name, + **kwargs, + ) self.target_groups[target_group.arn] = target_group if tags: self.add_tags(resource_arns=[target_group.arn], tags=tags) diff --git a/tests/test_elbv2/test_elbv2_target_health.py b/tests/test_elbv2/test_elbv2_target_health.py index 47d68b5e30a3..dea198acf6c5 100644 --- a/tests/test_elbv2/test_elbv2_target_health.py +++ b/tests/test_elbv2/test_elbv2_target_health.py @@ -3,7 +3,7 @@ import boto3 import pytest -from botocore.exceptions import WaiterError +from botocore.exceptions import ClientError, WaiterError from . import elbv2_aws_verified @@ -94,9 +94,6 @@ def test_register_targets(target_is_aws=False): UserData=init_script, )[0].id - waiter = ec2_client.get_waiter("instance_running") - waiter.wait(InstanceIds=[instance_id1, instance_id2]) - load_balancer_arn = conn.create_load_balancer( Name=lb_name, Subnets=[subnet1.id, subnet2.id], @@ -124,9 +121,7 @@ def test_register_targets(target_is_aws=False): )["TargetGroups"][0]["TargetGroupArn"] # No targets registered yet - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) assert len(healths) == 0 listener_arn = conn.create_listener( @@ -141,6 +136,11 @@ def test_register_targets(target_is_aws=False): ], )["Listeners"][0]["ListenerArn"] + # Instances were started a while ago + # Make sure they are ready before trying to register them + waiter = ec2_client.get_waiter("instance_running") + waiter.wait(InstanceIds=[instance_id1, instance_id2]) + # Register target # Verify that they are healthy conn.register_targets( @@ -150,9 +150,8 @@ def test_register_targets(target_is_aws=False): waiter = conn.get_waiter("target_in_service") waiter.wait(TargetGroupArn=target_group_arn) - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) + assert { "Target": {"Id": instance_id1, "Port": 80}, "HealthCheckPort": "80", @@ -173,9 +172,7 @@ def test_register_targets(target_is_aws=False): waiter = conn.get_waiter("target_deregistered") waiter.wait(TargetGroupArn=target_group_arn, Targets=[{"Id": instance_id1}]) - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) assert len(healths) == 1 assert { "Target": {"Id": instance_id2, "Port": 80}, @@ -187,6 +184,9 @@ def test_register_targets(target_is_aws=False): healths = conn.describe_target_health( TargetGroupArn=target_group_arn, Targets=[{"Id": instance_id1, "Port": 80}] )["TargetHealthDescriptions"] + for h in healths: + # Skip attributes not implemented in Moto + h.pop("AdministrativeOverride", None) assert len(healths) == 1 assert { "Target": {"Id": instance_id1, "Port": 80}, @@ -206,9 +206,7 @@ def test_register_targets(target_is_aws=False): waiter = conn.get_waiter("target_in_service") waiter.wait(TargetGroupArn=target_group_arn) - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) assert { "Target": {"Id": instance_id1, "Port": 80}, "HealthCheckPort": "80", @@ -224,12 +222,24 @@ def test_register_targets(target_is_aws=False): waiter = ec2_client.get_waiter("instance_stopped") waiter.wait(InstanceIds=[instance_id1]) + # Verify we can't register an instance that has been stopped + with pytest.raises(ClientError) as exc: + conn.register_targets( + TargetGroupArn=target_group_arn, + Targets=[{"Id": instance_id1}], + ) + err = exc.value.response + assert err["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert err["Error"]["Code"] == "InvalidTarget" + assert ( + err["Error"]["Message"] + == f"The following targets are not in a running state and cannot be registered: '{instance_id1}'" + ) + # Instance is marked as 'unused' when stopped states = set() while states != {"healthy", "unused"}: - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) states = set([h["TargetHealth"]["State"] for h in healths]) if target_is_aws: @@ -254,13 +264,9 @@ def test_register_targets(target_is_aws=False): waiter.wait(InstanceIds=[instance_id1]) # Instance is removed when terminated - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) while len(healths) != 1: - healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ - "TargetHealthDescriptions" - ] + healths = describe_healths(target_group_arn) if target_is_aws: sleep(5) @@ -275,6 +281,9 @@ def test_register_targets(target_is_aws=False): healths = conn.describe_target_health( TargetGroupArn=target_group_arn, Targets=[{"Id": instance_id1, "Port": 80}] )["TargetHealthDescriptions"] + for h in healths: + # Skip attributes not implemented in Moto + h.pop("AdministrativeOverride", None) assert healths == [ { "Target": {"Id": instance_id1, "Port": 80}, @@ -336,6 +345,17 @@ def test_register_targets(target_is_aws=False): ec2_client.delete_internet_gateway(InternetGatewayId=igw_id) +def describe_healths(target_group_arn): + conn = boto3.client("elbv2", region_name="us-east-1") + healths = conn.describe_target_health(TargetGroupArn=target_group_arn)[ + "TargetHealthDescriptions" + ] + for h in healths: + # Skip attributes not implemented in Moto + h.pop("AdministrativeOverride", None) + return healths + + @elbv2_aws_verified() @pytest.mark.aws_verified def test_describe_unknown_targets(target_is_aws=False):