Route53: change_tags_for_resource(): Validate ZoneId parameter (#8984)

Author: bblommers

moto/route53/models.py

@@ -68,7 +68,7 @@ def create_route53_zone_id(  # type: ignore
     tags: Tags = None,
 ) -> str:
     # New ID's look like this Z1RWWTK7Y8UDDQ
-    return "".join([random.choice(ROUTE53_ID_CHOICE) for _ in range(0, 15)])
+    return "".join([random.choice(ROUTE53_ID_CHOICE) for _ in range(0, 22)])
 
 
 def create_route53_caller_reference() -> str:
@@ -656,7 +656,12 @@ def disassociate_vpc_from_hosted_zone(self, zone_id: str, vpcid: str) -> FakeZon
         zone.delete_vpc(vpcid)
         return zone
 
-    def change_tags_for_resource(self, resource_id: str, tags: Any) -> None:
+    def change_tags_for_resource(
+        self, resource_type: str, resource_id: str, tags: Any
+    ) -> None:
+        if resource_type == "hostedzone" and resource_id not in self.zones:
+            raise NoSuchHostedZone(host_zone_id=resource_id)
+
         if "Tag" in tags:
             if isinstance(tags["Tag"], list):
                 for tag in tags["Tag"]:

moto/route53/responses.py

@@ -10,7 +10,7 @@
 from moto.core.common_types import TYPE_RESPONSE
 from moto.core.responses import BaseResponse
 from moto.core.utils import iso_8601_datetime_with_milliseconds
-from moto.route53.exceptions import InvalidChangeBatch
+from moto.route53.exceptions import InvalidChangeBatch, InvalidInput
 from moto.route53.models import Route53Backend, route53_backends
 
 XMLNS = "https://route53.amazonaws.com/doc/2013-04-01/"
@@ -350,6 +350,14 @@ def list_or_change_tags_for_resource_request(  # type: ignore[return]
         assert id_matcher
         type_ = id_matcher.group(1)
         id_ = unquote(id_matcher.group(2))
+        if type_ == "hostedzone" and len(id_) > 32:
+            # From testing, it looks like Route53 creates ID's that are either 21 or 22 characters long
+            # In practice, this error will typically appear when passing in the full ID: `/hostedzone/{id}`
+            # Users should pass in {id} instead
+            # NOTE: we don't know (yet) what kind of validation (if any) is in place for type_==healthcheck.
+            raise InvalidInput(
+                f"1 validation error detected: Value '{id_}' at 'resourceId' failed to satisfy constraint: Member must have length less than or equal to 32"
+            )
 
         if request.method == "GET":
             tags = self.backend.list_tags_for_resource(id_)
@@ -368,7 +376,7 @@ def list_or_change_tags_for_resource_request(  # type: ignore[return]
             elif "RemoveTagKeys" in tags:
                 tags = tags["RemoveTagKeys"]  # type: ignore
 
-            self.backend.change_tags_for_resource(id_, tags)
+            self.backend.change_tags_for_resource(type_, id_, tags)
             template = Template(CHANGE_TAGS_FOR_RESOURCE_RESPONSE)
             return 200, headers, template.render()
 

tests/test_route53/test_route53.py

@@ -478,7 +478,7 @@ def test_list_or_change_tags_for_resource_request():
             "HealthThreshold": 123,
         },
     )
-    healthcheck_id = health_check["HealthCheck"]["Id"]
+    healthcheck_id = health_check["HealthCheck"]["Id"].replace("/hostedzone/", "")
 
     # confirm this works for resources with zero tags
     response = conn.list_tags_for_resource(
@@ -558,11 +558,11 @@ def test_list_tags_for_resources():
     zone1 = conn.create_hosted_zone(
         Name="testdns1.aws.com", CallerReference=str(hash("foo"))
     )
-    zone1_id = zone1["HostedZone"]["Id"]
+    zone1_id = zone1["HostedZone"]["Id"].replace("/hostedzone/", "")
     zone2 = conn.create_hosted_zone(
         Name="testdns2.aws.com", CallerReference=str(hash("bar"))
     )
-    zone2_id = zone2["HostedZone"]["Id"]
+    zone2_id = zone2["HostedZone"]["Id"].replace("/hostedzone/", "")
 
     # Create two healthchecks
     health_check1 = conn.create_health_check(

tests/test_route53/test_route53_hosted_zone_ids.py

@@ -0,0 +1,115 @@
+"""
+Route53 has some idiosyncrasies related to HostedZone ids
+Some operations return `/hosted_zone/{id}`
+Other operations return `{id}`
+Some operations expect `/hosted_zone/{id}`
+Other operations expect `{id}`
+Some operations allow both (probably..)
+
+The tests in this file are purely to test the different scenarios.
+
+Note that all tests are verified against AWS.
+If you want to run the tests against AWS yourself, make sure `TEST_DOMAIN_NAME` is changed to a domain name that's known to AWS.
+"""
+
+from functools import wraps
+from uuid import uuid4
+
+import boto3
+import pytest
+from botocore.exceptions import ClientError
+
+from moto import mock_aws
+from tests import allow_aws_request
+
+TEST_DOMAIN_NAME = "mototests.click"
+TEST_HOSTED_ZONE_NAME = f"test.{TEST_DOMAIN_NAME}"
+
+
+def route53_aws_verified(func):
+    @wraps(func)
+    def pagination_wrapper(**kwargs):
+        def create_hosted_zone():
+            client = boto3.client("route53", "us-east-1")
+
+            hosted_zone = client.create_hosted_zone(
+                Name=TEST_HOSTED_ZONE_NAME, CallerReference=str(uuid4())
+            )["HostedZone"]
+
+            kwargs["hosted_zone"] = hosted_zone
+
+            try:
+                return func(**kwargs)
+            finally:
+                client.delete_hosted_zone(Id=hosted_zone["Id"])
+
+        if allow_aws_request():
+            return create_hosted_zone()
+        else:
+            with mock_aws():
+                return create_hosted_zone()
+
+    return pagination_wrapper
+
+
+@pytest.mark.aws_verified
+@route53_aws_verified
+def test_hosted_zone_id_in_change_tags(hosted_zone=None):
+    client = boto3.client("route53", "us-east-1")
+
+    full_zone_id = hosted_zone["Id"]
+    assert full_zone_id.startswith("/hostedzone/")
+
+    # Can't use full ID here
+    with pytest.raises(ClientError) as exc:
+        client.change_tags_for_resource(
+            ResourceType="hostedzone",
+            ResourceId=full_zone_id,
+            AddTags=[{"Key": "foo", "Value": "bar"}],
+        )
+    err = exc.value.response["Error"]
+    assert err["Code"] == "InvalidInput"
+    assert (
+        err["Message"]
+        == f"1 validation error detected: Value '{full_zone_id}' at 'resourceId' failed to satisfy constraint: Member must have length less than or equal to 32"
+    )
+
+    # if we naively limit the id-length to 32, we get a NoSuchHostedZone-exception (as expected)
+    with pytest.raises(ClientError) as exc:
+        client.change_tags_for_resource(
+            ResourceType="hostedzone",
+            ResourceId=full_zone_id[0:32],
+            AddTags=[{"Key": "foo", "Value": "bar"}],
+        )
+    err = exc.value.response["Error"]
+    assert err["Code"] == "NoSuchHostedZone"
+
+    # Need to strip the '/hosted_zone/'-prefix
+    id_without_prefix = full_zone_id.replace("/hostedzone/", "")
+    client.change_tags_for_resource(
+        ResourceType="hostedzone",
+        ResourceId=id_without_prefix,
+        AddTags=[{"Key": "foo", "Value": "bar"}],
+    )
+
+    # Test retrieval of tags with full ID
+    with pytest.raises(ClientError) as exc:
+        client.list_tags_for_resource(
+            ResourceType="hostedzone", ResourceId=full_zone_id
+        )
+    err = exc.value.response["Error"]
+    assert err["Code"] == "InvalidInput"
+    assert (
+        err["Message"]
+        == f"1 validation error detected: Value '{full_zone_id}' at 'resourceId' failed to satisfy constraint: Member must have length less than or equal to 32"
+    )
+
+    # Test retrieval of tags with stripped ID
+    tags = client.list_tags_for_resource(
+        ResourceType="hostedzone", ResourceId=id_without_prefix
+    )["ResourceTagSet"]
+    assert tags == {
+        "ResourceId": id_without_prefix,
+        "ResourceType": "hostedzone",
+        "Tags": [{"Key": "foo", "Value": "bar"}],
+    }