Skip to content
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

feat: support PG Notify for event streams using credentials #1176

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

mkanoor
Copy link
Contributor

@mkanoor mkanoor commented Jan 7, 2025

Create a Postgres credential that can store certificates and keys or userid/password at server startup and attach it to Activations that use EventStream.

This allows us to test mTLS for Postgres, a sample pg_hba file is attached to this PR.

To test this you need to create certificates and keys for

  • Postgres Server called (server.crt and server.key)
  • EDA Server called (client.crt and client.key)
  • You also need to create the CA certificate (ca.crt)

These files have to be present in
tools/docker/postgres_ssl_config/certs

The docker-compose file tools/docker/docker-compose-mac-pg-mtls.yml can be used to test mTLS

The docker-compose file tools/docker/docker-compose-mac.yml can be used to test userid/password auth

https://issues.redhat.com/browse/AAP-38315

@mkanoor mkanoor requested a review from a team as a code owner January 7, 2025 19:25
@mkanoor mkanoor marked this pull request as draft January 7, 2025 19:26
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 2 times, most recently from 0d6fc17 to 28ea7af Compare January 7, 2025 23:05
@mkanoor mkanoor added run-e2e and removed run-e2e labels Jan 7, 2025
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 2 times, most recently from c429d87 to 3fa5f37 Compare January 14, 2025 18:00
@mkanoor mkanoor marked this pull request as ready for review January 14, 2025 18:01
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 3 times, most recently from f77b4af to bda4178 Compare January 15, 2025 17:16
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 3 times, most recently from 732ef26 to bd60022 Compare January 17, 2025 21:02
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 2 times, most recently from 0b9cc22 to a721d08 Compare January 20, 2025 16:18
@mkanoor
Copy link
Contributor Author

mkanoor commented Jan 20, 2025

The api-e2e-tests work but the api-e2e-multinode-tests fails, seems like a flaky test unrelated to changes in this PR.

zkayyali812
zkayyali812 previously approved these changes Jan 21, 2025
Copy link
Collaborator

@zkayyali812 zkayyali812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, @mkanoor can you also add some documentation that explains how this can be tested?

hsong-rh
hsong-rh previously approved these changes Jan 21, 2025
@mkanoor
Copy link
Contributor Author

mkanoor commented Jan 21, 2025

In order to test the mTLS for postgres you need to generate certificates for Postgres Server and for our EDA Server and Activation.
The server.crt will be used by Postgres
The client.crt and client.key will be used by EDA Server, Websocket Worker, Event Stream Worker, Activation (ansible-rulebook)

You can use step-ca to generate certs, follow the instructions to install step-ca , once step-ca is installed and running locally you can generate the certs

For docker running on Mac

cd tools/docker/postgres_ssl_config/certs

1. step ca certificate -san postgres -san host.containers.internal -san localhost -san 127.0.0.1 "localhost" server.crt server.key

2. step ca certificate -san postgres  -san localhost -san 127.0.0.1 postgres client.crt client.key

3. step ca root > ca.crt

Run docker-compose with ./tools/docker/docker-compose-mac-pg-mtls.yml

Create an Event Stream with Basic Auth Credentials
Create an Activation and attach the Event Stream to Activation
Send the data to Event Stream using curl
The data should get to the Running Activation which should have verified itself to the Postgres Server using certificates
Co

@mkanoor mkanoor dismissed stale reviews from hsong-rh and zkayyali812 via 4088969 January 22, 2025 22:12
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch 3 times, most recently from a8bd3c6 to dde20b3 Compare January 22, 2025 22:41
src/aap_eda/settings/default.py Outdated Show resolved Hide resolved
Create a Postgres credential that can store certificates
and keys or userid/password at server startup and attach it to
Activations that use EventStream.

This allows us to test mTLS for Postgres, a sample pg_hba file
is attached to this PR.

To test this you need to create certificates and keys for
 * Postgres Server called (server.crt and server.key)
 * EDA Server called (client.crt and client.key)
 * You also need to create the CA certificate

These files have to be present in
  tools/docker/postgres_ssl_config/certs

The docker-compose file tools/docker/docker-compose-mac-pg-mtls.yml
can be used to test mTLS

The docker-compose file tools/docker/docker-compose-mac.yml can
be used to test userid/password auth
@mkanoor mkanoor force-pushed the pg_notify_with_credential branch from dde20b3 to 50a86fe Compare January 23, 2025 15:03
Copy link
Collaborator

@Alex-Izquierdo Alex-Izquierdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tested it because it would require some time as I have to replicate the env for linux systems (only mac provided) but so far, LGTM.
We need to revisit how we handle the database and pgnotify config as we were discussing.

@mkanoor mkanoor merged commit b3fc0d9 into ansible:main Jan 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants