Skip to content

fix(collector): honor clustering.tls for leader→member API calls#892

Open
vuthinh0301 wants to merge 1 commit into
openconfig:mainfrom
vuthinh0301:fix/collector-clustering-tls
Open

fix(collector): honor clustering.tls for leader→member API calls#892
vuthinh0301 wants to merge 1 commit into
openconfig:mainfrom
vuthinh0301:fix/collector-clustering-tls

Conversation

@vuthinh0301

Copy link
Copy Markdown

PR title

fix(collector): honor clustering.tls for leader→member API calls

PR body

Problem

In collector mode (gnmic collect), the cluster leader's HTTP clients that call other members' REST APIs (target assignment / unassignment / deletion) were plain &http.Client{} instances that never loaded the clustering.tls configuration — the client in pkg/collector/managers/cluster/cluster_manager.go even carried a literal // TODO:.

As a result, a TLS-secured API broke target dispatch:

  • private-CA server cert on api-server.tlsx509: certificate signed by unknown authority
  • mTLS (api-server.tls.client-auth: require-verify) → the dispatch client presents no client cert, so the leader gets remote error: tls: bad certificate calling members, including itself → no target ever assigned, no telemetry.

This works in subscribe mode, where clustering.tls was wired up in #544 (v0.39.0, for #529). Collector mode (v0.43.0) has a separate cluster implementation that never got the same treatment. The config was already parsed and env-expanded (pkg/config/clustering.go, pkg/collector/env/env.go:18-21) but never consumed anywhere under pkg/collector/.

Fix

Add newClusteringHTTPClient(*config.Clustering) which builds the client from clustering.tls via utils.NewTLSConfig(...), mirroring (*App).createAPIClient in pkg/app/clustering.go. Inject it into both the ClusterManager.apiClient (replacing the // TODO: client) and the assigner (NewAssigner now takes the client).

When clustering.tls is unset the behavior matches subscribe mode (skip-verify fallback), so plain-HTTP and public-CA setups are unaffected.

Testing

  • go build ./... — OK (go 1.25)
  • go vet ./pkg/collector/managers/cluster/... — clean
  • go test ./pkg/collector/managers/cluster/... ./pkg/app/... — pass
  • Manually verified end-to-end on a 3-instance gnmic collect cluster + Consul + Nokia SR Linux target: before the fix, enabling api-server.tls with a private CA (mTLS) produced the x509/bad certificate errors above and no target was assigned; after the fix, with matching clustering.tls, the leader dispatches over HTTPS+mTLS, the lock is acquired, and telemetry flows. Subscribe mode with the same config already worked and still does.

Files

  • pkg/collector/managers/cluster/cluster_manager.go
  • pkg/collector/managers/cluster/assigner.go
  • pkg/collector/managers/cluster/cluster_integration_test.go

@google-cla

google-cla Bot commented Jul 15, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

In collector mode the cluster leader dispatched targets to members using
plain http.Client instances that never loaded the clustering.tls config
(cluster_manager.go carried a `// TODO:` on the client). As a result a
TLS-secured API (private-CA server cert, or mTLS via
api-server.tls.client-auth=require-verify) broke target assignment with
`x509: certificate signed by unknown authority` / `tls: bad certificate`,
even for the leader calling itself.

The clustering.tls config was already parsed and env-expanded
(pkg/config/clustering.go, pkg/collector/env/env.go) but never consumed
under pkg/collector/.

Build the inter-member HTTP client from clustering.tls, mirroring
(*App).createAPIClient in subscribe mode (pkg/app/clustering.go), and
inject it into both the ClusterManager apiClient and the assigner. When
clustering.tls is unset the behavior matches subscribe mode (skip-verify
fallback), so plain-HTTP and public-CA setups are unaffected.
@vuthinh0301
vuthinh0301 force-pushed the fix/collector-clustering-tls branch from 16c4542 to a8e1165 Compare July 15, 2026 13:13
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.

1 participant