Skip to content

(feat): PubSub Harness - #18

Merged
WhitWaldo merged 8 commits into
mainfrom
pubsub-harness
Sep 14, 2026
Merged

WhitWaldo merged 8 commits into
mainfrom
pubsub-harness

Conversation

@WhitWaldo

Copy link
Copy Markdown
Contributor

Description

Added RabbitMQ container and PubSub harness

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
@WhitWaldo
WhitWaldo requested a lite review from Copilot September 13, 2026 23:42
@WhitWaldo WhitWaldo self-assigned this Sep 13, 2026
@WhitWaldo WhitWaldo added the enhancement New feature or request label Sep 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings affect broker authentication, credential handling, and endpoint configuration.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds RabbitMQ-backed Dapr Pub/Sub support with a reusable container, harness, integration, tests, exports, and documentation.

Changes:

  • Adds RabbitMQ container and Pub/Sub component helpers.
  • Adds PubSubHarness lifecycle and client support.
  • Integrates RabbitMQ with DaprContainer.
  • Adds tests, exports, and README usage.
File summaries
File Summary Findings
src/RabbitMQContainer.ts RabbitMQ container and component utilities Critical (3 votes): Default guest credentials fail for remote Dapr connections (lines 20, 49). Moderate (2 votes): Credentials must be percent-encoded in the AMQP URI.
src/RabbitMQContainer.test.ts RabbitMQ behavior and startup tests No final comments.
src/PubSubHarness.ts Pub/Sub harness lifecycle and clients Moderate (1 vote): Derive port and protocol from the effective client options.
src/PubSubHarness.test.ts Harness and end-to-end tests No final comments.
src/index.ts Public exports No final comments.
src/DaprContainer.ts RabbitMQ and Pub/Sub integration Moderate (3 votes): Propagate custom container credentials when Pub/Sub credentials are omitted.
README.md Pub/Sub usage documentation No final comments.
Review details

Suppressed comments (2)

src/PubSubHarness.ts:196

  • The default port is selected from the server's protocol, but serverOptions.clientOptions is spread over it below and can override the client protocol. For example, a gRPC client override still receives the HTTP port, so the server's internal Dapr client connects to the wrong endpoint. Derive the port and communicationProtocol from the effective nested client options together.
      clientOptions: {
        daprHost: started.getHost(),
        daprPort: defaultPort,
        communicationProtocol: protocol,
        ...serverOptions?.clientOptions,

src/RabbitMQContainer.ts:51

  • withPort changes the port exposed by Testcontainers and the port later used in the component, but it never configures RabbitMQ's listener. A container created with .withPort(5673) still listens on 5672, so getMappedPort(5673) and rabbitmq:5673 do not point at the broker. Configure RabbitMQ's node port here, or remove this override and keep the fixed internal port.
  public withPort(port: number): this {
    this.rabbitMQPort = port;
    this.withExposedPorts(port);
  • Files reviewed: 7/7 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/RabbitMQContainer.ts Outdated
Comment thread src/DaprContainer.ts Outdated
Comment thread src/RabbitMQContainer.ts Outdated
WhitWaldo and others added 7 commits September 13, 2026 19:26
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
@WhitWaldo
WhitWaldo merged commit cf56b7a into main Sep 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants