Checked in new pattern rabbitmq-private-lambda-python-sam

[gmedard]

Issue #, if available:

Description of changes: - We created a new pattern rabbitmq-private-lambda-java-sam a few months ago. Now checking in the Python equivalent of the pattern rabbitmq-private-lambda-python-sam

When running the CloudFormation template, please remember to change the input parameter ServerlessLandGithubLocation from https://github.com/aws-samples/serverless-patterns.git to https://github.com/gmedard/serverless-patterns.git, else the sample will not work. This is only for testing till the PR is merged. Customers won't have to undertake this step.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[bfreiberg]

Code review — rabbitmq-private-lambda-python-sam

Thanks for the contribution! I reviewed the pattern and validated the findings against a live end-to-end deployment. Findings below are grouped by severity, and I've marked each with how it was verified.

🔴 High — data loss

1. Consumer silently drops the entire batch on any error — rabbitmq_consumer_dynamo_sam/rabbitmq_event_consumer_function/lambda_function.py:19-90

lambda_handler wraps the whole message loop in try/except and, on any exception, logs it and return "500". Amazon MQ event source mappings ignore the handler's return value and only retry/redrive when the invocation raises. Returning normally is treated as success, so the batch is acked and removed from the queue.

Verified live: I pointed the function at a table it couldn't write, published 20 messages, and observed:

• ESM delivered 2 batches; each logged An exception occurred - ... PutItem ... AccessDeniedException
• Lambda Errors metric = 0, Invocations = 2 (ESM saw success)
• DynamoDB table count = 0 (nothing persisted)
• Queue depth = 0, no redelivery — the messages were gone

Because the try wraps the whole loop, the logs also showed only the first message of each batch reached put_item before the exception aborted the remaining 9 — so one bad/throttled message discards everything after it in the batch too.

Suggested fix: let exceptions propagate (remove the catch-all), or adopt partial-batch responses (ReportBatchItemFailures / bisectBatchOnFunctionError + a DLQ) so failed messages are retried instead of silently dropped.

2. Producer off-by-one skips the first record and crashes at the max count — rabbitmq_message_sender_json/rabbitmq_producer.py:76-77

for i in range(1, num_to_send + 1): ... people[i] skips people[0] and reads out of bounds when num_to_send == len(people). us-500.csv has 500 rows (0–499, no header).

Verified live: running the real producer with 500 raised IndexError: list index out of range at people[500] after publishing only 499 messages (the first CSV record was never sent). The README explicitly tells users to pass "a number between 1 and 500", so 500 is a documented input.

Suggested fix: for i in range(num_to_send): person = get_person_from_row(people[i]) (and update the header/number fields accordingly).

🟠 Medium — security posture

3. Broker security group opens all ports to the whole VPC — RabbitMQAndClientEC2.yaml:337-340

Alongside the scoped 5671/443 rules, RabbitMQSecurityGroup allows tcp 0-65535 from the VPC CIDR 10.2.0.0/16. Verified on the deployed SG. Recommend dropping this rule and keeping only 5671 (and 443 if the console/management API is needed) from the client SG.

4. Client EC2 instance role is effectively account administrator — RabbitMQAndClientEC2.yaml:681-732

The role attaches IAMFullAccess, AWSLambda_FullAccess, AmazonS3FullAccess, AmazonDynamoDBFullAccess_v2, AWSCloudFormationFullAccess, CloudWatchFullAccess, plus inline iam:* and mq:* on *. Verified on the deployed role. IAMFullAccess + iam:* also allows privilege escalation. Recommend scoping to the specific actions the setup scripts and sam deploy actually need.

5. Hardcoded default broker password — RabbitMQAndClientEC2.yaml:39-43

RabbitMQBrokerPassword defaults to rabbitmqPassword123, committed to a public repo and also written into Secrets Manager. Recommend removing the default and requiring the value at deploy time (or generating one via Secrets Manager).

🟡 Low — correctness / docs

6. Empty messageId breaks the DynamoDB write — lambda_function.py:95

