CLD-485: only create cloud trail if nesessary

aws/sam-app/README.md

@@ -16,10 +16,18 @@ and records in your AWS account.
 |   Route 53   |  -->  |  CloudTrail  |  -->  |  EventBridge |  -->  |      SQS     |  -->  |    Lambda    |  -->  |      NS1     |
 +--------------+       +--------------+       +--------------+       +--------------+       +--------------+       +--------------+
 
-When zones and records are created, changed, and deleted, events are automatically recorded in CloudTrail. The installed EventBridge rule watches for these events and queues them up 
+When zones and records are created, changed, and deleted, events are automatically recorded in CloudTrail. The installed EventBridge rule watches for these events and queues them up
 for the Lambda to take on the next leg. Processing of the messages for the events is minimal. The message is taken in its entirety, lightly wrapped, and sent
 off across the Internet to the NS1 CloudSync REST API endpoint.  Events are then processed asynchronously in NS1 Connect.
-
+
+## Parameters
+
+| Parameter | Default | Description |
+|---|---|---|
+| `NS1APIKey` | *(required)* | Your NS1 Connect account API key. |
+| `CreateCloudTrail` | `false` | Set to `true` only if your AWS account has no existing active multi-region CloudTrail trail. Most accounts already have a default trail (`management-events`); leave as `false` to avoid creating a duplicate trail and incurring unnecessary charges. |
+| `CloudTrailName` | `NS1CloudSyncTrail` | **Only used when `CreateCloudTrail` is `true`** — leave as default if `CreateCloudTrail` is `false`. Name for the new trail created by this stack. Do not set this to the name of an existing trail. |
+
 ## Security and permissions
 You will need to provide an NS1 API key when installing the stack. A secret in AWS Secrets Manager is created for this key. When the `dns_updates` Lambda initializes it will request the key
 from Secrets Manager and store it in memory for the lifetime of the Lambda execution environment.

aws/sam-app/src/configure/app.py

@@ -8,6 +8,8 @@
 secrets_manager_client = boto3.client('secretsmanager')
 cloudformation_client = boto3.client('cloudformation')
 logs_client = boto3.client('logs')
+cloudtrail_client = boto3.client('cloudtrail')
+s3_resource = boto3.resource('s3')
 
 # env variables
 ns1_api_key = os.environ['NS1_API_KEY']
@@ -39,6 +41,8 @@ def configure_application(event, context):
     elif cloud_sync_api_key:
         secret_handler.upsert(CS_API_KEY_NAME, cloud_sync_api_key)
 
+    retained_bucket = ""
+
     try:
         request_type = event['RequestType']
         if request_type == 'Create':
@@ -48,17 +52,90 @@ def configure_application(event, context):
             else:
                 secret_handler.upsert(NS1_API_KEY_NAME, ns1_api_key)
 
+        elif request_type == 'Update':
+            # If CreateCloudTrail is transitioning from true → false (or upgrading
+            # from 0.4.3 where CreateCloudTrail did not exist and the trail was always
+            # created), stop and delete the CloudSync-managed trail so it stops writing
+            # to the S3 bucket.
+            # The S3 bucket is intentionally NOT emptied or deleted here. Bucket deletion
+            # may take a long time depending on the size of the bucket, and may cause the
+            # CloudFormation stack update to timeout or fail if the deletion takes too long.
+            # The customer is warned via CloudWatch logs to empty and delete the bucket
+            # manually once they have verified DNS sync is working.
+            #
+            # Note: OldResourceProperties will not contain CreateCloudTrail when
+            # upgrading from 0.4.3 (the parameter did not exist in that version and
+            # the trail was always created). Defaulting to 'true' here means the
+            # upgrade path correctly detects the true→false transition and cleans up
+            # the old trail. This is NOT the customer-facing default —
+            # the template parameter default is 'false'.
+            old_create_trail = event['OldResourceProperties'].get('CreateCloudTrail', 'true')
+            # new_create_trail uses 'true' as fallback only for safety — in practice
+            # this key will always be present in ResourceProperties from 0.4.4 onwards.
+            new_create_trail = event['ResourceProperties'].get('CreateCloudTrail', 'true')
+
+            if old_create_trail == 'true' and new_create_trail == 'false':
+                trail_name = event['OldResourceProperties'].get('CloudTrailName', 'NS1CloudSyncTrail')
+
+                # CloudTrailBucketName is not present in OldResourceProperties when
+                # upgrading from 0.4.3 — find the bucket by its known name prefix instead.
+                bucket_name = event['OldResourceProperties'].get('CloudTrailBucketName')
+                if not bucket_name:
+                    s3_client = boto3.client('s3')
+                    buckets = s3_client.list_buckets().get('Buckets', [])
+                    for b in buckets:
+                        if b['Name'].startswith('cloudsync-trail-bucket-'):
+                            bucket_name = b['Name']
+                            break
+
+                # Stop and delete the trail so it stops writing to the S3 bucket.
+                # Safety guard: only delete trails with names that match known
+                # CloudSync-created trail names. We never delete trails that were
+                # not created by this stack.
+                # Known CloudSync trail names across all versions:
+                #   - NS1CloudSyncTrail (default from 0.4.x)
+                #   - CloudSyncTrail (used in older deployments)
+                # Any other name (e.g. management-events, org trails) is skipped.
+                cloudsync_trail_names = {'NS1CloudSyncTrail', 'CloudSyncTrail'}
+                if trail_name in cloudsync_trail_names:
+                    try:
+                        cloudtrail_client.stop_logging(Name=trail_name)
+                        cloudtrail_client.delete_trail(Name=trail_name)
+                        print(f"Deleted CloudSync-managed trail: {trail_name}")
+                    except cloudtrail_client.exceptions.TrailNotFoundException:
+                        pass
+                else:
+                    print(f"Skipping deletion of trail not managed by CloudSync: {trail_name}")
+
+                # The S3 bucket is intentionally retained. Emptying it synchronously
+                # risks exceeding the Lambda timeout on large buckets and leaving the
+                # stack in UPDATE_ROLLBACK_FAILED. The trail has been stopped so no
+                # new objects will be written. The customer must empty and delete the
+                # bucket manually.
+                if bucket_name:
+                    retained_bucket = bucket_name
+                    print(
+                        f"WARNING: CloudTrail log bucket '{bucket_name}' has been retained. "
+                        f"The trail has been stopped — no new logs will be written. "
+                        f"Please empty and delete this bucket manually once you have "
+                        f"verified that DNS sync is working correctly. "
+                        f"You can do this from the S3 console or with the AWS CLI: "
+                        f"aws s3 rm s3://{bucket_name} --recursive && "
+                        f"aws s3api delete-bucket --bucket {bucket_name}"
+                    )
+
         elif request_type == 'Delete':
             # clean up secrets stored in Secrets Manager
             secret_handler.delete(NS1_API_KEY_NAME)
             secret_handler.delete(CS_API_KEY_NAME)
             secret_handler.delete(ACCESS_TOKEN_NAME)
             secret_handler.delete(REFRESH_TOKEN_NAME)
 
-            # empty CloudTrail bucket and remove it
-            s3_client = boto3.resource('s3')
-            bucket = s3_client.Bucket(event['ResourceProperties']['CloudTrailBucketName'])
-            bucket.objects.all().delete()
+            # empty CloudTrail bucket and remove it (only if the stack created one)
+            bucket_name = event['ResourceProperties'].get('CloudTrailBucketName')
+            if bucket_name:
+                bucket = s3_resource.Bucket(bucket_name)
+                bucket.objects.all().delete()
 
             # clean up log groups
             delete_log_groups()
@@ -68,7 +145,9 @@ def configure_application(event, context):
         response_data = build_response(event, 'FAILED', {"foo": "bar"}, reason=str(err))
 
     else:
-        response_data = build_response(event, 'SUCCESS', {"foo": "bar"})
+        response_data = build_response(event, 'SUCCESS', {
+            "RetainedCloudTrailBucket": retained_bucket,
+        })
 
     # Respond to Cloudformation to let it know we are done
     response_url = event['ResponseURL']

aws/sam-app/template.yaml

@@ -21,17 +21,44 @@ Metadata:
     ReadmeUrl: ./README.md
     Labels: [serverless, NS1, Route53, DNS]
     HomePageUrl: https://github.com/ns1/cloudsync
-    SemanticVersion: 0.4.3
-    SourceCodeUrl: https://github.com/ns1/cloudsync/tree/0.4.3
+    SemanticVersion: 0.4.4
+    SourceCodeUrl: https://github.com/ns1/cloudsync/tree/0.4.4
 
 Parameters:
+  CreateCloudTrail:
+    Type: String
+    Default: "false"
+    AllowedValues:
+      - "true"
+      - "false"
+    Description: >
+      Controls whether this stack creates a dedicated CloudTrail trail.
+      Set to "false" if your AWS account already has an existing active CloudTrail
+      trail. The trail must be capturing Route 53 management events and delivering
+      them to EventBridge. If DNS sync stops working after setting this to "false",
+      your existing trail may not be configured correctly - set to "true" to have
+      this stack create a dedicated trail with the correct settings.
+      Set to "true" if you are unsure or if your account has no existing trail.
   CloudTrailName:
     Type: String
     Default: NS1CloudSyncTrail
+    AllowedPattern: "^(?!management-events$)(?!aws-controltower-BaselineCloudTrail$)(?!AccountTrail$).+"
+    ConstraintDescription: >-
+      CloudTrailName must not be the name of an existing system or organization trail
+      (e.g. management-events, aws-controltower-BaselineCloudTrail, AccountTrail).
+      Leave as default (NS1CloudSyncTrail) unless you have a specific reason to change it.
+    Description: >-
+      ONLY USED when CreateCloudTrail is "true" — ignored otherwise, leave as default.
+      Name for the new CloudTrail trail created by this stack.
+      WARNING: Do not set this to the name of an existing trail such as
+      "management-events" — this stack will delete the trail when it is
+      removed or when CreateCloudTrail is changed to "false".
   NS1APIKey:
     Type: String
     Description: NS1 account API key.
 
+Conditions:
+  ShouldCreateTrail: !Equals [!Ref CreateCloudTrail, "true"]
 
 Resources:
   UpdatesDeadLetterQueue:
@@ -47,7 +74,8 @@ Resources:
   # KMS key for CloudTrail S3 bucket.
   TrailKMSKey:
     Type: AWS::KMS::Key
-    Properties: 
+    Condition: ShouldCreateTrail
+    Properties:
       Enabled: true
       KeySpec: SYMMETRIC_DEFAULT
       KeyUsage: ENCRYPT_DECRYPT
@@ -123,13 +151,15 @@ Resources:
   # S3 bucket for CloudTrail.
   TrailS3Bucket:
     Type: AWS::S3::Bucket
+    Condition: ShouldCreateTrail
     DeletionPolicy: Delete
     Properties:
       BucketName: !Join ['-', ['cloudsync-trail-bucket', !Select [4, !Split ['-', !Select [2, !Split ['/', !Ref AWS::StackId]]]]]]
 
   # Bucket policy to allow CloudTrail access to the bucket.
   TrailBucketPolicy:
     Type: AWS::S3::BucketPolicy
+    Condition: ShouldCreateTrail
     Properties:
       Bucket: !Ref TrailS3Bucket
       PolicyDocument:
@@ -156,9 +186,12 @@ Resources:
               StringEquals:
                 s3:x-amz-acl: bucket-owner-full-control
 
-  # There must be a trail in order for EventBridge to capture events from Route 53.
+  # A trail is only created when no existing trail is present in the account.
+  # EventBridge receives Route 53 management events from any active trail, so the
+  # default AWS trail (management-events) is sufficient for most accounts.
   Trail:
     Type: AWS::CloudTrail::Trail
+    Condition: ShouldCreateTrail
     DependsOn: TrailBucketPolicy
     Properties:
       S3BucketName: !Ref TrailS3Bucket
