Enable email for Connect integration tests #90

cgraham-rs · 2025-05-01T21:47:28Z

Our previous attempt at enabling email was incorrect.

Add mailhog container to provide a SMTP service that will log emails to the CLI
Configure Connect to use mailhog service using environment variables to override/extend the default Connect config included with the public container images. This seemed much easier than trying to figure out how to inject a second config file but only after the included config file was loaded first
Included new makefile targets to start the Connect container without the tests running, this was needed to manually test the mailhog functionality
Add new Python script to set the bootstrap admin's email address to suppress email errors logged related to this, and a related makefile target to execute it
Fixed the makefile packaging target

Example of mailhog CLI logging when an account email is sent from Connect.

2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Starting session
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: INVALID] Started session, switching to ESTABLISH state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 35 bytes: '220 mailhog.example ESMTP MailHog\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Received 19 bytes: 'EHLO 09c02e9da568\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: ESTABLISH] Processing line: EHLO 09c02e9da568
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: ESTABLISH] In state 1, got command 'EHLO', args '09c02e9da568'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: ESTABLISH] In ESTABLISH state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: ESTABLISH] Got EHLO command, switching to MAIL state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 24 bytes: '250-Hello 09c02e9da568\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 16 bytes: '250-PIPELINING\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 16 bytes: '250 AUTH PLAIN\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Received 34 bytes: 'MAIL FROM:<no-reply@example.com>\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: MAIL] Processing line: MAIL FROM:<no-reply@example.com>
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: MAIL] In state 6, got command 'MAIL', args 'FROM:<no-reply@example.com>'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: MAIL] In MAIL state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: MAIL] Got MAIL command, switching to RCPT state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 36 bytes: '250 Sender no-reply@example.com ok\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Received 29 bytes: 'RCPT TO:<admin@example.com>\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] Processing line: RCPT TO:<admin@example.com>
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] In state 7, got command 'RCPT', args 'TO:<admin@example.com>'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] In RCPT state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] Got RCPT command
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 36 bytes: '250 Recipient admin@example.com ok\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Received 6 bytes: 'DATA\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] Processing line: DATA
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] In state 7, got command 'DATA', args ''
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] In RCPT state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: RCPT] Got DATA command, switching to DATA state
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Sent 37 bytes: '354 End data with <CR><LF>.<CR><LF>\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] Received 535 bytes: 'Date: Thu, 08 May 2025 14:42:04 +0000\r\nFrom: "Posit Connect" <no-reply@example.com>\r\nSubject: [Posit Connect] Your new Posit Connect account\r\nMime-Version: 1.0\r\nTo: "admin admin" <admin@example.com>\r\nContent-Type: text/plain; charset=UTF-8\r\nContent-Transfer-Encoding: quoted-printable\r\n\r\n\r\nA new account has been created for you on Posit Connect!\r\n\r\nYour new username is "admin".\r\n\r\nPlease visit the following link to confirm your account:\r\nhttp://localhost:3939/__login__?user=3Dadmin&utoken=3D89a7231ec8189013d3ec6=\r\na30d7ffdd8e\r\n.\r\n'
2025/05/08 14:42:04 [SMTP 172.19.0.3:54078] [PROTO: DATA] Got EOF, storing message and switching to MAIL state
2025/05/08 14:42:04 Parsing Content from string: 'Date: Thu, 08 May 2025 14:42:04 +0000
From: "Posit Connect" <no-reply@example.com>
Subject: [Posit Connect] Your new Posit Connect account
Mime-Version: 1.0
To: "admin admin" <admin@example.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable


A new account has been created for you on Posit Connect!


Your new username is "admin".


Please visit the following link to confirm your account:

cgraham-rs · 2025-05-08T14:47:23Z

Getting warmer. Connect no longer complains about email not being configured. However, when Connect fails to deploy content and attempts to send the owner an email we get this error.

tests-1                     | E                       'Failure emailing the error: The email cannot be sent because it '
tests-1                     | E                       'has no valid recipients.'],

I believe this is because we're doing all of our testing via the bootstrap API and there are no actual users in the system.

cgraham-rs · 2025-06-06T18:08:44Z

