Repository: getmoto/moto
Pull request: fix(describe_trails): trail not multiregion case
From https://github.com/spulec/moto/issues/5604 we have realised that there is a missing condition when listing the trails from `describe_trail`.

If we don't include the condition:
```
if trail.is_multi_region or trail.region_name == backend.region_name:
```
Single multiregion trails will be shown in another regions when launching `describe_trail`

To test its effect please refer to the script included into the issue description

Problem statement:
Cloudtrail describe_trails returning wrong info
Hi,

I'am trying to mock trail creation and describing those trails, and I realise that something is wrong when describing the trails.
When you create a single region trail for example with:

`aws cloudtrail create-trail --name "single-region-trail-test" --s3-bucket-name "test" --no-is-multi-region-trail --region eu-west-1`

and then if you try to describe trails with:

`aws cloudtrail describe-trail --region us-west-2`

You should not get any results, since the created trail is not multiregion or organization trail.

If you replicate this steps with moto, using `mock_s3 ` and `mock_cloudtrail` decorators you'll get data from the `describe_trail` api call from regions that should be empty.

You can see this behaviour with the following code

```
from boto3 import client, session
from moto import mock_cloudtrail, mock_s3

@mock_cloudtrail
@mock_s3
def test():
    aws_session = session.Session(profile_name=None)
    cloudtrail_client_us_east_1 = aws_session.client("cloudtrail", region_name="us-east-1")
    s3_client_us_east_1 = client("s3", region_name="us-east-1")
    cloudtrail_client_eu_west_1 = aws_session.client("cloudtrail", region_name="eu-west-1")
    s3_client_eu_west_1 = aws_session.client("s3", region_name="eu-west-1")
    trail_name_us = "trail_test_us"
    bucket_name_us = "bucket_test_us"
    trail_name_eu = "trail_test_eu"
    bucket_name_eu = "bucket_test_eu"
    s3_client_us_east_1.create_bucket(Bucket=bucket_name_us)
    s3_client_eu_west_1.create_bucket(Bucket=bucket_name_eu, CreateBucketConfiguration={
        'LocationConstraint': 'eu-west-1'
            })
    cloudtrail_client_us_east_1.create_trail(
            Name=trail_name_us, S3BucketName=bucket_name_us, IsMultiRegionTrail=False
            )
    cloudtrail_client_eu_west_1.create_trail(
                Name=trail_name_eu, S3BucketName=bucket_name_eu, IsMultiRegionTrail=False
            )

    regions = ["af-south-1",
    "ap-east-1",
    "ap-northeast-1",
    "ap-northeast-2",
    "eu-west-3",
    "me-central-1",
    "sa-east-1",
    "us-west-2",
    "ap-northeast-3",
    "ap-southeast-2",
    "ap-southeast-3",
    "ca-central-1",
    "eu-north-1",
    "eu-south-1",
    "eu-west-1",
    "us-east-2",
    "ap-south-1",
    "ap-southeast-1",
    "eu-central-1",
    "eu-west-2",
    "me-south-1",
    "us-east-1",
    "us-west-1"]

    for region in regions:
        cloudtrail_describe_client = aws_session.client("cloudtrail", region_name=region)
        list_of_trails=cloudtrail_describe_client.describe_trails()["trailList"]
        print("REGION:" + region)
        print("LIST OF TRAILS:")
        print(list_of_trails)
        # if the region is the one with the trail created 
        if region == "us-east-1" or region == "eu-west-1":
            assert len(list_of_trails) == 1
        else:
            assert len(list_of_trails) == 0

test()
```

The output is

