Allow initialising nats server with no credentials set#1077
Allow initialising nats server with no credentials set#1077george-of-croton wants to merge 6 commits into
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
george-of-croton
changed the title
|
The credentials are already handled transparently - what's the use case for explicitly not setting them? |
I'm working on a project that runs nats as a message bus in k8s cluster that doesn't use username / password auth. I can't use this test container because projects library doesn't expect username and password to be provided. |
|
+1, we use nats with TLS. In unit tests we'd like to disable auth entirely. If we use username+password we'd have to introduce a variant into the NATS client code solely to use the test container which is not ideal. |
There was a problem hiding this comment.
Pull Request Overview
This PR enables starting a NATS server container without credentials, which is a valid NATS configuration. It adds a method to control whether credentials are used during container startup.
- Added
withCredentials(boolean)method to enable/disable credential usage - Modified container startup to conditionally pass credentials to the started container
- Added test coverage for the no-credentials scenario
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/modules/nats/src/nats-container.ts | Implements credential control logic and conditional credential passing |
| packages/modules/nats/src/nats-container.test.ts | Adds test for no-credentials functionality |
| docs/modules/nats.md | Documents the new no-credentials feature |
| (this.useCredentials && this.getUser()) || undefined, | ||
| (this.useCredentials && this.getPass()) || undefined |
There was a problem hiding this comment.
[nitpick] The conditional logic could be simplified by using a ternary operator for better readability: this.useCredentials ? this.getPass() : undefined
| (this.useCredentials && this.getUser()) || undefined, | |
| (this.useCredentials && this.getPass()) || undefined | |
| this.useCredentials ? this.getUser() : undefined, | |
| this.useCredentials ? this.getPass() : undefined |
| return this; | ||
| } | ||
|
|
||
| public withCredentials(useCredentials: boolean): this { |
There was a problem hiding this comment.
@george-of-croton Need to handle this scenario:
const container = await
.withUsername("x")
.withUsername("y")
.withCredentialsEnabled(true)
.start()
container.getUser(); // test
container.getPass(); // test
cristianrgreco
added
enhancement
|
@george-of-croton this PR is getting stale, are you planning on addressing the review comments? |
it's valid to connect to nats with no username or password. This PR adds a method to remove credential flags on startup.