Replace Ruby bosh-nats-sync with Golang implementation

[aramprice]

Summary

• Replaces the Ruby bosh-nats-sync gem 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 reliability
• Removes all Ruby runtime dependencies from the nats BOSH job and package (director-ruby-3.3, gem bundling, BUNDLE_GEMFILE/GEM_HOME env vars)
• Adds GitHub Actions CI coverage for the new Go code (lint + test jobs in go.yml); removes the defunct nats_sync:parallel Ruby matrix entry from ruby.yml

Commits

1. Golang bosh-nats-sync — full Go implementation with parity to the Ruby version:

   • Correct TLS CA selection (uaa_ca_cert preferred over director_ca_cert)
   • Proper Director API TLS peer verification (no InsecureSkipVerify)
   • isConnectionError retry logic covering deadline exceeded / eof (matching Ruby's Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET)
   • Fatal startup behavior on failed NATS reload (matching Ruby)
   • Full Ginkgo/Gomega test suite

2. Remove ruby-isms now that nats is golang — strips Ruby from jobs/nats/, packages/nats/, src/Gemfile, and CI:

   • packages/nats/packaging now builds the Go binary via go build
   • jobs/nats/ wrapper script calls Go binary directly (no Ruby runtime sourcing)
   • CI go.yml: adds lint (bosh-nats-sync) and test (bosh-nats-sync) jobs
   • CI ruby.yml: removes nats_sync:parallel matrix entry
   • Adds src/bosh-nats-sync/.golangci.yml

Test plan

• go.yml / lint (bosh-nats-sync) passes
• go.yml / test (bosh-nats-sync) passes
• ruby.yml / unit_specs passes for all remaining sub-projects (common:parallel, monitor:parallel, release)
• Release ERB template specs (spec/nats_templates_spec.rb) pass

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: daeaf7d0-37a8-4b5c-aa52-dc65f53ee37d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch experiment-golang-bosh-nats-sync

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Actionable comments posted: 14

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/go.yml:
- Line 21: The actions/checkout steps lack the persist-credentials parameter
which increases the risk of token reuse in subsequent steps and artifacts. Add
the parameter `persist-credentials: false` to both instances of the
`actions/checkout@v7` action to disable credential persistence and reduce token
exposure risk. This parameter should be added as a separate line within the uses
section for both checkout actions.
- Around line 21-22: Replace the GitHub Actions version tag pins with full
commit SHA pins for enhanced security. Update the `uses:` entries for
`actions/checkout@v7` and `actions/setup-go@v6` (and any other actions mentioned
in the comment) to use their full commit SHA instead of the `@v*` tag format.
This applies to all `uses:` statements throughout the workflow file where
actions are currently pinned to version tags rather than specific commit hashes.

In `@src/bosh-nats-sync/go.mod`:
- Line 3: The Go toolchain and dependencies contain multiple security
vulnerabilities that must be addressed before release. Update the go directive
from version 1.26.1 to 1.26.3 to resolve five known security vulnerabilities.
Additionally, upgrade golang.org/x/net from v0.49.0 to v0.53.0 to address the
GO-2026-4918 vulnerability affecting net/http infinite loops, and upgrade
golang.org/x/sys from v0.40.0 to v0.44.0 to address the GO-2026-5024
vulnerability affecting sys/windows integer overflow. After making these version
updates in the go.mod file, refresh the module graph by running go mod tidy to
ensure all transitive dependencies are properly resolved and consistent.

In `@src/bosh-nats-sync/pkg/authprovider/auth_provider_test.go`:
- Around line 305-308: The test assertion in the "Token expiration deadline"
describe block is tautological because it compares two identical constant
expressions (60 * time.Second to 60 * time.Second) which will always pass
regardless of actual production code changes. Replace the hardcoded 60 *
time.Second on the left side of the Expect call in the It block with
authprovider.ExpirationDeadline so the test actually verifies the production
constant has the expected value, while keeping the right side as 60 *
time.Second for the comparison target.

In `@src/bosh-nats-sync/pkg/authprovider/auth_provider.go`:
- Around line 87-91: In the error handling block following the failed token
acquisition call to a.cfg.Token(a.ctx), change the return statement from
returning ("", nil) to returning ("", err) to properly propagate the
authentication error instead of suppressing it. This ensures that downstream
code receives the error and can handle the transient failure appropriately
through the intended retry and error handling flow.
- Around line 109-111: The http.Client being returned from this function lacks
an explicit timeout, which can cause token requests to hang indefinitely. Add a
Timeout field to the http.Client struct that is being returned to specify a
reasonable timeout duration for UAA token requests. This ensures that network
operations on the client will not block indefinitely and will allow sync
progress to continue even if token requests become unresponsive.
- Around line 99-107: The code currently silently ignores read errors and parse
failures when loading the configured CA certificate file in the block around
caCertPath and x509.NewCertPool(). Instead of only proceeding when err is nil,
you should fail fast by returning an error when either the file read fails or
when AppendCertsFromPEM fails to parse the certificates. This ensures that
configuration issues with the CA file are surfaced immediately rather than
silently degrading TLS trust behavior at runtime.

In `@src/bosh-nats-sync/pkg/config/config.go`:
- Around line 55-60: The NewLogger function dereferences the cfg parameter
without first checking if it is nil, which will cause a panic if a nil config is
passed. Add a nil check at the start of the NewLogger function before accessing
cfg.LogFile, and when cfg is nil, fall back to creating a logger that writes to
stdout instead of attempting to access cfg fields.
- Around line 41-53: The Load function unmarshals the YAML data into the Config
struct but does not validate the configuration before returning it, which can
lead to runtime failures when downstream code attempts to use fields like
Intervals.PollUserSync with time.NewTicker. After the yaml.Unmarshal call
succeeds, add validation logic to check that required fields are present and
that interval values (specifically Intervals.PollUserSync) are positive non-zero
durations. If validation fails, return an error with a descriptive message. This
ensures the config is semantically valid before being used.

In `@src/bosh-nats-sync/pkg/runner/runner_test.go`:
- Around line 193-204: The failCmdRunner in this test fails immediately on the
first call (startup reload), causing Run() to exit before reaching the periodic
ticker loop, so the periodic sync failure path is never tested. Modify the
failCmdRunner to succeed on the first invocation (to allow the startup reload to
complete) and fail on subsequent invocations (to test the periodic sync failure
path). Use a counter or state tracking within the failCmdRunner function to
differentiate between the first call and later calls, ensuring the test actually
validates periodic-sync failure as described.

In `@src/bosh-nats-sync/pkg/runner/runner.go`:
- Around line 57-59: The code creates a time.NewTicker with a duration from
r.config.Intervals.PollUserSync without validating that the value is greater
than zero, which can cause a panic if the config value is unset or invalid. Add
validation logic before creating the ticker to check if the interval duration is
greater than zero, and return an appropriate error or handle the invalid
configuration gracefully instead of allowing time.NewTicker to panic. This
validation should occur before the line where time.NewTicker is called.

In `@src/bosh-nats-sync/pkg/userssync/users_sync_test.go`:
- Around line 500-549: The test context is titled "when director becomes
available after retries" but the It block only verifies that Execute() succeeds
without validating that retries actually occurred. The /info handler is designed
to return StatusServiceUnavailable on the first request and StatusOK on
subsequent requests, but the test never asserts this retry behavior happened.
Add an assertion after the sync.Execute() call in the It block to check that
requestCount >= 2, ensuring the /info endpoint was called multiple times and the
retry mechanism in withDirectorConnection() was actually exercised.

In `@src/bosh-nats-sync/pkg/userssync/users_sync.go`:
- Around line 307-309: The deploymentName parameter in the getVMsByDeployment
method is directly embedded into the API path using fmt.Sprintf without URL
encoding, which will cause issues if the deployment name contains reserved URL
characters like forward slashes, percent signs, or spaces. Fix this by
URL-escaping the deploymentName before embedding it into the path string using
the appropriate URL encoding function from the net/url package, specifically
url.QueryEscape or url.PathEscape as appropriate for path parameters.
- Around line 107-127: The ConnectionWaitTimeout is used to calculate retry
attempts but doesn't enforce an overall elapsed-time budget across all probes.
Each HTTP call can block independently for up to 30 seconds, causing startup to
stall far longer than the configured timeout. In the withDirectorConnection()
method, create a deadline based on the configured timeout at the start and track
elapsed time throughout the retry loop. Before each call to
boshAPIResponseBody() and before sleeping, check if the elapsed time would
exceed the deadline and bail out if so. This ensures the total time spent across
all attempts, including individual HTTP timeouts, never exceeds the
ConnectionWaitTimeout. Apply the same deadline-checking pattern to all other
callers of boshAPIResponseBody() in getAuthHeader(), queryAllDeployments(), and
getVMsByDeployment() methods.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 74b1ea7c-2402-441d-a6e3-cda6eee9250b

📥 Commits

Reviewing files that changed from the base of the PR and between a13a1db and 980d15b.

⛔ Files ignored due to path filters (2)

• src/Gemfile.lock is excluded by !**/*.lock
• src/bosh-nats-sync/go.sum is excluded by !**/*.sum

📒 Files selected for processing (49)

• .github/workflows/go.yml
• .github/workflows/ruby.yml
• jobs/nats/spec
• jobs/nats/templates/bosh_nats_sync
• jobs/nats/templates/bpm.yml
• packages/nats/packaging
• packages/nats/spec
• src/Gemfile
• src/bosh-nats-sync/.gitignore
• src/bosh-nats-sync/.golangci.yml
• src/bosh-nats-sync/bin/bosh-nats-sync
• src/bosh-nats-sync/bosh-nats-sync.gemspec
• src/bosh-nats-sync/cmd/bosh-nats-sync/main.go
• src/bosh-nats-sync/go.mod
• src/bosh-nats-sync/lib/nats_sync.rb
• src/bosh-nats-sync/lib/nats_sync/auth_provider.rb
• src/bosh-nats-sync/lib/nats_sync/config.rb
• src/bosh-nats-sync/lib/nats_sync/nats_auth_config.rb
• src/bosh-nats-sync/lib/nats_sync/runner.rb
• src/bosh-nats-sync/lib/nats_sync/users_sync.rb
• src/bosh-nats-sync/lib/nats_sync/version.rb
• src/bosh-nats-sync/lib/nats_sync/yaml_helper.rb
• src/bosh-nats-sync/pkg/authprovider/auth_provider.go
• src/bosh-nats-sync/pkg/authprovider/auth_provider_suite_test.go
• src/bosh-nats-sync/pkg/authprovider/auth_provider_test.go
• src/bosh-nats-sync/pkg/config/config.go
• src/bosh-nats-sync/pkg/config/config_suite_test.go
• src/bosh-nats-sync/pkg/config/config_test.go
• src/bosh-nats-sync/pkg/natsauthconfig/nats_auth_config.go
• src/bosh-nats-sync/pkg/natsauthconfig/nats_auth_config_suite_test.go
• src/bosh-nats-sync/pkg/natsauthconfig/nats_auth_config_test.go
• src/bosh-nats-sync/pkg/runner/runner.go
• src/bosh-nats-sync/pkg/runner/runner_suite_test.go
• src/bosh-nats-sync/pkg/runner/runner_test.go
• src/bosh-nats-sync/pkg/userssync/users_sync.go
• src/bosh-nats-sync/pkg/userssync/users_sync_suite_test.go
• src/bosh-nats-sync/pkg/userssync/users_sync_test.go
• src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
• src/bosh-nats-sync/spec/nats_sync/config_spec.rb
• src/bosh-nats-sync/spec/nats_sync/nats_auth_config_spec.rb
• src/bosh-nats-sync/spec/nats_sync/runner_spec.rb
• src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb
• src/bosh-nats-sync/spec/nats_sync_spec.rb
• src/bosh-nats-sync/spec/spec_helper.rb
• src/bosh-nats-sync/spec/support/uaa_helpers.rb
• src/bosh-nats-sync/testdata/director-subject
• src/bosh-nats-sync/testdata/hm-subject
• src/bosh-nats-sync/testdata/sample_config.yml
• src/tasks/spec.rake

💤 Files with no reviewable changes (25)

• src/bosh-nats-sync/lib/nats_sync/version.rb
• jobs/nats/templates/bosh_nats_sync
• src/bosh-nats-sync/spec/support/uaa_helpers.rb
• src/bosh-nats-sync/lib/nats_sync/nats_auth_config.rb
• .github/workflows/ruby.yml
• src/bosh-nats-sync/.gitignore
• src/bosh-nats-sync/lib/nats_sync/yaml_helper.rb
• src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
• src/bosh-nats-sync/lib/nats_sync/runner.rb
• src/bosh-nats-sync/spec/nats_sync/runner_spec.rb
• src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb
• src/bosh-nats-sync/bin/bosh-nats-sync
• src/bosh-nats-sync/bosh-nats-sync.gemspec
• src/bosh-nats-sync/spec/nats_sync/nats_auth_config_spec.rb
• src/bosh-nats-sync/spec/nats_sync/config_spec.rb
• src/bosh-nats-sync/lib/nats_sync.rb
• src/Gemfile
• jobs/nats/spec
• src/bosh-nats-sync/lib/nats_sync/config.rb
• src/bosh-nats-sync/spec/nats_sync_spec.rb
• src/bosh-nats-sync/testdata/sample_config.yml
• src/bosh-nats-sync/spec/spec_helper.rb
• src/bosh-nats-sync/lib/nats_sync/users_sync.rb
• src/bosh-nats-sync/lib/nats_sync/auth_provider.rb
• jobs/nats/templates/bpm.yml

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 46 out of 51 changed files in this pull request and generated 6 comments.