@@ -361,20 +394,27 @@ Resources:
                   - !Sub 'arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/*SnapshotListFunction*'
                   - !Sub 'arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/*TriggerStateMachineFunction*'
                   - !Sub 'arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/*DNSUpdateFunction*'
+              - Effect: Allow
+                Action:
+                  - s3:ListAllMyBuckets
+                Resource: '*'
               - Effect: Allow
                 Action:
                   - s3:DeleteObject
                   - s3:GetObject
                   - s3:ListBucket
-                Resource: !GetAtt TrailS3Bucket.Arn
+                Resource: !Sub 'arn:aws:s3:::cloudsync-trail-bucket-*'
               - Effect: Allow
                 Action:
                   - s3:DeleteObject
                   - s3:GetObject
                   - s3:ListBucket
-                Resource: !Sub 
-                  - arn:aws:s3:::${Bucket}/*
-                  - Bucket: !Ref TrailS3Bucket
+                Resource: !Sub 'arn:aws:s3:::cloudsync-trail-bucket-*/*'
+              - Effect: Allow
+                Action:
+                  - cloudtrail:StopLogging
+                  - cloudtrail:DeleteTrail
+                Resource: !Sub 'arn:aws:cloudtrail:us-east-1:${AWS::AccountId}:trail/*'
 
   ConfigureFunction:
     Type: AWS::Serverless::Function
@@ -396,7 +436,12 @@ Resources:
     Type: Custom::ConfigureFunction
     Properties:
       ServiceToken: !GetAtt ConfigureFunction.Arn
-      CloudTrailBucketName: !Ref TrailS3Bucket
+      CloudTrailBucketName: !If
+        - ShouldCreateTrail
+        - !Ref TrailS3Bucket
+        - ""
+      CloudTrailName: !Ref CloudTrailName
+      CreateCloudTrail: !Ref CreateCloudTrail
 
   SnapshotFunctionRole:
     Type: AWS::IAM::Role
@@ -681,4 +726,41 @@ Resources:
     Type: AWS::CloudFormation::CustomResource
     Properties:
       ServiceToken: !GetAtt TriggerStateMachineFunction.Arn
-    DependsOn: SnapshotStateMachine
+    DependsOn: SnapshotStateMachine
+
+Outputs:
+  CloudTrailStatus:
+    Description: >-
+      Whether this stack created a CloudTrail trail. If "Not managed by this stack",
+      ensure your account has an existing active CloudTrail trail that captures
+      Route 53 management events. If DNS sync stops working, your existing trail
+      may not be configured correctly - redeploy with CreateCloudTrail=true.
+      If upgrading from a previous version that created its own trail: the old trail
+      has been stopped and deleted, but the S3 log bucket (cloudsync-trail-bucket-*)
+      has been retained. Check the RetainedCloudTrailBucket output for the bucket name.
+      Once you have verified DNS sync is working, empty and delete the bucket manually
+      to stop incurring storage charges.
+    Value: !If
+      - ShouldCreateTrail
+      - !Sub "Created and managed by this stack: ${Trail}"
+      - "Not managed by this stack - ensure your existing trail captures Route 53 management events"
+
+  EventBridgeRuleArn:
+    Description: ARN of the EventBridge rule that captures Route 53 events
+    Value: !GetAtt EventRule.Arn
+
+  DNSUpdateQueueUrl:
+    Description: URL of the SQS queue receiving Route 53 change events
+    Value: !GetAtt DNSUpdateQueue.QueueUrl
+
+  DNSUpdateFunctionArn:
+    Description: ARN of the Lambda function processing Route 53 change events
+    Value: !GetAtt DNSUpdateFunction.Arn
+
+  RetainedCloudTrailBucket:
+    Description: >-
+      Name of the CloudTrail log bucket retained after upgrading from a version that
+      created a trail. This bucket is no longer needed and can be safely deleted to
+      stop incurring storage charges. If no bucket was retained during this deployment,
+      this value will be empty.
+    Value: !GetAtt ConfigureCustomResource.RetainedCloudTrailBucket