ECS - LongARN is enabled by default (#4762)

docs/docs/services/ecs.rst

@@ -12,6 +12,8 @@
 ecs
 ===
 
+.. autoclass:: moto.ecs.models.EC2ContainerServiceBackend
+
 |start-h3| Example usage |end-h3|
 
 .. sourcecode:: python

moto/ecs/models.py

@@ -716,6 +716,14 @@ class TaskSet(BaseObject):
 
 
 class EC2ContainerServiceBackend(BaseBackend):
+    """
+    ECS resources use the new ARN format by default.
+    Use the following environment variable to revert back to the old/short ARN format:
+    `MOTO_ECS_NEW_ARN=false`
+
+    AWS reference: https://aws.amazon.com/blogs/compute/migrating-your-amazon-ecs-deployment-to-the-new-arn-and-resource-id-format-2/
+    """
+
     def __init__(self, region_name):
         super().__init__()
         self.account_settings = dict()
@@ -1557,6 +1565,12 @@ class EC2ContainerServiceBackend(BaseBackend):
 
     @staticmethod
     def _parse_resource_arn(resource_arn):
+        match = re.match(
+            "^arn:aws:ecs:(?P<region>[^:]+):(?P<account_id>[^:]+):(?P<service>[^:]+)/(?P<cluster_id>[^:]+)/(?P<id>.*)$",
+            resource_arn,
+        )
+        if not match:
+            # maybe a short-format ARN
             match = re.match(
                 "^arn:aws:ecs:(?P<region>[^:]+):(?P<account_id>[^:]+):(?P<service>[^:]+)/(?P<id>.*)$",
                 resource_arn,
@@ -1784,12 +1798,10 @@ class EC2ContainerServiceBackend(BaseBackend):
         self.account_settings.pop(name, None)
 
     def enable_long_arn_for_name(self, name):
-        if settings.ecs_new_arn_format():
-            return True
         account = self.account_settings.get(name, None)
-        if account and account.value == "enabled":
+        if account and account.value == "disabled":
-            return True
             return False
+        return settings.ecs_new_arn_format()
 
 
 ecs_backends = BackendDict(EC2ContainerServiceBackend, "ecs")

moto/settings.py

@@ -43,7 +43,8 @@ def get_s3_default_key_buffer_size():
 
 
 def ecs_new_arn_format():
-    return os.environ.get("MOTO_ECS_NEW_ARN", "false").lower() == "true"
+    # True by default - only the value 'false' will return false
+    return os.environ.get("MOTO_ECS_NEW_ARN", "true").lower() != "false"
 
 
 def allow_unknown_region():

tests/test_core/test_settings.py

@@ -0,0 +1,27 @@
+import os
+import mock
+import pytest
+import sure  # noqa # pylint: disable=unused-import
+
+from moto import settings
+
+
+"""
+Sanity checks for interpretation of the MOTO_ECS_NEW_ARN-variable
+"""
+
+
+def test_default_is_true():
+    settings.ecs_new_arn_format().should.equal(True)
+
+
+@pytest.mark.parametrize("value", ["TrUe", "true", "invalid", "0", "1"])
+def test_anything_but_false_is_true(value):
+    with mock.patch.dict(os.environ, {"MOTO_ECS_NEW_ARN": value}):
+        settings.ecs_new_arn_format().should.equal(True)
+
+
+@pytest.mark.parametrize("value", ["False", "false", "faLse"])
+def test_only_false_is_false(value):
+    with mock.patch.dict(os.environ, {"MOTO_ECS_NEW_ARN": value}):
+        settings.ecs_new_arn_format().should.equal(False)

tests/test_ecs/test_ecs_account_settings.py

@@ -103,7 +103,7 @@ def test_delete_account_setting():
 @mock_ecs
 def test_put_account_setting_changes_service_arn():
     client = boto3.client("ecs", region_name="eu-west-1")
-    client.put_account_setting(name="serviceLongArnFormat", value="enabled")
+    client.put_account_setting(name="serviceLongArnFormat", value="disabled")
 
     _ = client.create_cluster(clusterName="dummy-cluster")
     _ = client.register_task_definition(
@@ -126,7 +126,15 @@ def test_put_account_setting_changes_service_arn():
         tags=[{"key": "ResourceOwner", "value": "Dummy"}],
     )
 
-    # Initial response is long
+    # Initial response is short (setting serviceLongArnFormat=disabled)
+    response = client.list_services(cluster="dummy-cluster", launchType="FARGATE")
+    service_arn = response["serviceArns"][0]
+    service_arn.should.equal(
+        "arn:aws:ecs:eu-west-1:{}:service/test-ecs-service".format(ACCOUNT_ID)
+    )
+
+    # Second invocation returns long ARN's by default, after deleting the preference
+    client.delete_account_setting(name="serviceLongArnFormat")
     response = client.list_services(cluster="dummy-cluster", launchType="FARGATE")
     service_arn = response["serviceArns"][0]
     service_arn.should.equal(
@@ -135,14 +143,6 @@ def test_put_account_setting_changes_service_arn():
         )
     )
 
-    # Second invocation returns short ARN's, after deleting the longArn-preference
-    client.delete_account_setting(name="serviceLongArnFormat")
-    response = client.list_services(cluster="dummy-cluster", launchType="FARGATE")
-    service_arn = response["serviceArns"][0]
-    service_arn.should.equal(
-        "arn:aws:ecs:eu-west-1:{}:service/test-ecs-service".format(ACCOUNT_ID)
-    )
-
 
 @mock_ec2
 @mock_ecs
@@ -162,19 +162,7 @@ def test_put_account_setting_changes_containerinstance_arn():
         ec2_utils.generate_instance_identity_document(test_instance)
     )
 
