Provide ClickHouse Peers validations as a library by pfcoperez · Pull Request #4288 · PeerDB-io/peerdb

pfcoperez · 2026-05-08T11:30:24Z

This Pull Request introduces the possibility of performing ClickHouse peers validations outside PeerDB while
offering maximum re-utilization of the validation logic.

To achieve this, it offers four main changes:

Staging bucket validation logic is abstracted out behind the StagingValidator type which is implemented as a function taking a context and potentially returning an error. The logic used at PeerDB to implement this function is shared at flow/pkg for external code to use through NewS3StagingValidator and NewGCSStagingValidator factory methods.
In a similar vein, accepted ClickHouse hosts becomes a parameter for the ClickHouse PeerValidation logic.
Which in turn is shared at flow/pkg through the ValidateClickHousePeer method. This method is also used within PeerDB when peer validation is not disabled.
~~Hence, peer validation can be now disabled for any peer in the gRPC methods CreatePeer and ValidatePeer using a new optional parameter: disable_connector_validation~~ We assume validation happens both upstream and at PeerDB provision side
We have disabled bucket checks at ClickHouse validation time, this was actually testing static infrastructure.

Part of DBI-65

Copilot

Pull request overview

This PR refactors ClickHouse peer validation so it can be reused as a standalone library (flow/pkg), and adds a gRPC option to skip connector-side validations while still performing basic connectivity checks.

Changes:

Added disable_connector_validation to ValidatePeer and CreatePeer requests and wired it through the Flow API handlers.
Extracted ClickHouse peer validation + staging-bucket smoke tests into flow/pkg/clickhouse as reusable helpers (ValidateClickHousePeer, NewS3StagingValidator, NewGCSStagingValidator).
Updated ClickHouse connector validation and added E2E/test coverage for the new staging validators and bypass behavior.

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
protos/route.proto	Adds optional `disable_connector_validation` to peer-validation related RPC requests.
flow/shared/random.go	Routes `RandomString` through `flow/pkg/common` to share logic across modules.
flow/pkg/go.mod	Adds dependencies needed for reusable staging validators and ClickHouse validation library.
flow/pkg/go.sum	Updates sums for new/updated `flow/pkg` dependencies.
flow/pkg/common/utils.go	Introduces shared `RandomString` helper in the `flow/pkg` module.
flow/pkg/clickhouse/validation.go	Adds reusable ClickHouse peer validation API and host allow-list validation logic.
flow/pkg/clickhouse/validation_test.go	Minor test struct field ordering adjustment.
flow/pkg/clickhouse/staging_validators.go	Adds reusable S3/GCS staging bucket smoke-test validators.
flow/pkg/clickhouse/staging_validators_test.go	Adds unit tests for the new S3/GCS staging validators.
flow/go.mod	Updates main Flow module dependencies (incl. versions aligned with `flow/pkg`).
flow/go.sum	Updates sums for main Flow module dependency changes.
flow/e2e/api_test.go	Adds E2E test ensuring connector validation can be bypassed for ClickHouse.
flow/connectors/clickhouse/staging_s3.go	Switches S3 staging validation to the shared `flow/pkg` validator.
flow/connectors/clickhouse/staging_gcs.go	Switches GCS staging validation to the shared `flow/pkg` validator.
flow/connectors/clickhouse/clickhouse.go	Switches ClickHouse connector validation to the shared `ValidateClickHousePeer`.
flow/cmd/validate_peer.go	Implements connector-validation bypass while keeping connectivity checks.
flow/cmd/handler.go	Plumbs `disable_connector_validation` from `CreatePeer` into `ValidatePeer`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-05-08T11:41:53Z

❌ Test Failure

Analysis: TestClickHousePeerValidationBypass panics deterministically at api_test.go:1251 because it calls t.Setenv inside a parallel test suite (e2eshared.RunSuite uses t.Parallel), which Go's testing framework forbids — this is a real bug, not a flaky failure.
Confidence: 0.97

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

github-actions · 2026-05-08T12:21:26Z

🔄 Flaky Test Detected

Analysis: The e2e test TestGenericBQ/Test_Inheritance_Table_With_Dynamic_Setting timed out after 195s with an "UNEXPECTED TIMEOUT" waiting for BigQuery row counts to converge (destination had 6 rows while source had 3), a classic WaitFor polling timeout pattern in CDC streaming tests indicating a race condition or timing sensitivity rather than a deterministic code bug.
Confidence: 0.78

✅ Automatically retrying the workflow

View workflow run

github-actions · 2026-05-08T13:04:44Z

🔄 Flaky Test Detected

Analysis: The entire e2e test suite hit Go's global 900-second timeout (panic: test timed out after 15m0s), killing all remaining tests simultaneously — this is a CI infrastructure slowness issue, not a real code bug.
Confidence: 0.95