MessageID (the table's partition key) defaults to '' when basicProperties.messageId is absent. Verified live: PutItem with an empty-string key returns ValidationException: The AttributeValue for a key attribute cannot contain an empty string value (and that error is then swallowed by finding #1). Recommend skipping/dead-lettering messages without a usable key.

7. MessageType stores the message ID, not the type — lambda_function.py:116

'MessageType': message_id — should likely be basic_properties.get('type', '') (the type is otherwise only stored under Type).

8. templateFile points at a non-existent directory — example-pattern.json:20

"activemq_consumer_dynamo_sam/template_original.yaml" — this pattern's directory is rabbitmq_consumer_dynamo_sam (looks like a leftover from an ActiveMQ pattern). Breaks the generated template link.

9. Python3Version parameter description contradicts its allowed values — RabbitMQAndClientEC2.yaml:10-18

The description says "between 3.9 and 3.12 … maximum allowed version is 3.12", but AllowedValues goes up to python3.14 and the default is python3.14.

[gmedard]

🔴 High — Data Loss

1: Consumer silently drops the entire batch on any error

What we changed: Removed the catch-all try/except block entirely from lambda_handler. Exceptions now propagate naturally, causing the Lambda invocation to fail. The ESM sees the failure and retries the batch (or sends to DLQ if configured).

Why: The ESM contract for Amazon MQ is that a successful invocation (no exception raised) means the batch was processed. The only way to signal failure is to let the exception propagate. This is different from SQS-based event sources where you can use batchItemFailures in the response.

2: Producer off-by-one skips the first record and crashes at the max count

What we changed: Changed to for i in range(num_to_send): person = get_person_from_row(people[i]). Updated MessageNumberInBatch, correlation_id, message_id, and the print statement to use i + 1 for 1-based display numbers.

🟠 Medium — Security Posture

3: Broker security group opens all ports to the whole VPC

What we changed: Removed the tcp 0-65535 rule entirely. The broker SG now only allows:

Port 5671 from the client SG (AMQPS connections)
Port 5671 from itself (self-referencing — needed for Lambda ESM ENIs)
Port 443 from the client SG (management API/console)

4: Client EC2 instance role is effectively account administrator

What we changed: Removed all overly broad managed policies and the iam:* inline policy. Replaced with a single SAMDeployPolicy scoped to specific actions needed for sam deploy:

• cloudformation:* (stack CRUD, changesets, template operations including GetTemplateSummary)
• lambda:* (function and event source mapping CRUD)
• iam:CreateRole/DeleteRole/GetRole/PassRole/AttachRolePolicy/DetachRolePolicy/PutRolePolicy/DeleteRolePolicy/GetRolePolicy/TagRole/UntagRole scoped to account
• s3:* (artifact bucket operations)
• dynamodb:CreateTable/DeleteTable/DescribeTable/UpdateTable/TagResource/UntagResource
• logs:CreateLogGroup/CreateLogStream/PutLogEvents/DescribeLogGroups/DescribeLogStreams
• ec2:Describe* (networking for Lambda VPC config)
• secretsmanager:GetSecretValue/PutSecretValue/DescribeSecret

Kept only AmazonMQFullAccess (legitimate — scripts call aws mq describe-broker) and AmazonSSMManagedInstanceCore (for Session Manager access).

5: Hardcoded default broker password

What we changed: Removed the RabbitMQBrokerPassword parameter entirely. The RabbitMQSecret resource now uses GenerateSecretString to auto-generate a 24-character alphanumeric password at stack creation time:

GenerateSecretString:
  SecretStringTemplate: !Sub '{"username": "${RabbitMQBrokerAdminUser}"}'
  GenerateStringKey: 'password'
  PasswordLength: 24
  ExcludePunctuation: true

The broker retrieves the password via a CloudFormation dynamic reference:

Password: !Sub '{{resolve:secretsmanager:${RabbitMQSecret}:SecretString:password}}'

Shell scripts (create_rabbit_queue.sh, query_rabbit_queue.sh) fetch credentials from Secrets Manager at runtime using aws secretsmanager get-secret-value.

🟡 Low — Correctness / Docs

6: Empty messageId breaks the DynamoDB write

What we changed: Added a guard at the top of insert_into_dynamodb:

if not message_id:
    print("WARNING: Skipping message with empty messageId - cannot use as DynamoDB partition key")
    return

7: MessageType stores the message ID, not the type

What we changed: Changed to 'MessageType': basic_properties.get('type', '').

8: templateFile points at a non-existent directory

What we changed: Fixed to "rabbitmq_consumer_dynamo_sam/template_original.yaml".

9: Python3Version parameter description contradicts its allowed values

What we changed: Updated description to: "Choose the version of Python 3. Available options are 3.11, 3.12, 3.13, and 3.14. Note that Python 3.10 is not available on Amazon Linux 2023 so it is not offered as an option."