-    # Initial ARN should be short
+    # Initial ARN should be long
-    response = ecs_client.register_container_instance(
-        cluster=test_cluster_name, instanceIdentityDocument=instance_id_document
-    )
-    full_arn = response["containerInstance"]["containerInstanceArn"]
-    full_arn.should.match(
-        f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:container-instance/[a-z0-9-]+$"
-    )
-
-    # Now enable long-format
-    ecs_client.put_account_setting(
-        name="containerInstanceLongArnFormat", value="enabled"
-    )
     response = ecs_client.register_container_instance(
         cluster=test_cluster_name, instanceIdentityDocument=instance_id_document
     )
@@ -183,6 +171,18 @@ def test_put_account_setting_changes_containerinstance_arn():
         f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:container-instance/{test_cluster_name}/[a-z0-9-]+$"
     )
 
+    # Now disable long-format
+    ecs_client.put_account_setting(
+        name="containerInstanceLongArnFormat", value="disabled"
+    )
+    response = ecs_client.register_container_instance(
+        cluster=test_cluster_name, instanceIdentityDocument=instance_id_document
+    )
+    full_arn = response["containerInstance"]["containerInstanceArn"]
+    full_arn.should.match(
+        f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:container-instance/[a-z0-9-]+$"
+    )
+
 
 @mock_ec2
 @mock_ecs
@@ -217,20 +217,7 @@ def test_run_task_default_cluster_new_arn_format():
             }
         ],
     )
-    # Initial ARN is short-format
+    # Initial ARN is long-format
-    client.put_account_setting(name="taskLongArnFormat", value="disabled")
-    response = client.run_task(
-        launchType="FARGATE",
-        overrides={},
-        taskDefinition="test_ecs_task",
-        count=1,
-        startedBy="moto",
-    )
-    response["tasks"][0]["taskArn"].should.match(
-        f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:task/[a-z0-9-]+$"
-    )
-
-    # Enable long-format for the next task
     client.put_account_setting(name="taskLongArnFormat", value="enabled")
     response = client.run_task(
         launchType="FARGATE",
@@ -242,3 +229,16 @@ def test_run_task_default_cluster_new_arn_format():
     response["tasks"][0]["taskArn"].should.match(
         f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:task/{test_cluster_name}/[a-z0-9-]+$"
     )
+
+    # Enable short-format for the next task
+    client.put_account_setting(name="taskLongArnFormat", value="disabled")
+    response = client.run_task(
+        launchType="FARGATE",
+        overrides={},
+        taskDefinition="test_ecs_task",
+        count=1,
+        startedBy="moto",
+    )
+    response["tasks"][0]["taskArn"].should.match(
+        f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:task/[a-z0-9-]+$"
+    )

tests/test_ecs/test_ecs_boto3.py

@@ -520,7 +520,9 @@ def test_create_service():
     response["service"]["pendingCount"].should.equal(0)
     response["service"]["runningCount"].should.equal(0)
     response["service"]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service".format(
+            ACCOUNT_ID
+        )
     )
     response["service"]["serviceName"].should.equal("test_ecs_service")
     response["service"]["status"].should.equal("ACTIVE")
