testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.65k stars 250 forks source link

feat: Add Apache Pulsar module #1103

Closed entvex closed 1 month ago

entvex commented 5 months ago

What does this PR do?

This PR introduces Apache Pulsar support to testcontainers-dotnet, enabling users to easily spin up Pulsar containers for testing purposes. It also provides an option to enable TokenAuthentication, allowing for secure Pulsar interactions.

Why is it important?

Apache Pulsar is a rapidly growing distributed messaging system gaining traction in the cloud-native ecosystem. Integrating Pulsar support into testcontainers-dotnet will make it easier for developers to test their applications using Pulsar. Additionally, providing TokenAuthentication support enhances security and privacy in Pulsar-based testing scenarios.

How to test this PR

Run the XUnit tests in the testcontainers-dotnet project to ensure the Pulsar containerization and TokenAuthentication functionality work as expected.

Follow-ups

@HofmeisterAn, I encountered a challenge while adding WithTokenAuthentication capabilities to PulsarBuilder. The current implementation of WithCommand appends commands instead of replacing them, making it difficult to modify container startup commands based on selected Pulsar features. In the testcontainers-java module, the container command is modified just before the container starts, enabling control over its execution. Could you provide suggestions on how to achieve a similar approach in testcontainers-dotnet?

In the java module they seem to have the control to edit it, just before the container starts

Do you have a suggestion on a suggestion on how I should implement this for testcontainers-dotnet ?

netlify[bot] commented 5 months ago

Deploy Preview for testcontainers-dotnet ready!

Name Link
Latest commit f4d5be96de5e451c39b5cc2efd92b5db30714a54
Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/665c242add249c0008729202
Deploy Preview https://deploy-preview-1103--testcontainers-dotnet.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

HofmeisterAn commented 5 months ago

The current implementation of WithCommand appends commands instead of replacing them, making it difficult to modify container startup commands based on selected Pulsar features.

You can apply the final command at the end. We are doing this in other modules too; sometimes it is necessary to configure the wait strategy at the end. You just need to keep a reference somewhere (in the configuration) to decide which command should be used. Or you can use a similar approach we are doing with Kafka and upload a script that gets executed. Does that help?

entvex commented 5 months ago

The current implementation of WithCommand appends commands instead of replacing them, making it difficult to modify container startup commands based on selected Pulsar features.

You can apply the final command at the end. We are doing this in other modules too; sometimes it is necessary to configure the wait strategy at the end. You just need to keep a reference somewhere (in the configuration) to decide which command should be used. Or you can use a similar approach we are doing with Kafka and upload a script that gets executed. Does that help?

Yes! Thanks 👍

entvex commented 5 months ago

Hi @HofmeisterAn , Testcontainers.Pulsar is now ready for a code review.

entvex commented 4 months ago

Dear @HofmeisterAn. I can see you have started looking at my PR. Please let me know if you need any help getting it across the finish line 💪.

entvex commented 2 months ago

@HofmeisterAn, I have successfully integrated the approach you suggested from Couchbase into the pulsar module. During my individual testing, the tests passed without any issues. However, when I queued them to run consecutively, I noticed that the test without authentication was hanging. I am wondering if there might be an issue with the [UsedImplicitly] after you refactored the tests. Could you please take a closer look and let me know if you spot anything?

Thank you for your valuable contribution to the open-source community.

Loydik commented 1 month ago

Can you please complete this? This library is needed for my project. Thank you

entvex commented 1 month ago

Can you please complete this? This library is needed for my project. Thank you

I am waiting for a reponse from @HofmeisterAn :)

HofmeisterAn commented 1 month ago

I stumbled across this discussion where the friends from Go are trying to achieve something similar. Maybe this can help to sort out the issue or align the configuration.

entvex commented 1 month ago

I stumbled across this discussion where the friends from Go are trying to achieve something similar. Maybe this can help to sort out the issue or align the configuration.

Very interesting! Thanks for the link!

entvex commented 1 month ago

Hi @HofmeisterAn,

I've identified a critical bug in Apache Pulsar related to token creation. The issue stems from mishandling time units (specifically, treating seconds as milliseconds). In my latest commit to this module, I've adjusted the expiration time for tokens used in the authentication to be one year. This change mitigates the impact of the token creation bug, which affected versions 3.2.0 to 3.2.3.

entvex commented 1 month ago

The tests now run reliably, so you are free to merge 😄 @HofmeisterAn

HofmeisterAn commented 1 month ago

specifically, treating seconds as milliseconds

I see. Do you have any additional information, such as official documentation, a related issue, etc., regarding the workaround? Do we set the --expiry-time correct? I can review the PR again sometime this week :v:.

entvex commented 1 month ago

specifically, treating seconds as milliseconds

I see. Do you have any additional information, such as official documentation, a related issue, etc., regarding the workaround? Do we set the --expiry-time correct? I can review the PR again sometime this week ✌️.

Jep, we have set the --expiry-time correct. But as I mentioned before, the bug is in pulsar. I have already talked to another Apache Pulsar PMC. I'll create an issue and submit a PR for pulsar, but I have not created them yet. Therefore, I can't link to them here.

entvex commented 1 month ago

Thanks for the PR, the efforts, and the patience. Great first contribution 🙏.

Thanks a lot! It was a pleasure working with you. I hope we can collaborate more modules soon 👍

smbecker commented 1 month ago

I noticed that this merged. Do you have an estimate on when it will be available in nuget?