Support custom dead-letter-queue names for self-provisioned SQS/RabbitMQ queues - #1033
Conversation
Sorry, something went wrong.
…bbitMQ Adds AWS_SQS_DELIVERY_DLQ/AWS_SQS_LOG_DLQ and RABBITMQ_DELIVERY_DLQ/RABBITMQ_LOG_DLQ so operators whose org naming conventions don't match the hardcoded <queue>-dlq / <queue>.dlq suffixes can set the DLQ name explicitly, instead of working around it externally. Falls back to the existing derived name when unset. Generated with AI Co-Authored-By: Claude <noreply@anthropic.com>
|
hi there, thanks for opening the PR, I think the motivation is reasonable. Let me think through the approach and share more soon! |
Sorry, something went wrong.
alexluong
left a comment
There was a problem hiding this comment.
This approach works great! Can we apply the same approach to GCP and Azure as well? For Azure, I believe DLQ is a part of the subscription itself. I think we should add some sort of comment or something to make it explicit that we don't accept DLQ config instead of forgetting about it.
If you're not comfortable with GCP/Azure, I can follow up on that for you as well, no worries. Let me know how you'd like to proceed.
Sorry, something went wrong.
Addresses review feedback on hookdeck#1033: - Move the "override or derive" fallback from mqinfra's dlqName() helpers into getDLQName() in the config layer, so the resolved name is available to callers. mqinfra now provisions the name it is given; naming policy lives in one place. - Log the resolved delivery/log DLQ names in the startup configuration summary for SQS and RabbitMQ, per the convention in logging.go. - Add config-layer unit tests for DLQ name resolution (derive, override, no-leak-between-queue-types, empty cases) and set DLQ explicitly in the mqinfra integration tests now that the layer no longer derives it. Generated with AI Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review feedback on hookdeck#1033: - Resolve DLQ names in the config layer's getDLQName() so the effective name is available to callers, and define each provider's naming convention once in mqinfra (DefaultAWSSQSDLQName / DefaultRabbitMQDLQName), used by both layers. The mqinfra DLQ field stays optional and still falls back to the convention, so constructing the infra config directly keeps working as before. - Log the resolved delivery/log DLQ names in the startup configuration summary for SQS and RabbitMQ, per the convention in logging.go. - Add config-layer unit tests for DLQ name resolution: explicit override, fallback to the default, no leaking between queue types, and empty cases. Generated with AI Co-Authored-By: Claude <noreply@anthropic.com>
|
Azure: confirmed there's no DLQ entity to name: Declare only sets DeadLetteringOnMessageExpiration + MaxDeliveryCount, and consumption uses azservicebus.SubQueueDeadLetter (Azure's fixed $DeadLetterQueue) GCP: want your call on the shape first, since it creates two DLQ resources and already names topics/subscriptions independently:
|
Sorry, something went wrong.
|
Let's go with option 1!
|
Sorry, something went wrong.
…b/Sub Extends the SQS/RabbitMQ DLQ naming config to GCP Pub/Sub, using separate topic and subscription settings to match how GCP names its main resources: GCP_PUBSUB_DELIVERY_DLQ_TOPIC / GCP_PUBSUB_DELIVERY_DLQ_SUBSCRIPTION GCP_PUBSUB_LOG_DLQ_TOPIC / GCP_PUBSUB_LOG_DLQ_SUBSCRIPTION Both are optional and fall back to the existing '<topic>-dlq' and '<dlq-topic>-sub' conventions, now defined once in mqinfra and shared with the config layer. The derived subscription follows the resolved DLQ topic, so overriding only the topic keeps the pair consistent. Azure Service Bus takes no DLQ name settings: it dead-letters into each subscription's built-in $DeadLetterQueue sub-queue, whose name is fixed by the platform. Noted in the config struct so the omission reads as deliberate. Also logs the resolved GCP DLQ names at startup and documents the new settings. Generated with AI Co-Authored-By: Claude <noreply@anthropic.com>
alexluong
left a comment
There was a problem hiding this comment.
👌 🙏
Sorry, something went wrong.
Why
When Outpost self-provisions its internal delivery and log queues, it also auto-creates a DLQ for each, but the DLQ name is currently hardcoded as a derived suffix (<queue>-dlq for SQS, <queue>.dlq for RabbitMQ), with no way to override it.
If you're self-provisioning, it seems reasonable that you'd want to control the name of every resource Outpost creates, not just the main queues. Our organization has its own naming standard for dead-letter queues driven by infra/security policy (dead_letter-<queue_name>), which doesn't match what's assumed here, and we suspect other self-hosters run into the same kind of mismatch with their own conventions. Without an override, the only options are to fight the convention or build workarounds outside Outpost just to get provisioning to comply.
Wanted to check: is this something you'd be open to supporting, and if so, does this approach look right, or would you prefer a different shape (e.g. a full name-pattern/template instead of a fixed override, or a single shared suffix config instead of per-queue)?
What changed
Added optional config fields, symmetric to the existing *_DELIVERY_QUEUE / *_LOG_QUEUE settings, that let operators name each DLQ explicitly:
When unset, behavior is unchanged — Outpost falls back to the existing derived name (<queue>-dlq / <queue>.dlq), so this is purely additive and backward compatible.