@@ -610,7 +612,9 @@ def test_create_service_scheduling_strategy():
     response["service"]["pendingCount"].should.equal(0)
     response["service"]["runningCount"].should.equal(0)
     response["service"]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service".format(
+            ACCOUNT_ID
+        )
     )
     response["service"]["serviceName"].should.equal("test_ecs_service")
     response["service"]["status"].should.equal("ACTIVE")
@@ -657,10 +661,14 @@ def test_list_services():
     unfiltered_response = client.list_services(cluster="test_ecs_cluster")
     len(unfiltered_response["serviceArns"]).should.equal(2)
     unfiltered_response["serviceArns"][0].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service1".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service1".format(
+            ACCOUNT_ID
+        )
     )
     unfiltered_response["serviceArns"][1].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+            ACCOUNT_ID
+        )
     )
 
     filtered_response = client.list_services(
@@ -668,7 +676,9 @@ def test_list_services():
     )
     len(filtered_response["serviceArns"]).should.equal(1)
     filtered_response["serviceArns"][0].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service1".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service1".format(
+            ACCOUNT_ID
+        )
     )
 
 
@@ -715,16 +725,22 @@ def test_describe_services():
         cluster="test_ecs_cluster",
         services=[
             "test_ecs_service1",
-            "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID),
+            "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+                ACCOUNT_ID
+            ),
         ],
     )
     len(response["services"]).should.equal(2)
     response["services"][0]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service1".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service1".format(
+            ACCOUNT_ID
+        )
     )
     response["services"][0]["serviceName"].should.equal("test_ecs_service1")
     response["services"][1]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+            ACCOUNT_ID
+        )
     )
     response["services"][1]["serviceName"].should.equal("test_ecs_service2")
 
@@ -745,7 +761,9 @@ def test_describe_services():
         cluster="test_ecs_cluster",
         services=[
             "test_ecs_service1",
-            "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID),
+            "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+                ACCOUNT_ID
+            ),
         ],
         include=["TAGS"],
     )
@@ -830,17 +848,23 @@ def test_describe_services_scheduling_strategy():
         cluster="test_ecs_cluster",
         services=[
             "test_ecs_service1",
-            "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID),
+            "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+                ACCOUNT_ID
+            ),
             "test_ecs_service3",
         ],
     )
     len(response["services"]).should.equal(3)
     response["services"][0]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service1".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service1".format(
+            ACCOUNT_ID
+        )
     )
     response["services"][0]["serviceName"].should.equal("test_ecs_service1")
     response["services"][1]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service2".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service2".format(
+            ACCOUNT_ID
+        )
     )
     response["services"][1]["serviceName"].should.equal("test_ecs_service2")
 
@@ -1038,7 +1062,9 @@ def test_delete_service():
     response["service"]["pendingCount"].should.equal(0)
     response["service"]["runningCount"].should.equal(0)
     response["service"]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service".format(
+            ACCOUNT_ID
+        )
     )
     response["service"]["serviceName"].should.equal("test_ecs_service")
     response["service"]["status"].should.equal("ACTIVE")
@@ -1085,7 +1111,9 @@ def test_delete_service_force():
     response["service"]["pendingCount"].should.equal(0)
     response["service"]["runningCount"].should.equal(0)
     response["service"]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service".format(