✅ Automatically retrying the workflow

View workflow run

pfcoperez · 2026-05-11T16:26:41Z

@copilot resolve the merge conflicts in this pull request

CLAassistant · 2026-05-11T16:33:14Z

All committers have signed the CLA.

Copilot · 2026-05-11T16:36:16Z

@copilot resolve the merge conflicts in this pull request

Merge conflicts are resolved and pushed in commit 26d449b.

…ed by common code.

…e used outside peerdb

…rnally at PeerDB and made available for external users of flow/pkg to use

…act. This allows skipping checks that can happen upstream in the API calls chain.

…ly required. Absent is not skipping is actually what we want, this commit simplifies the protocol

…ClickHousePeer`

This reverts commit 2eaba06.

…nal on the boolean flag is not actually required. Absent is not skipping is actually what we want, this commit simplifies the protocol" This reverts commit a389643.

This reverts commit c213c28.

…n` optional field to PeerDB API contract." This reverts commit 6361f08.

codecov · 2026-05-12T09:32:54Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
2212	2	2210	189

View the top 2 failed test(s) by shortest run time

github.com/PeerDB-io/peerdb/flow/e2e::TestApiPg

Stack Traces | 0.01s run time

=== RUN   TestApiPg
=== PAUSE TestApiPg
=== CONT  TestApiPg
--- FAIL: TestApiPg (0.01s)
2026/05/12 11:12:11 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/12 11:12:11 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}

github.com/PeerDB-io/peerdb/flow/e2e::TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch

Stack Traces | 0.05s run time

=== RUN   TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch
=== PAUSE TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch
=== CONT  TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch
2026/05/12 11:10:35 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/12 11:10:35 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/12 11:10:35 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/12 11:10:35 ERROR Failed to get key x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} error="FATAL: terminating connection due to administrator command (SQLSTATE 57P01)"
2026/05/12 11:10:35 ERROR Failed to parse as int64 x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} key=PEERDB_CLICKHOUSE_MAX_INSERT_THREADS error="failed to get key: FATAL: terminating connection due to administrator command (SQLSTATE 57P01)"
    e2eshared.go:34: 
        	Error Trace:	.../flow/e2e/clickhouse.go:415
        	            				.../flow/e2e/api_test.go:233
        	            				.../flow/e2eshared/e2eshared.go:34
        	Error:      	Received unexpected error:
        	            	failed to open connection to ClickHouse peer: failed to load max_insert_threads config: failed to parse PEERDB_CLICKHOUSE_MAX_INSERT_THREADS as int64: failed to get key: FATAL: terminating connection due to administrator command (SQLSTATE 57P01)
        	Test:       	TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch
--- FAIL: TestApiPg/TestPostgresDestinationValidation_UserDefinedTypeMismatch (0.05s)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

github-actions · 2026-05-12T09:35:06Z

❌ Test Failure

Analysis: Deterministic build failure: internal/env.go imports cloud.google.com/go/kms/apiv1 and cloud.google.com/go/kms/apiv1/kmspb but go.sum is missing the required entries — run go get github.com/PeerDB-io/peerdb/flow/internal to fix.
Confidence: 0.98

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

it's redundant and checks a piece of infrastructure which is static and reliable.

github-actions · 2026-05-12T09:53:53Z

❌ Test Failure

Analysis: Real build failure: missing go.sum entries for cloud.google.com/go/kms/apiv1 and cloud.google.com/go/kms/apiv1/kmspb imported in internal/env.go — go mod tidy was not run after adding these imports.
Confidence: 0.98

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

…y to limit the changeset

claude · 2026-05-12T10:30:58Z

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

serprex · 2026-05-12T20:17:37Z

 	gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-)
+)


jgao54

lgtm!

jgao54 · 2026-05-12T22:15:59Z

