Replace Ruby bosh-nats-sync with Golang implementation - #2746
Draft
aramprice wants to merge 3 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
2 times, most recently
from
June 19, 2026 23:49
7577681 to
d923c47
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
June 20, 2026 00:15
d923c47 to
b468aaa
Compare
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
4 times, most recently
from
June 20, 2026 01:47
84f41f0 to
8d773dc
Compare
colins
reviewed
Jun 23, 2026
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
2 times, most recently
from
June 25, 2026 20:36
fddaf21 to
5980267
Compare
colins
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
June 26, 2026 18:31
118abae to
5b1d58a
Compare
aramprice
added a commit
that referenced
this pull request
Jul 9, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 9, 2026 17:47
35830f0 to
87e76cb
Compare
aramprice
added a commit
that referenced
this pull request
Jul 9, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 9, 2026 22:46
87e76cb to
1e15707
Compare
aramprice
added a commit
that referenced
this pull request
Jul 13, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 13, 2026 16:30
1e15707 to
e94d5d2
Compare
aramprice
added a commit
that referenced
this pull request
Jul 13, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 13, 2026 23:49
e94d5d2 to
7e3388f
Compare
aramprice
added a commit
that referenced
this pull request
Jul 14, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 14, 2026 16:27
7e3388f to
8befa9e
Compare
aramprice
added a commit
that referenced
this pull request
Jul 16, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 16, 2026 21:58
8befa9e to
9061c91
Compare
aramprice
added a commit
that referenced
this pull request
Jul 17, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
July 17, 2026 19:12
9061c91 to
cd58b16
Compare
aramprice
added a commit
that referenced
this pull request
Jul 17, 2026
…olled tls.Config Address item 1 in PR review comment #2746 (comment): authprovider and userssync both built their director/UAA TLS client by hand (manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF already ships code.cloudfoundry.org/tlsconfig as the standard helper for exactly this, and the reviewer noted the hand-rolled config was actually *less* hardened than the org-standard one (no MaxVersion, no cipher-suite pinning). Deliberately does NOT touch items 2/3 from the same comment (reusing bosh-cli's director/uaa packages for the director API and UAA calls) per explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency; that remains a footprint-vs-reuse tradeoff for a future PR, not something to pull in incidentally here. pkg/authprovider/auth_provider.go: - buildHTTPClient now builds its *tls.Config via tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...), applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath() resolves to a file with non-empty content. WithExternalServiceDefaults() pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate cipher-suite list, in place of the previous MinVersion-only config. - Preserves existing behavior exactly: a configured-but-unreadable CA file is still a hard error, an empty file still falls back to the system trust store, and a malformed (but present/non-empty) cert is still a hard error. pkg/userssync/users_sync.go: - buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern for the director API client. Replaced directorCACertPool (manual x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path) predicate that gates whether tlsconfig.WithAuthorityFromFile is applied. - Preserves existing behavior: director_ca_cert missing/unreadable/empty falls back to the system trust store (no error, matching Ruby's usable_director_ca_cert?), while a configured-but-unparseable cert is a hard error rather than a silent fallback. pkg/userssync/export_test.go: - Replaced the removed DirectorCACertPool test hook with BuildDirectorTLSConfig, which builds the real *http.Client via buildHTTPClient and returns its *tls.Config, so tests assert against the actual TLS configuration (RootCAs, MinVersion) rather than an internal CertPool-returning helper that no longer exists. pkg/userssync/users_sync_test.go: - Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig: same four cases (not configured / missing file / whitespace-only file / unparseable content) now assert on tlsCfg.RootCAs and the error message, plus a new case asserting MinVersion is pinned to TLS 1.2 via WithExternalServiceDefaults. Dependency footprint: go.mod/go.sum/vendor pick up code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency). tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto, golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness but are not imported by any non-test file, so `go mod vendor` does not pull them into vendor/ or the build. Verified `go build`, `go vet`, and `go test -race ./...` all pass with both the default module resolution and explicit `-mod=vendor` (matching how packages/nats/packaging invokes go build); gofmt and golangci-lint are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
5 times, most recently
from
July 17, 2026 23:03
8619ba8 to
b9a9ff5
Compare
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
2 times, most recently
from
August 7, 2026 19:16
7d32916 to
bbefe1e
Compare
aramprice
force-pushed
the
experiment-golang-bosh-nats-sync
branch
from
August 19, 2026 21:52
bbefe1e to
dc10816
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bosh-nats-syncgem with a Go binary (src/bosh-nats-sync/) that mirrors all Ruby behavior including TLS verification preferences (uaa_ca_cert>director_ca_cert), HTTP peer verification, and startup reliabilitynatsBOSH job and package (director-ruby-3.3, gem bundling,BUNDLE_GEMFILE/GEM_HOMEenv vars)go.yml); removes the defunctnats_sync:parallelRuby matrix entry fromruby.ymlCommits
Golang bosh-nats-sync — full Go implementation with parity to the Ruby version:
uaa_ca_certpreferred overdirector_ca_cert)InsecureSkipVerify)isConnectionErrorretry logic coveringdeadline exceeded/eof(matching Ruby'sNet::OpenTimeout,Net::ReadTimeout,Errno::ECONNRESET)Remove ruby-isms now that nats is golang — strips Ruby from
jobs/nats/,packages/nats/,src/Gemfile, and CI:packages/nats/packagingnow builds the Go binary viago buildjobs/nats/wrapper script calls Go binary directly (no Ruby runtime sourcing)go.yml: addslint (bosh-nats-sync)andtest (bosh-nats-sync)jobsruby.yml: removesnats_sync:parallelmatrix entrysrc/bosh-nats-sync/.golangci.ymlTest plan
go.yml/ lint (bosh-nats-sync) passesgo.yml/ test (bosh-nats-sync) passesruby.yml/ unit_specs passes for all remaining sub-projects (common:parallel,monitor:parallel,release)spec/nats_templates_spec.rb) pass