Skip to content

Replace fake-smtp-server with smtp4dev #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

feO2x
Copy link
Contributor

@feO2x feO2x commented Jun 7, 2023

I found out that the fake-smtp-server did not work at all in local dev mode with docker compose. The Dapr SMTP Binding always requires a TLS connection and fake-smtp-server cannot be configured to run with a self-signed certificate.

I thus replaced it with [smtp4dev], a .NET-based SMTP server which generates a self-signed certificate automatically if needed.

I also added a sign-up command to the AdminCLI in this PR. This way, we can perform the following commands with it:

./AdminCli products list
./AdminCli products sign-up -e [email protected] -p a028d630-2da8-432d-ad8c-b4990d288841 -t 9.5
./AdminCli products drop-price -p a028d630-2da8-432d-ad8c-b4990d288841 -d 0.2

Furthermore, the price watcher service and the products service now use the same products. I created a migration that inserts products with the appropriate ID. A single transaction now spans a migration.

feO2x and others added 6 commits June 6, 2023 15:59
This command allows you to sign up for price drop notifications with an email address, product ID, and target price. The overall workflow should be:
- list all products and pick the ID of the target product
- call sign up to register your email address for a specific product and target price
- drop the price

Signed-off-by: Kenny Pflug <[email protected]>
…rvice and PriceWatcherService

Signed-off-by: Kenny Pflug <[email protected]>
This migration inserts the same products that the in-memory repositories use to the Azure SQL database. Another great thing would be to actually retrieve the products in the price watcher service when checking that a product ID is valid.

I also restructured the migrations in this commit: Script as well as PostMigration are now executed within the same transaction.

Signed-off-by: Kenny Pflug <[email protected]>
Otherwise, we would need to adjust the whole Dockerfile infrastructure.

Signed-off-by: Kenny Pflug <[email protected]>
The Dapr SMTP binding requires the target STMP server to use a TLS connection which fake-smtp-server does not support out-of-the-box. I replaced it with smtp4dev, a .NET-based solution which automatically creates self-signed certificates if necessary. See https://github.com/rnwood/smtp4dev

Signed-off-by: Kenny Pflug <[email protected]>
@feO2x feO2x requested a review from ThorstenHans June 7, 2023 11:30
@feO2x
Copy link
Contributor Author

feO2x commented Jun 7, 2023

The Helm charts for fake-smtp still need to be updated.

@feO2x feO2x linked an issue Jun 7, 2023 that may be closed by this pull request
@feO2x
Copy link
Contributor Author

feO2x commented Jun 7, 2023

Alternatively, we could also just leave fake-smtp-server in our Azure environment, I just realized that we use an nginx ingress server which handles TLS connections for fake-smtp-server.

@ThorstenHans
Copy link
Contributor

I tested the main branch locally. Works like a charm here:

image

Having also a UI in front of the SMTP was mandatory for demonstration purpose, that's why we did not went the smtp4dev route

@feO2x
Copy link
Contributor Author

feO2x commented Jun 7, 2023

Do you use an older image of fake-smtp-server? I can't even access localhost:5080 on the main branch. That's because in this commit the default ports were changed (see also first commit in this PR).

fake-smtp-server not accessible on localhost:5080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fake-smtp-server changed default ports
2 participants