```
REGION:af-south-1
LIST OF TRAILS:
[{'Name': 'trail_test_us', 'S3BucketName': 'bucket_test_us', 'IncludeGlobalServiceEvents': True, 'IsMultiRegionTrail': False, 'HomeRegion': 'us-east-1', 'TrailARN': 'arn:aws:cloudtrail:us-east-1:123456789012:trail/trail_test_us', 'LogFileValidationEnabled': False, 'HasCustomEventSelectors': False, 'HasInsightSelectors': False, 'IsOrganizationTrail': False}, 
{'Name': 'trail_test_eu', 'S3BucketName': 'bucket_test_eu', 'IncludeGlobalServiceEvents': True, 'IsMultiRegionTrail': False, 'HomeRegion': 'eu-west-1', 'TrailARN': 'arn:aws:cloudtrail:eu-west-1:123456789012:trail/trail_test_eu', 'LogFileValidationEnabled': False, 'HasCustomEventSelectors': False, 'HasInsightSelectors': False, 'IsOrganizationTrail': False}]
Traceback (most recent call last):
  File "", line 64, in <module>
    test()
  File "", line 111, in wrapper
    result = func(*args, **kwargs)
  File "", line 111, in wrapper
    result = func(*args, **kwargs)
  File "", line 62, in test
    assert len(list_of_trails) == 0
AssertionError
```

As it can be seen it is retrieving both single region trails querying from region `af-south-1` which is wrong

Hints from issue discussion:
Trying to debug  launching the example script for the region `eu-west-3` it seems that when initialising that when the backends are being initialised here:
https://github.com/spulec/moto/blob/master/moto/cloudtrail/models.py#L320

It creates a single backend for each found trail and the passed region

![Screenshot 2022-10-26 at 13 12 02](https://user-images.githubusercontent.com/59198746/198017904-05e9343d-098a-4e4a-a861-7d700e0ee91a.png)

When it should only create a backend for `eu-west-3`
Proposed changes here https://github.com/spulec/moto/pull/5607

Commit message: fix(tests): describe_trail fixed tests

Patch to review:
diff --git a/moto/cloudtrail/models.py b/moto/cloudtrail/models.py
index aa604f6e5702..7be9b3a42a04 100644
--- a/moto/cloudtrail/models.py
+++ b/moto/cloudtrail/models.py
@@ -318,7 +318,12 @@ def describe_trails(self, include_shadow_trails):
         if include_shadow_trails:
             current_account = cloudtrail_backends[self.account_id]
             for backend in current_account.values():
-                all_trails.extend(backend.trails.values())
+                for trail in backend.trails.values():
+                    if (
+                        trail.is_multi_region
+                        or trail.region_name == backend.region_name
+                    ):
+                        all_trails.append(trail)
         else:
             all_trails.extend(self.trails.values())
         return all_trails


Code review comments:
diff_hunk: @@ -318,7 +318,12 @@ def describe_trails(self, include_shadow_trails):
         if include_shadow_trails:
             current_account = cloudtrail_backends[self.account_id]
             for backend in current_account.values():
-                all_trails.extend(backend.trails.values())
+                for trail in backend.trails.values():
+                    if (
+                        trail.is_multi_region
+                        or trail.region_name == backend.region_name
path: moto/cloudtrail/models.py
text: @user1:
```suggestion
                        or trail.region_name == self.region_name
```
The current `trail` is coming from `backend`, so this will always return True at the moment.

Would you mind adding a test case for this as well?
We already have `tests/test_cloudwatch/test_cloudwatch.py::test_describe_trails_without_shadowtrails` that tests this functionality, but I'm expecting that to fail with this fix. Ideally we cover all all possibilities in the existing tests, i.e. we create trails with MultiRegion=True and with MultiRegion=False, in multiple regions, and then verify only the correct trails are returned.

@user2:
Hi @user1, you're right. I've modified the CloudTrail test to address this change, because with this new logic the MultiRegion/NotMultiRegion and Home trails are not correctly handled.

Thanks!



@author:
@user1  We have not reviewed the whole cloudtrail service, but maybe this issue is affecting other parts.
It is really tricky how AWS handle the `describe-trails` api call when listing multiregion and organization trails.
If you need help let us know.

Thanks !!