Enable mTLS for PG sources#4468
Conversation
🔄 Flaky Test DetectedAnalysis: The flow/e2e package hit its exact 1200s global test timeout on a single unrelated MySQL matrix shard with no panic, data race, assertion failure, or TLS error — the classic signature of a flaky e2e timeout rather than a regression from this mTLS-for-Postgres PR. ✅ Automatically retrying the workflow |
|
🔄 Flaky Test DetectedAnalysis: The maria/8.0 job failed at the Docker setup step with a transient Docker Hub registry network error pulling the MariaDB image (exit code 125) before any test ran, indicating infrastructure flakiness rather than a code bug. ✅ Automatically retrying the workflow |
10279e4 to
50d32ed
Compare
❌ Test FailureAnalysis: All matrix jobs fail deterministically with a shell "syntax error: unexpected end of file" in the CI catalog-setup step before any test runs, indicating a real malformed-workflow-script bug, not flakiness. |
❌ Test FailureAnalysis: The CI setup step "create postgres extensions...setup catalog database" fails deterministically with a shell "syntax error: unexpected end of file" — caused by an inline # comment the mTLS PR added inside a &&-chained command — so tests never ran; this is a real bug in the PR's setup script, not a flaky test. |
❌ Test FailureAnalysis: A deterministic bash syntax error ("unexpected end of file") in the CI setup step fails identically across all matrix jobs before any test runs, indicating a real malformed-workflow bug rather than a flaky test. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
50d32ed to
ddb4782
Compare
ddb4782 to
e2fcbe1
Compare
| t.Parallel() | ||
|
|
||
| if _, ok := internal.GetMutualTLSPostgresConfigFromEnv(); !ok { | ||
| t.Skip("mutual-TLS Postgres fixtures not configured; " + |
There was a problem hiding this comment.
This would silently skip if something becomes unset. If it's possible to test locally, can just require them, if not, can follow the FLOW_TESTS_RDS_IAM_AUTH_SKIP=true pattern
There was a problem hiding this comment.
This test is running correctly in CI and everything is set in local Tilt for it to work by default so I think the best approach is just to run it unconditionally. This condition is a trace for initial iterations where CI didn't support the cert yet.
Addressed in b129d16
|
|
||
| message ClientTlsConfig { | ||
| string certificate = 1 [(peerdb_redacted) = true]; | ||
| string private_key = 2 [(peerdb_redacted) = true]; |
There was a problem hiding this comment.
Add to APITestSuite.TestSchemaEndpoints? At least as a test that nested redacted fields are respected
…prostgres/tls/mutual-authentication
2 participants
This Pull Request allows users to add client authentication (PeerDB acting as client here) through mTLS by expanding the PG configuration contract to optionally include the client private certificate and its key.
It also includes PeerDB UI changes.
Part of: https://linear.app/clickhouse/issue/DBI-793