LocalstackContainer API enhancement #702
Conversation
…- Add environment variables retrieval after container startup Signed-off-by: Laurent Broudoux <[email protected]>
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: Laurent Broudoux <[email protected]>
| it("should use access environment variables and default creds", async () => { | ||
| const container = await new LocalstackContainer() | ||
| .withEnvironment({ | ||
| DEFAULT_REGION: "eu-west-3", |
There was a problem hiding this comment.
Where does the DEFAULT_REGION convention come from? I had a quick look at the docs and found https://docs.localstack.cloud/references/configuration/#legacy, which says that it's no longer supported.
There was a problem hiding this comment.
Yes, I think you're right. Many things have changed with 3.x and I should have refreshed my knowledge. The point is the default image is still localstack/localstack:2.2.0. As multi-region has been introduced incrementally, this may not affect the module, but it may be nice to upgrade to 3.x to definitely get rid of this question. What do you think?
|
|
||
| /** | ||
| * Provides a default access key that is preconfigured to communicate with a given simulated service. | ||
| * <a href="https://github.com/localstack/localstack/blob/master/doc/interaction/README.md?plain=1#L32">AWS Access Key</a> |
There was a problem hiding this comment.
I'm getting a 404 for these URLs. I also couldn't find reference to these environment variables in the LS config
There was a problem hiding this comment.
Sorry, I blindly copy/paste here the comments coming from testcontainers-java/localstack
|
|
||
| expect(container.getRegion()).toBe("eu-west-3"); | ||
| expect(container.getAccessKey()).toBe("test"); | ||
| expect(container.getSecretKey()).toBe("test"); |
There was a problem hiding this comment.
The container is mapping the environment variables to these getter methods, but this test is not checking that LS is actually using them. From the deprecation notices in the docs I get the impression they're not being used.
There was a problem hiding this comment.
Yes, I think you're right... I see nothing in current documentation referencing these... Here again, I tried to reproduce the behavior of the test containers-java/local stack module, but this may be inappropriate.
|
I've done some other client tests with the So I tend to think this PR (and related issue) is, in fact, no longer needed with the recent version of localstack... |
|
Sounds good to me @lbroudoux, shall we close this PR and raise another for upgrading the LS version to v3? |
|
Hey @lbroudoux could you also fix missing reference in mkdocs.yml to add localstack to the documentation? It was forgotten in PR #640. @cristianrgreco or should I create a separate PR with this small fix for you? - Modules:
- ....
- Localstack: modules/localstack.md |
If you could that would be much appreciated! |
@cristianrgreco Done, see #710 |
|
Sorry I was away from keyboard these last days. I think we can close that PR as unsolved and I'll submit another one tomorrow to upgrade to localstack latest 3.x. |
As discussed in #701
Thank you!
Signed-off-by: Laurent Broudoux [email protected]