From 2df2023f8443d304da105a48869f91843e491959 Mon Sep 17 00:00:00 2001 From: johnnally Date: Fri, 26 Jun 2026 11:59:33 +0100 Subject: [PATCH 1/6] CLD-485: only create cloud trail if nesessary EventBridge receives Route 53 management events from any active trail in the account - it does not require a specific trail. Most AWS accounts already have a default multi-region trail (e.g. management-events) with IncludeGlobalServiceEvents enabled, which is sufficient. The custom trail we were creating was a redundant duplicate costing customers ~$3/month extra (2nd trail + KMS key + S3 storage). Changes: - Add CreateCloudTrail parameter (default: false) with AllowedValues true/false - Add ShouldCreateTrail condition gating TrailKMSKey, TrailS3Bucket, TrailBucketPolicy and Trail behind it - Use !If in ConfigureCustomResource to pass an empty CloudTrailBucketName when the bucket was not created - Use !If with AWS::NoValue in ConfigureFunctionRole to omit S3 policy statements when the bucket does not exist - Guard S3 bucket cleanup in configure/app.py behind a non-empty bucket name check to avoid errors on stack delete when no bucket was created - Document the new parameter in README.md Customers who have no existing trail can set CreateCloudTrail=true to retain the previous behaviour. --- aws/sam-app/README.md | 12 +++++-- aws/sam-app/src/configure/app.py | 10 +++--- aws/sam-app/template.yaml | 62 +++++++++++++++++++++++--------- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/aws/sam-app/README.md b/aws/sam-app/README.md index dd293f4..450d0e6 100644 --- a/aws/sam-app/README.md +++ b/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` | Name for the trail. Only used when `CreateCloudTrail` is `true`. | + ## 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. diff --git a/aws/sam-app/src/configure/app.py b/aws/sam-app/src/configure/app.py index b9897b8..5f56644 100644 --- a/aws/sam-app/src/configure/app.py +++ b/aws/sam-app/src/configure/app.py @@ -55,10 +55,12 @@ def configure_application(event, context): 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: + s3_client = boto3.resource('s3') + bucket = s3_client.Bucket(bucket_name) + bucket.objects.all().delete() # clean up log groups delete_log_groups() diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index 0fb1375..5c8da2e 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -25,6 +25,17 @@ Metadata: SourceCodeUrl: https://github.com/ns1/cloudsync/tree/0.4.3 Parameters: + CreateCloudTrail: + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + Description: > + Set to "true" only if your AWS account does NOT already have an active + multi-region CloudTrail trail with IncludeGlobalServiceEvents enabled. + Most accounts already have a default trail; leave as "false" to avoid + creating a duplicate trail and incurring unnecessary charges. CloudTrailName: Type: String Default: NS1CloudSyncTrail @@ -32,6 +43,8 @@ Parameters: Type: String Description: NS1 account API key. +Conditions: + ShouldCreateTrail: !Equals [!Ref CreateCloudTrail, "true"] Resources: UpdatesDeadLetterQueue: @@ -47,7 +60,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,6 +137,7 @@ 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]]]]]] @@ -130,6 +145,7 @@ Resources: # Bucket policy to allow CloudTrail access to the bucket. TrailBucketPolicy: Type: AWS::S3::BucketPolicy + Condition: ShouldCreateTrail Properties: Bucket: !Ref TrailS3Bucket PolicyDocument: @@ -156,9 +172,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 +380,26 @@ 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:DeleteObject - - s3:GetObject - - s3:ListBucket - Resource: !GetAtt TrailS3Bucket.Arn - - Effect: Allow - Action: - - s3:DeleteObject - - s3:GetObject - - s3:ListBucket - Resource: !Sub - - arn:aws:s3:::${Bucket}/* - - Bucket: !Ref TrailS3Bucket + - !If + - ShouldCreateTrail + - Effect: Allow + Action: + - s3:DeleteObject + - s3:GetObject + - s3:ListBucket + Resource: !GetAtt TrailS3Bucket.Arn + - !Ref AWS::NoValue + - !If + - ShouldCreateTrail + - Effect: Allow + Action: + - s3:DeleteObject + - s3:GetObject + - s3:ListBucket + Resource: !Sub + - arn:aws:s3:::${Bucket}/* + - Bucket: !Ref TrailS3Bucket + - !Ref AWS::NoValue ConfigureFunction: Type: AWS::Serverless::Function @@ -396,7 +421,10 @@ Resources: Type: Custom::ConfigureFunction Properties: ServiceToken: !GetAtt ConfigureFunction.Arn - CloudTrailBucketName: !Ref TrailS3Bucket + CloudTrailBucketName: !If + - ShouldCreateTrail + - !Ref TrailS3Bucket + - "" SnapshotFunctionRole: Type: AWS::IAM::Role From a750cb940071519847b328e1c2500ac77818c434 Mon Sep 17 00:00:00 2001 From: johnnally Date: Mon, 29 Jun 2026 14:30:33 +0100 Subject: [PATCH 2/6] CLD-485: bump template version --- aws/sam-app/template.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index 5c8da2e..e80b6a7 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -21,8 +21,8 @@ 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: From c3403259433cda0a7e22cb118d34cf307711869f Mon Sep 17 00:00:00 2001 From: johnnally Date: Mon, 29 Jun 2026 17:15:27 +0100 Subject: [PATCH 3/6] CLD-485: add Outputs section to template.yaml Surfaces CloudTrail status, EventBridge rule ARN, SQS queue URL and Lambda ARN in the CloudFormation console. The CloudTrailStatus output explicitly tells customers whether the stack is managing a trail and reminds them to clean up any orphaned NS1CloudSyncTrail after upgrade. --- aws/sam-app/template.yaml | 43 ++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index e80b6a7..b76b551 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -32,13 +32,19 @@ Parameters: - "true" - "false" Description: > - Set to "true" only if your AWS account does NOT already have an active - multi-region CloudTrail trail with IncludeGlobalServiceEvents enabled. - Most accounts already have a default trail; leave as "false" to avoid - creating a duplicate trail and incurring unnecessary charges. + 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 + Description: > + Name for the CloudTrail trail created by this stack. + Only used when CreateCloudTrail is "true". Ignored otherwise. NS1APIKey: Type: String Description: NS1 account API key. @@ -709,4 +715,31 @@ Resources: Type: AWS::CloudFormation::CustomResource Properties: ServiceToken: !GetAtt TriggerStateMachineFunction.Arn - DependsOn: SnapshotStateMachine \ No newline at end of file + 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, the old NS1CloudSyncTrail may remain + as an orphaned resource — once sync is verified working you can safely delete + it along with its S3 bucket and KMS key to stop incurring 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 From 7a4316b205ba43df04d2dc274cce2a37e43601d7 Mon Sep 17 00:00:00 2001 From: johnnally Date: Tue, 30 Jun 2026 12:53:09 +0100 Subject: [PATCH 4/6] CLD-485: andle trail/bucket cleanup on upgrade and add stack outputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Update handler in configure/app.py to stop, delete and empty the CloudSync-created trail and S3 bucket when upgrading from CreateCloudTrail=true to false, preventing CloudFormation from failing to delete a non-empty bucket - Handle 0.4.3 upgrade path where CreateCloudTrail did not exist in OldResourceProperties, defaulting old value to 'true' (trail was always createdi in 0.4.3 and earlier) - Fall back to s3:ListAllMyBuckets scan for bucket name when upgrading from 0.4.3 where CloudTrailBucketName was not present in OldResourceProperties - Safety guard: only delete trails named NS1CloudSyncTrail or CloudSyncTrail — all other trail names (management-events, org trails etc.) are skipped - Pass CloudTrailName and CreateCloudTrail through ConfigureCustomResource so the Lambda has access to them in OldResourceProperties during updates - Add s3:ListAllMyBuckets, s3:DeleteObject/GetObject/ListBucket, cloudtrail: StopLogging/DeleteTrail permissions to ConfigureFunctionRole - Update CreateCloudTrail and CloudTrailName parameter descriptions with clearer guidance and warnings DNSUpdateFunctionArn --- aws/sam-app/src/configure/app.py | 61 ++++++++++++++++++++++++++++++-- aws/sam-app/template.yaml | 48 ++++++++++++++----------- 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/aws/sam-app/src/configure/app.py b/aws/sam-app/src/configure/app.py index 5f56644..20da629 100644 --- a/aws/sam-app/src/configure/app.py +++ b/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'] @@ -48,6 +50,62 @@ 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), the stack will attempt to delete the Trail, TrailS3Bucket and + # TrailKMSKey resources. The S3 bucket cannot be deleted by CloudFormation + # while it still has objects in it, so we empty it and delete the trail here + # before CloudFormation attempts resource deletion. + # 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 and bucket. 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 releases the 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}") + + # empty the S3 bucket so CloudFormation can delete it + if bucket_name: + bucket = s3_resource.Bucket(bucket_name) + bucket.objects.all().delete() + elif request_type == 'Delete': # clean up secrets stored in Secrets Manager secret_handler.delete(NS1_API_KEY_NAME) @@ -58,8 +116,7 @@ def configure_application(event, context): # empty CloudTrail bucket and remove it (only if the stack created one) bucket_name = event['ResourceProperties'].get('CloudTrailBucketName') if bucket_name: - s3_client = boto3.resource('s3') - bucket = s3_client.Bucket(bucket_name) + bucket = s3_resource.Bucket(bucket_name) bucket.objects.all().delete() # clean up log groups diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index b76b551..3f3bb4c 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -43,8 +43,11 @@ Parameters: Type: String Default: NS1CloudSyncTrail Description: > - Name for the CloudTrail trail created by this stack. + Name for the new CloudTrail trail created by this stack. Only used when CreateCloudTrail is "true". Ignored otherwise. + 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. @@ -386,26 +389,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*' - - !If - - ShouldCreateTrail - - Effect: Allow - Action: - - s3:DeleteObject - - s3:GetObject - - s3:ListBucket - Resource: !GetAtt TrailS3Bucket.Arn - - !Ref AWS::NoValue - - !If - - ShouldCreateTrail - - Effect: Allow - Action: - - s3:DeleteObject - - s3:GetObject - - s3:ListBucket - Resource: !Sub - - arn:aws:s3:::${Bucket}/* - - Bucket: !Ref TrailS3Bucket - - !Ref AWS::NoValue + - Effect: Allow + Action: + - s3:ListAllMyBuckets + Resource: '*' + - Effect: Allow + Action: + - s3:DeleteObject + - s3:GetObject + - s3:ListBucket + Resource: !Sub 'arn:aws:s3:::cloudsync-trail-bucket-*' + - Effect: Allow + Action: + - s3:DeleteObject + - s3:GetObject + - s3:ListBucket + 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 @@ -431,6 +435,8 @@ Resources: - ShouldCreateTrail - !Ref TrailS3Bucket - "" + CloudTrailName: !Ref CloudTrailName + CreateCloudTrail: !Ref CreateCloudTrail SnapshotFunctionRole: Type: AWS::IAM::Role From 7776a682f79f328f7a5015f5c050f85d24ed79ac Mon Sep 17 00:00:00 2001 From: johnnally Date: Thu, 2 Jul 2026 16:33:58 +0100 Subject: [PATCH 5/6] CLD-485: retain CloudTrail bucket on upgrade instead of auto-deleting Bucket deletion during a stack update is size-dependent and may cause the Lambda to timeout, leaving the stack in UPDATE_ROLLBACK_FAILED. A real-world test on a 1-month-old bucket (~927MB, ~144k objects) caused a 15-minute DNS sync outage via Lambda timeout and CloudFormation retry. Instead, stop and delete the CloudSync-managed trail only (fast), retain the S3 bucket, and warn the customer via CloudWatch logs to empty and delete it manually once DNS sync is verified working. Also surface the retained bucket name in a new RetainedCloudTrailBucket CloudFormation output so customers can find it without digging through logs. --- aws/sam-app/src/configure/app.py | 40 ++++++++++++++++++++++++-------- aws/sam-app/template.yaml | 17 ++++++++++---- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/aws/sam-app/src/configure/app.py b/aws/sam-app/src/configure/app.py index 20da629..1be415f 100644 --- a/aws/sam-app/src/configure/app.py +++ b/aws/sam-app/src/configure/app.py @@ -41,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': @@ -53,15 +55,19 @@ def configure_application(event, context): 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), the stack will attempt to delete the Trail, TrailS3Bucket and - # TrailKMSKey resources. The S3 bucket cannot be deleted by CloudFormation - # while it still has objects in it, so we empty it and delete the trail here - # before CloudFormation attempts resource deletion. + # 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 and bucket. This is NOT the customer-facing default — + # 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 @@ -82,7 +88,7 @@ def configure_application(event, context): bucket_name = b['Name'] break - # stop and delete the trail so it releases the bucket. + # 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. @@ -101,10 +107,22 @@ def configure_application(event, context): else: print(f"Skipping deletion of trail not managed by CloudSync: {trail_name}") - # empty the S3 bucket so CloudFormation can delete it + # 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: - bucket = s3_resource.Bucket(bucket_name) - bucket.objects.all().delete() + 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 @@ -127,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'] diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index 3f3bb4c..1d87657 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -725,14 +725,15 @@ Resources: Outputs: CloudTrailStatus: - Description: > + 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, the old NS1CloudSyncTrail may remain - as an orphaned resource — once sync is verified working you can safely delete - it along with its S3 bucket and KMS key to stop incurring charges. + If upgrading from 0.4.3: the old trail has been stopped and deleted, but the + S3 log bucket (cloudsync-trail-bucket-*) has been retained. 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}" @@ -749,3 +750,11 @@ Outputs: 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 — empty and delete it manually + to stop incurring storage charges. Empty if no bucket was retained during this + deployment. + Value: !GetAtt ConfigureCustomResource.RetainedCloudTrailBucket From 3f6b63a11024d41e063ca73537c9ef84c0e80e70 Mon Sep 17 00:00:00 2001 From: johnnally Date: Fri, 3 Jul 2026 12:33:26 +0100 Subject: [PATCH 6/6] CLD-485: clarify CloudTrailName is unused when CreateCloudTrail=false Add AllowedPattern to CloudTrailName to block known system trail names (management-events, aws-controltower-BaselineCloudTrail, AccountTrail) at CloudFormation validation time, preventing accidental deletion of existing trails. Prepend description with "ONLY USED when CreateCloudTrail is true" so it is the first thing visible in the SAR console parameter UI. Update README to bold the same message in the parameters table. Fix wording on RetainedCloudTrailBucket output description. --- aws/sam-app/README.md | 2 +- aws/sam-app/template.yaml | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/aws/sam-app/README.md b/aws/sam-app/README.md index 450d0e6..fc7f79d 100644 --- a/aws/sam-app/README.md +++ b/aws/sam-app/README.md @@ -26,7 +26,7 @@ off across the Internet to the NS1 CloudSync REST API endpoint. Events are then |---|---|---| | `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` | Name for the trail. Only used when `CreateCloudTrail` is `true`. | +| `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 diff --git a/aws/sam-app/template.yaml b/aws/sam-app/template.yaml index 1d87657..8d9fc46 100644 --- a/aws/sam-app/template.yaml +++ b/aws/sam-app/template.yaml @@ -42,9 +42,14 @@ Parameters: CloudTrailName: Type: String Default: NS1CloudSyncTrail - Description: > + 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. - Only used when CreateCloudTrail is "true". Ignored otherwise. 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". @@ -730,10 +735,11 @@ Outputs: 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 0.4.3: the old trail has been stopped and deleted, but the - S3 log bucket (cloudsync-trail-bucket-*) has been retained. Once you have - verified DNS sync is working, empty and delete the bucket manually to stop - incurring storage charges. + 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}" @@ -754,7 +760,7 @@ Outputs: RetainedCloudTrailBucket: Description: >- Name of the CloudTrail log bucket retained after upgrading from a version that - created a trail. This bucket is no longer needed — empty and delete it manually - to stop incurring storage charges. Empty if no bucket was retained during this - deployment. + 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