Revisiting this.

I've updated the test target to set an email address for our boostrap admin user and now there are no errors in the test logs related to email as seen in this latest run.

...
tests-1                     | E                       '2025/06/06 17:48:21.186277059 Error in abs_path(input) : The '
tests-1                     | E                       "file 'does-not-exist.Rmd' does not exist.",
tests-1                     | E                       '2025/06/06 17:48:21.186292558 Calls: local ... eval -> '
tests-1                     | E                       '<Anonymous> -> setwd -> dirname -> abs_path',
tests-1                     | E                       '2025/06/06 17:48:21.186342040 Execution halted',
tests-1                     | E                       'Unable to render the deployed content: Rendering exited '
tests-1                     | E                       'abnormally: exit status 1',
tests-1                     | E                       'Stopped session pings to http://127.0.0.1:41785/'],
tests-1                     | E           'result': { 'data': 'An error occurred while attempting to launch the '
tests-1                     | E                               'content after deployment',
tests-1                     | E                       'type': 'launch-error'}}
tests-1                     | E       assert 1 == 0
tests-1                     | E        +  where 1 = {'id': 'WBBxupmH0Wpd0KSf', 'output': ['Building R Markdown document...', 'Bundle created with R version 4.4.3 (~=4.4) ...ype': 'launch-error'}, 'finished': True, 'code': 1, 'error': 'Rendering exited abnormally: exit status 1', 'last': 548}.error_code
tests-1                     | 
tests-1                     | tests/posit/connect/test_deployments.py:59: AssertionError
tests-1                     | - generated xml file: /connect-extensions/integration/reports/connect-preview.xml -
tests-1                     | =========================== short test summary info ============================
tests-1                     | FAILED tests/posit/connect/test_deployments.py::TestExtensionDeployment::test_extension_deploys
tests-1                     | ======================== 1 failed in 142.55s (0:02:22) =========================
tests-1                     | make: *** [Makefile:385: test] Error 1
Aborting on container exit...

tests-1 exited with code 2

… tests

… run did not fail

…ilure" This reverts commit c422ec7.

…mail" This reverts commit 54a6144.

…ailure" This reverts commit 7508dff.

jonkeane