+	// To enable it back, just replace `objectstore.NoStagingValidator`
+	// with `c.staging.Validate`.
+	if err := peerdb_clickhouse.ValidateClickHousePeer(
+		ctx, c.logger, allowedDomains, c.Config.Host, c.database, objectstore.NoStagingValidator,


actually, because of OSS validation flow via /api/v1/peers/validate, we probably want to set this to c.staging.Validate still; since with OSS user set up staging themselves and and validation is still useful.
(the side-effect of this is that CreatePeer will re-validate object store, which is fine for now as we have been running this pattern for a while).

The alternative is passing in an option to CreatePeerRequest to skip object storage validation and thread it thru, but for now I think it's okay to just validate during peer creation

pfcoperez requested a review from a team as a code owner May 8, 2026 11:30

pfcoperez requested a review from Copilot May 8, 2026 11:33

Copilot started reviewing on behalf of pfcoperez May 8, 2026 11:33 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

pfcoperez force-pushed the DBI-65/shared-ch-validations branch from f71810c to c213c28 Compare May 8, 2026 11:54

pfcoperez force-pushed the DBI-65/shared-ch-validations branch from 86ca2cd to 2eaba06 Compare May 8, 2026 12:55

pfcoperez requested a review from jgao54 May 8, 2026 13:17

Copilot started work on behalf of pfcoperez May 11, 2026 16:26 View session

Copilot finished work on behalf of pfcoperez May 11, 2026 16:37

jgao54 reviewed May 12, 2026

View reviewed changes

Comment thread flow/shared/random.go Outdated

Comment thread flow/pkg/objectstore/validation.go

Comment thread protos/route.proto Outdated

pfcoperez added 14 commits May 12, 2026 11:20

Move shared#RamdomString implementation to flow/pkg so it can be us…

9519814

…ed by common code.

Factor out into flow/pkg ClickHouse service validations so they can b…

0519f7e

…e used outside peerdb

Dependency injection interface for Staging checks in peerdb/pkg

857d9cb

Factories for Staging checks injected validation. These are used inte…

f63f8c0

…rnally at PeerDB and made available for external users of flow/pkg to use

Move hostname validator to flow/pkg check

a755db2

Fix linting issues

e508fbf

Add disable_connector_validation optional field to PeerDB API contr…

5dacc7d

…act. This allows skipping checks that can happen upstream in the API calls chain.

Skip peer validation e2e test

8fd3302

Remove local debugging session logs

9123f30

Since we are using proto3, optional on the boolean flag is not actual…

deef77e

…ly required. Absent is not skipping is actually what we want, this commit simplifies the protocol

Update UI

9683c3c

Bug: Missing host validation in the checks that are part of `Validate…

bb07003

…ClickHousePeer`

Unconditional CH Peer check: Revert "Update UI"

0c209e6

This reverts commit 2eaba06.

Unconditional CH Peer check: Revert "Since we are using proto3, optio…

0f18c7a

…nal on the boolean flag is not actually required. Absent is not skipping is actually what we want, this commit simplifies the protocol" This reverts commit a389643.

pfcoperez added 2 commits May 12, 2026 11:29

Unconditional CH Peer check: Revert "Skip peer validation e2e test"

83d583f

This reverts commit c213c28.

Unconditional CH Peer check: Revert "Add `disable_connector_validatio…

4905fec

…n` optional field to PeerDB API contract." This reverts commit 6361f08.

pfcoperez force-pushed the DBI-65/shared-ch-validations branch from 26d449b to 4905fec Compare May 12, 2026 09:29

Let's skip staging validation at ClickHouse connector checks,

bb0ba1e

it's redundant and checks a piece of infrastructure which is static and reliable.

PR Comments: Use pkg/common#RamdonString directly, no need for a prox…

fed4fae

…y to limit the changeset

pfcoperez requested review from a team as code owners May 12, 2026 10:25

PeerDB-io deleted a comment from claude Bot May 12, 2026

PR Comments: Move staging object store definitions to their own package.

cb2d73c

Fix pinned version bump

9a9670f

pfcoperez requested a review from jgao54 May 12, 2026 10:57

pfcoperez changed the title ~~Provide ClickHouse Peers validations as a library and allow skipping peer validations via CreatePeer and ValidatePeer request parameters~~ Provide ClickHouse Peers validations as a library May 12, 2026

Align dependencies with main

adfe7f4

serprex reviewed May 12, 2026

View reviewed changes

serprex approved these changes May 12, 2026

View reviewed changes

jgao54 approved these changes May 12, 2026

View reviewed changes

jgao54 reviewed May 12, 2026

View reviewed changes

Conversation

pfcoperez commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 8, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 8, 2026

🔄 Flaky Test Detected

Uh oh!

github-actions Bot commented May 8, 2026

🔄 Flaky Test Detected

Uh oh!

pfcoperez commented May 11, 2026

Uh oh!

CLAassistant commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI commented May 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 2 Tests Failed:

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

claude Bot commented May 12, 2026

Code review

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 12, 2026

🔄 Flaky Test Detected

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 12, 2026

❌ Test Failure

Uh oh!

serprex May 12, 2026

Choose a reason for hiding this comment

Uh oh!

jgao54 left a comment

Choose a reason for hiding this comment

Uh oh!

jgao54 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pfcoperez commented May 8, 2026 •

edited

Loading

CLAassistant commented May 11, 2026 •

edited

Loading

codecov Bot commented May 12, 2026 •

edited

Loading

jgao54 May 12, 2026 •

edited

Loading