+            ACCOUNT_ID
+        )
     )
     response["service"]["serviceName"].should.equal("test_ecs_service")
     response["service"]["status"].should.equal("ACTIVE")
@@ -1176,7 +1204,8 @@ def test_register_container_instance():
     arn_part[0].should.equal(
         "arn:aws:ecs:us-east-1:{}:container-instance".format(ACCOUNT_ID)
     )
-    arn_part[1].should.equal(str(UUID(arn_part[1])))
+    arn_part[1].should.equal("test_ecs_cluster")
+    arn_part[2].should.equal(str(UUID(arn_part[2])))
     response["containerInstance"]["status"].should.equal("ACTIVE")
     len(response["containerInstance"]["registeredResources"]).should.equal(4)
     len(response["containerInstance"]["remainingResources"]).should.equal(4)
@@ -1356,7 +1385,7 @@ def test_describe_container_instances():
 
         test_instance_arns.append(response["containerInstance"]["containerInstanceArn"])
 
-    test_instance_ids = list(map((lambda x: x.split("/")[1]), test_instance_arns))
+    test_instance_ids = list(map((lambda x: x.split("/")[-1]), test_instance_arns))
     response = ecs_client.describe_container_instances(
         cluster=test_cluster_name, containerInstances=test_instance_ids
     )
@@ -1424,7 +1453,7 @@ def test_update_container_instances_state():
 
         test_instance_arns.append(response["containerInstance"]["containerInstanceArn"])
 
-    test_instance_ids = list(map((lambda x: x.split("/")[1]), test_instance_arns))
+    test_instance_ids = list(map((lambda x: x.split("/")[-1]), test_instance_arns))
     response = ecs_client.update_container_instances_state(
         cluster=test_cluster_name,
         containerInstances=test_instance_ids,
@@ -1641,7 +1670,7 @@ def test_run_task_default_cluster():
     len(response["tasks"]).should.equal(2)
     response["tasks"][0].should.have.key("launchType").equals("FARGATE")
     response["tasks"][0]["taskArn"].should.match(
-        "arn:aws:ecs:us-east-1:{}:task/[a-z0-9-]+$".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:task/default/[a-z0-9-]+$".format(ACCOUNT_ID)
     )
     response["tasks"][0]["clusterArn"].should.equal(
         "arn:aws:ecs:us-east-1:{}:cluster/default".format(ACCOUNT_ID)
@@ -1790,7 +1819,7 @@ def test_start_task():
         "arn:aws:ecs:us-east-1:{}:task-definition/test_ecs_task:1".format(ACCOUNT_ID)
     )
     response["tasks"][0]["containerInstanceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{0}:container-instance/{1}".format(
+        "arn:aws:ecs:us-east-1:{0}:container-instance/test_ecs_cluster/{1}".format(
             ACCOUNT_ID, container_instance_id
         )
     )
@@ -2681,7 +2710,9 @@ def test_create_service_load_balancing():
     response["service"]["pendingCount"].should.equal(0)
     response["service"]["runningCount"].should.equal(0)
     response["service"]["serviceArn"].should.equal(
-        "arn:aws:ecs:us-east-1:{}:service/test_ecs_service".format(ACCOUNT_ID)
+        "arn:aws:ecs:us-east-1:{}:service/test_ecs_cluster/test_ecs_service".format(
+            ACCOUNT_ID
+        )
     )
     response["service"]["serviceName"].should.equal("test_ecs_service")
     response["service"]["status"].should.equal("ACTIVE")
@@ -2779,8 +2810,14 @@ def test_list_tags_for_resource_ecs_service():
 
 
 @mock_ecs
-def test_ecs_service_tag_resource():
+@pytest.mark.parametrize("long_arn", ["disabled", "enabled"])
+def test_ecs_service_tag_resource(long_arn):
+    """
+    Tagging does some weird ARN parsing - ensure it works with both long and short formats
+    """
     client = boto3.client("ecs", region_name="us-east-1")
+    client.put_account_setting(name="serviceLongArnFormat", value=long_arn)
+
     _ = client.create_cluster(clusterName="test_ecs_cluster")
     _ = client.register_task_definition(
         family="test_ecs_task",