Thanks for this — a few comments (some of which might open up other bigger cans of worms, so I've got this as "request changes"). The biggest question I have is that I'm a little surprised we don't have an easier / builtin way to bootstrap the stuff that we are doing in set_bootstrap_admin_email.py here. Is that true?

jonkeane · 2025-06-17T18:53:23Z

integration/Makefile

+	@# Change directory to the parent and create tarball with relative paths
+	@cd .. && tar -czf ./integration/bundles/$(EXTENSION_NAME).tar.gz -C ./extensions $(EXTENSION_NAME)


It's not super consequential here, but would pushd and popd be a better option here?

TIL about pushd and popd 👍

jonkeane · 2025-06-17T18:54:23Z

integration/Makefile

-	$(UV) run pytest $(PYTEST_ARGS) \
-					--junit-xml=./reports/$(CONNECT_VERSION).xml | \
-	tee ./logs/$(CONNECT_VERSION).log
+	$(SHELL) -c "set -o pipefail; \


I'm a little surprised by the changes(?) to the shell here? What's the reason for that? It might be listed earlier in this PR and I just forgot it(?)

Also, reading down below this ... is all she same as before, yeah? Other than swapping out -s http://connect:3939 for -s $(CONNECT_SERVER) yeah? Or is my mental diffing not catching more things that are diferent?

Was the only way I could get the command we inserted to create the admin email to work $(MAKE) set-bootstrap-admin-email as it is dependent on the env vars we are setting before it.

jonkeane · 2025-06-17T18:56:00Z

integration/compose.yaml

-      interval: 10s
+      interval: 5s
+      timeout: 5s
+      retries: 3
+      start_period: 5s


Do we need these changes still?

Shaves ~5s from the healthcheck as checking immediately at startup with start_period: 0s apparently fails. Could likely achieve the same with just setting interval: 5s and leaving the others at default.

From the Docker docs:

--interval=DURATION (default: 30s)
--timeout=DURATION (default: 30s)
--start-period=DURATION (default: 0s)
--start-interval=DURATION (default: 5s)
--retries=N (default: 3)

jonkeane · 2025-06-17T18:56:53Z

integration/tests/posit/connect/set_bootstrap_admin_email.py

+    headers = {"Authorization": f"Key {api_key}"}
+
+    # Get current user (this will be the bootstrap admin since we only use the bootstrap user in these tests)
+    user_resp = requests.get(f"{base_url}/__api__/v1/user", headers=headers)


Could we use the posit-sdk for this?

We could. I'll make that change.

jonkeane · 2025-06-17T18:57:41Z

integration/tests/posit/connect/set_bootstrap_admin_email.py

+This allows deployment error emails to be sent to a valid address, otherwise 
+the send will fail and log an error in the Connect logs making it more 
+difficult to debug deployment issues.


Is this actually sending email out? Or is it that we need a valid address or else Connect will bonk and say "I can't send to this nonsense address"?

Hmmm, I'll update the wording here to be more clear.

To clarify we have tackled two issues in this PR:

Connect throws an error if Connect is not configured for email

Connect throws an error if Connect is configured for email, but the publisher has no email configured when Connect attempts to send the content error email

With the changes in this PR Connect is physically sending the email but we have mailhog set to log only so it doesn't leave the box.

Aaaah got it. Great. Thanks, yeah a slightly clearer note would be nice there

cgraham-rs · 2025-06-17T20:39:22Z

I'm a little surprised we don't have an easier / builtin way to bootstrap the stuff that we are doing in set_bootstrap_admin_email.py here. Is that true?

I agree, we're jumping through a lot of hoops here. The documented use of boostrap is to programmatically create the first admin user that can then programmatically create "real" users via the API. However, all of this presents some challenges for us:

When using password auth any new users created must verify their accounts by responding to an email that is sent to them. Even if we tackle that challenge with the goal of using those new users to publish in our tests those new users won't have an API key which you have to create via the UI... That's a really big can of worms.
The bootstrap user is not created with an email address. So the easiest solution seemed to simply add an email address for the boostrap user so we can continue using the bootstrap user to publish content for our tests.

cgraham-rs · 2025-06-17T21:14:54Z

After all of the recent changes this is still working as intended and suppressing all of the email failure logging as seen in the most recent workflow run.

tests-1                     | E                       '2025/06/17 20:57:15.476382523 processing file: '
tests-1                     | E                       'portfolio-report-email.Rmd',
tests-1                     | E                       '2025/06/17 20:57:15.480358421 1/3                  ',
tests-1                     | E                       '2025/06/17 20:57:15.481877436 2/3 [unnamed-chunk-1]',
tests-1                     | E                       '2025/06/17 20:57:15.691612179 Error in `formatter()`:',
tests-1                     | E                       '2025/06/17 20:57:15.691627618 ! could not find function '
tests-1                     | E                       '"formatter"',
tests-1                     | E                       '2025/06/17 20:57:15.692842978 ',
tests-1                     | E                       '2025/06/17 20:57:15.692851814 Quitting from '
tests-1                     | E                       'portfolio-report-email.Rmd:10-25 [unnamed-chunk-1]',
tests-1                     | E                       '2025/06/17 20:57:15.693810184 Execution halted',
tests-1                     | E                       'Unable to render the deployed content: Rendering exited '
tests-1                     | E                       'abnormally: exit status 1',
tests-1                     | E                       'Stopped session pings to http://127.0.0.1:35885/',
tests-1                     | E                       'Failure emailing the error: The email cannot be sent because it '
tests-1                     | E                       'has no valid recipients.'],
tests-1                     | E           'result': { 'data': 'An error occurred while attempting to launch the '
tests-1                     | E                               'content after deployment',
tests-1                     | E                       'type': 'launch-error'}}
tests-1                     | E       assert 1 == 0
tests-1                     | E        +  where 1 = {'id': 'LFJ4evOMuQ5udquq', 'output': ['Building R Markdown document...', 'Bundle created with R version 4.4.3 (~=4.4) ...ype': 'launch-error'}, 'finished': True, 'code': 1, 'error': 'Rendering exited abnormally: exit status 1', 'last': 556}.error_code
tests-1                     | 
tests-1                     | tests/posit/connect/test_deployments.py:59: AssertionError
tests-1                     | - generated xml file: /connect-extensions/integration/reports/connect-preview.xml -
tests-1                     | =========================== short test summary info ============================
tests-1                     | FAILED tests/posit/connect/test_deployments.py::TestExtensionDeployment::test_extension_deploys
tests-1                     | ======================== 1 failed in 152.57s (0:02:32) =========================
tests-1                     | make: *** [Makefile:386: test] Error 1
Aborting on container exit...

…mail" This reverts commit 86f9dc5.

dotNomad

This is looking good to me, and it looks like you addressed all of @jonkeane's feedback. I believe they will still need to approve since they initially requested changes though.

cgraham-rs changed the title ~~Enable Connect config overrides via ENV instead of config file~~ Enable email for Connect integration tests May 8, 2025

cgraham-rs marked this pull request as draft May 9, 2025 19:22

cgraham-rs force-pushed the cgraham/enable-connect-email branch from 8aade44 to 99efc18 Compare June 6, 2025 14:27

cgraham-rs added 10 commits June 17, 2025 14:40

Apply Connect config overrides via ENV instead of config file

236032d

Temporarily break portfolio-report manifest to test Connect email

b15084a

Add mailhog for email logging

7b0ed26

Add makefile command to be able to start/stop connect without running…

fd24ac7

… tests

Set boostrap admin email, fix local packaging

3ea4dde

Temporarily break portfolio-report even more, unclear why previous CI…

24bbeca

… run did not fail

Ensure tests can exit with correct status code to cause CI failure

73b18dc

Revert "Ensure tests can exit with correct status code to cause CI fa…

4ea6b38

…ilure" This reverts commit c422ec7.

Revert "Temporarily break portfolio-report manifest to test Connect e…

c9ad7fb

…mail" This reverts commit 54a6144.

Reapply "Ensure tests can exit with correct status code to cause CI f…

3dbe596

…ailure" This reverts commit 7508dff.

cgraham-rs force-pushed the cgraham/enable-connect-email branch from 1d12786 to 3dbe596 Compare June 17, 2025 18:41

cgraham-rs added 3 commits June 17, 2025 14:52

Update phony and help

46444a6

Remove DEBUG prints

0e162cc

Remove calls to debug-env

6e8373c

jonkeane requested changes Jun 17, 2025

View reviewed changes

cgraham-rs added 4 commits June 17, 2025 15:02

Refactor to consistently use CONNECT_VERSION

4bdd694

Refactor package-extension to use pushd and popd and fix folder creation

cae8152

Make script docstring more understandable

d937ff2

Use posit-sdk in email script

851e6ce

Temporarily break portfolio-report manifest to test Connect email

86f9dc5

cgraham-rs marked this pull request as ready for review June 17, 2025 21:14

Revert "Temporarily break portfolio-report manifest to test Connect e…

62243f9

…mail" This reverts commit 86f9dc5.

cgraham-rs requested a review from toph-allen June 18, 2025 16:09

cgraham-rs requested review from toph-allen, fizzboop and dotNomad June 18, 2025 16:09

dotNomad approved these changes Jun 23, 2025

View reviewed changes

cgraham-rs requested a review from jonkeane June 24, 2025 21:28

		@# Change directory to the parent and create tarball with relative paths
		@cd .. && tar -czf ./integration/bundles/$(EXTENSION_NAME).tar.gz -C ./extensions $(EXTENSION_NAME)

Enable email for Connect integration tests #90

Are you sure you want to change the base?

Enable email for Connect integration tests #90

Conversation

cgraham-rs commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgraham-rs commented May 8, 2025

Uh oh!

cgraham-rs commented Jun 6, 2025

Uh oh!

jonkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgraham-rs commented Jun 17, 2025

Uh oh!

cgraham-rs commented Jun 17, 2025

Uh oh!

dotNomad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cgraham-rs commented May 1, 2025 •

edited

Loading