ELBv2: register_targets() now validates that instances are running (#…

…8350)

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")

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)

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):