pjfanning opened a new pull request, #965: URL: https://github.com/apache/pekko-management/pull/965
### Motivation `ConsulServiceDiscovery` loaded its CA file with `CertificateFactory.generateCertificate`, which stops at the first PEM block, and stored the result under a single `ca` alias. A `pekko.discovery.pekko-consul.ca-path` bundling a root plus intermediates, or two roots while a CA is being rotated, silently trusted only the first certificate — Consul presenting a certificate issued by any other CA in the bundle was rejected at handshake time. This is the same defect that apache/pekko#3528 fixed for `RotatingKeysSSLEngineProvider`. The `management-pki` `PemManagersProvider` used by the Kubernetes discovery, lease and rolling-update modules already loads bundles correctly, so only the Consul path was affected. While here: neither `ConsulServiceDiscovery` nor `PemManagersProvider.loadCertificates` closed the stream it opened over the CA file. The chain-building half of apache/pekko#3528 (`buildCertificateChain`, `signedBy`) is not applicable here — nothing in this repo presents its own certificate chain to a peer; `PemManagersProvider.createSslContext` deliberately inits with empty key material because these clients authenticate with a bearer token. ### Modification - Add `ConsulServiceDiscovery.loadCertificates`, reading every certificate in the file with `generateCertificates` and closing the stream. - Add `ConsulServiceDiscovery.buildTrustManagers`, giving each certificate its own alias so no certificate hides another. - A CA file that yields no certificates now fails with a clear `IllegalArgumentException` naming the path, preserving the previous fail-fast behaviour (`generateCertificate` used to throw on an empty file). - Close the stream in `PemManagersProvider.loadCertificates`. - Note bundle support on `ca-path` in the Consul `reference.conf`. Both new methods are `private[consul]` / `@InternalApi`; no public API change. ### Result Consul TLS verification trusts every certificate in the configured CA file, so root-plus-intermediate bundles and CA rotation both work, and neither certificate loader leaks a file handle. ### Tests - `sbt "discovery-consul/Test/testOnly org.apache.pekko.discovery.consul.ConsulCaCertificateSpec"` — 3 passed. New spec covers a single-certificate file, a two-certificate bundle, and an empty file. Directional: against the old `generateCertificate` logic the bundle case trusts only `CN=pekko-consul-test-root-1`, so the test fails before the fix. - `sbt "discovery-kubernetes-api/test"` — 11 passed - `sbt "management-cluster-bootstrap/Test/compile" "lease-kubernetes/Test/compile" "rolling-update-kubernetes/Test/compile"` — ok - `sbt "++3.3.8; discovery-consul/Test/compile; management-pki/compile"` — ok - `sbt "discovery-consul/mimaReportBinaryIssues" "management-pki/mimaReportBinaryIssues" "headerCheckAll"` — ok - `scalafmt --mode diff-ref=origin/main` — clean - `ConsulDiscoverySpec` not run — requires Docker/testcontainers, unavailable locally. Test fixtures `ca-single.crt` / `ca-bundle.crt` are self-signed certificates generated with `openssl` for this test; they are not copied from any external project. ### References Refs apache/pekko#3528 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
