Skip to content

Commit e059a19

Browse files
colinsaramprice
authored andcommitted
Fix health_monitor NATS Authorization Violation on fresh deployments
On a fresh bosh create-env, pre-start writes a fooBar token placeholder to auth.json. The old startup code only sent a SIGHUP to reload this placeholder, switching NATS to token-based auth and causing every health_monitor connection attempt to fail with "Authorization Violation" until the first periodic sync ran. Add UsersSync.Bootstrap() which reads the director-subject and hm-subject files written by pre-start and immediately writes a proper user-based NATS config, then sends SIGHUP — all without querying the director. Runner.Run() now calls bootstrapNATSConfig() at startup instead of a bare ReloadNATSServerConfig, ensuring health_monitor can authenticate against NATS before the director finishes initializing. Bootstrap failures are non-fatal and logged; the periodic sync loop continues so the full user list is populated once the director is up.
1 parent c40f364 commit e059a19

4 files changed

Lines changed: 225 additions & 21 deletions

File tree

src/bosh-nats-sync/pkg/runner/runner.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,11 @@ func (r *Runner) Run() error {
4646
cmdRunner = userssync.DefaultCommandRunner
4747
}
4848

49-
if err := userssync.ReloadNATSServerConfig(
50-
r.config.NATS.NATSServerExecutable,
51-
r.config.NATS.NATSServerPIDFile,
52-
cmdRunner,
53-
); err != nil {
54-
return fmt.Errorf("failed to reload NATS server config on startup: %w", err)
55-
}
49+
// Bootstrap: write the initial NATS config from on-disk subject files
50+
// immediately, before the director is queried. This replaces the
51+
// placeholder token written by pre-start so that health_monitor and the
52+
// director can authenticate against NATS during director startup.
53+
r.bootstrapNATSConfig(cmdRunner)
5654

5755
interval := time.Duration(r.config.Intervals.PollUserSync) * time.Second
5856
if interval <= 0 {
@@ -80,6 +78,20 @@ func (r *Runner) Stop() {
8078
<-r.stopped
8179
}
8280

81+
func (r *Runner) bootstrapNATSConfig(cmdRunner userssync.CommandRunner) {
82+
sync := &userssync.UsersSync{
83+
NATSConfigFilePath: r.config.NATS.ConfigFilePath,
84+
BoshConfig: r.config.Director,
85+
NATSServerExecutable: r.config.NATS.NATSServerExecutable,
86+
NATSServerPIDFile: r.config.NATS.NATSServerPIDFile,
87+
Logger: r.logger,
88+
CommandRunner: cmdRunner,
89+
}
90+
if err := sync.Bootstrap(); err != nil {
91+
r.logger.Error("Bootstrap failed, health_monitor may not connect to NATS until next sync", "error", err)
92+
}
93+
}
94+
8395
func (r *Runner) syncNATSUsers(cmdRunner userssync.CommandRunner) {
8496
sync := &userssync.UsersSync{
8597
NATSConfigFilePath: r.config.NATS.ConfigFilePath,

src/bosh-nats-sync/pkg/runner/runner_test.go

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,28 +138,91 @@ var _ = Describe("Runner", func() {
138138
})
139139
})
140140

141-
Describe("exception handling", func() {
142-
Context("when startup reload fails", func() {
143-
It("returns an error and exits Run() without starting the sync loop", func() {
144-
startupFailRunner := func(executable string, args ...string) ([]byte, error) {
145-
return nil, fmt.Errorf("cannot execute: reload failed on startup")
141+
Describe("bootstrap on startup", func() {
142+
It("writes the initial NATS config from subject files before the first sync tick", func() {
143+
// Override natsConfigFile with the fooBar placeholder that pre-start creates.
144+
os.WriteFile(natsConfigFile.Name(), []byte(`{"authorization":{"token":"f0oBar"}}`), 0644)
145+
146+
r := runner.NewWithCommandRunner(cfg, logger, cmdRunner)
147+
148+
go r.Run()
149+
// Sleep well under PollUserSync (1s) — bootstrap must fire synchronously.
150+
time.Sleep(200 * time.Millisecond)
151+
r.Stop()
152+
153+
data, err := os.ReadFile(natsConfigFile.Name())
154+
Expect(err).NotTo(HaveOccurred())
155+
var result map[string]interface{}
156+
Expect(json.Unmarshal(data, &result)).To(Succeed())
157+
158+
auth := result["authorization"].(map[string]interface{})
159+
users := auth["users"].([]interface{})
160+
Expect(len(users)).To(BeNumerically(">=", 2), "expected director and HM users in bootstrap config")
161+
162+
subjects := make([]string, 0, len(users))
163+
for _, u := range users {
164+
subjects = append(subjects, u.(map[string]interface{})["user"].(string))
165+
}
166+
Expect(subjects).To(ContainElement(ContainSubstring("director.bosh-internal")))
167+
Expect(subjects).To(ContainElement(ContainSubstring("hm.bosh-internal")))
168+
169+
// cmdRunner must have been called at least once for the bootstrap SIGHUP.
170+
Expect(len(commandRunnerCalls)).To(BeNumerically(">=", 1))
171+
})
172+
173+
It("does not contact the director during bootstrap", func() {
174+
var directorCalled bool
175+
isolatedMux := http.NewServeMux()
176+
isolatedMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
177+
directorCalled = true
178+
w.WriteHeader(http.StatusOK)
179+
})
180+
isolatedServer := httptest.NewServer(isolatedMux)
181+
defer isolatedServer.Close()
182+
183+
isolatedCfg := *cfg
184+
isolatedCfg.Director.URL = isolatedServer.URL
185+
186+
r := runner.NewWithCommandRunner(&isolatedCfg, logger, cmdRunner)
187+
go r.Run()
188+
time.Sleep(200 * time.Millisecond)
189+
r.Stop()
190+
191+
Expect(directorCalled).To(BeFalse(), "bootstrap must not query the director")
192+
})
193+
194+
Context("when bootstrap fails (e.g. NATS SIGHUP error)", func() {
195+
It("logs the error but continues running the sync loop", func() {
196+
var callCount int32
197+
nonFatalRunner := func(executable string, args ...string) ([]byte, error) {
198+
n := atomic.AddInt32(&callCount, 1)
199+
if n == 1 {
200+
// bootstrap reload fails
201+
return nil, fmt.Errorf("cannot execute: bootstrap reload failed")
202+
}
203+
return []byte("ok"), nil
146204
}
147205

148-
r := runner.NewWithCommandRunner(cfg, logger, startupFailRunner)
206+
r := runner.NewWithCommandRunner(cfg, logger, nonFatalRunner)
149207

150208
done := make(chan struct{})
151-
var runErr error
152209
go func() {
153-
runErr = r.Run()
210+
r.Run()
154211
close(done)
155212
}()
156213

214+
// Run() must NOT exit immediately after a bootstrap failure.
215+
Consistently(done, 500*time.Millisecond).ShouldNot(BeClosed())
216+
r.Stop()
157217
Eventually(done, 2*time.Second).Should(BeClosed())
158-
Expect(runErr).To(HaveOccurred())
159-
Expect(runErr.Error()).To(ContainSubstring("reload failed on startup"))
218+
219+
Expect(logBuf.String()).To(ContainSubstring("Bootstrap failed"))
220+
Expect(logBuf.String()).To(ContainSubstring("bootstrap reload failed"))
160221
})
161222
})
223+
})
162224

225+
Describe("exception handling", func() {
163226
Context("when an error occurs during periodic sync", func() {
164227
It("stops the runner and logs the error", func() {
165228
var syncCount int32
@@ -194,10 +257,7 @@ var _ = Describe("Runner", func() {
194257

195258
var reloadCount int32
196259
failCmdRunner := func(executable string, args ...string) ([]byte, error) {
197-
n := atomic.AddInt32(&reloadCount, 1)
198-
if n == 1 {
199-
return []byte("ok"), nil // startup reload succeeds
200-
}
260+
atomic.AddInt32(&reloadCount, 1)
201261
return nil, fmt.Errorf("cannot execute: reload failed")
202262
}
203263

src/bosh-nats-sync/pkg/userssync/users_sync.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,31 @@ func ReloadNATSServerConfig(executable, pidFile string, runner CommandRunner) er
104104
return err
105105
}
106106

107+
// Bootstrap writes the initial NATS authorization config with only the
108+
// director and health-monitor subjects read from their subject files on disk.
109+
// It is called once at startup before the periodic sync loop so that
110+
// health_monitor and the director can authenticate against NATS immediately,
111+
// without waiting for bosh_nats_sync to successfully query the director API.
112+
func (u *UsersSync) Bootstrap() error {
113+
directorSubject := readSubjectFile(u.BoshConfig.DirectorSubjectFile)
114+
hmSubject := readSubjectFile(u.BoshConfig.HMSubjectFile)
115+
116+
if directorSubject == nil && hmSubject == nil {
117+
u.Logger.Info("Bootstrap: no subject files found, skipping initial NATS config write")
118+
return nil
119+
}
120+
121+
u.Logger.Info("Bootstrap: writing initial NATS config with director/HM subjects")
122+
if err := u.writeNATSConfigFile(nil, directorSubject, hmSubject); err != nil {
123+
return fmt.Errorf("bootstrap: failed to write NATS config: %w", err)
124+
}
125+
if err := ReloadNATSServerConfig(u.NATSServerExecutable, u.NATSServerPIDFile, u.getCommandRunner()); err != nil {
126+
return fmt.Errorf("bootstrap: failed to reload NATS server config: %w", err)
127+
}
128+
u.Logger.Info("Bootstrap: NATS config written and server reloaded")
129+
return nil
130+
}
131+
107132
func (u *UsersSync) withDirectorConnection(fn func() error) error {
108133
timeout := u.BoshConfig.ConnectionWaitTimeout
109134
if timeout <= 0 {

src/bosh-nats-sync/pkg/userssync/users_sync_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,113 @@ var _ = Describe("UsersSync", func() {
775775
Expect(err).To(HaveOccurred())
776776
})
777777
})
778+
779+
// Mirrors the BOSH startup race: pre-start writes a fooBar token placeholder
780+
// to auth.json and the hm-subject / director-subject files, then bosh-nats-sync
781+
// starts. Bootstrap() must overwrite the placeholder with real user credentials
782+
// immediately, without querying the director, so that health_monitor can
783+
// authenticate against NATS before the director finishes starting up.
784+
Describe("Bootstrap", func() {
785+
var (
786+
bootstrapSync *userssync.UsersSync
787+
bootstrapCmdCalls []string
788+
bootstrapCmdErr error
789+
)
790+
791+
BeforeEach(func() {
792+
bootstrapCmdCalls = nil
793+
bootstrapCmdErr = nil
794+
795+
// Simulate the fooBar token placeholder written by pre-start.
796+
os.WriteFile(natsConfigFilePath, []byte(`{"authorization":{"token":"f0oBar"}}`), 0644)
797+
798+
// boshConfig is only populated in inner BeforeEach blocks elsewhere,
799+
// so we build it explicitly here with the subject files from the
800+
// outer BeforeEach.
801+
bootstrapSync = &userssync.UsersSync{
802+
NATSConfigFilePath: natsConfigFilePath,
803+
BoshConfig: config.DirectorConfig{
804+
URL: "http://127.0.0.1:1", // unreachable; Bootstrap must not contact it
805+
DirectorSubjectFile: directorSubjectFile,
806+
HMSubjectFile: hmSubjectFile,
807+
},
808+
NATSServerExecutable: natsExecutable,
809+
NATSServerPIDFile: natsServerPIDFile,
810+
Logger: logger,
811+
CommandRunner: func(executable string, args ...string) ([]byte, error) {
812+
bootstrapCmdCalls = append(bootstrapCmdCalls, fmt.Sprintf("%s %s", executable, strings.Join(args, " ")))
813+
return []byte("ok"), bootstrapCmdErr
814+
},
815+
}
816+
})
817+
818+
It("writes the director and HM users to the NATS config without querying the director", func() {
819+
err := bootstrapSync.Bootstrap()
820+
Expect(err).NotTo(HaveOccurred())
821+
822+
data, readErr := os.ReadFile(natsConfigFilePath)
823+
Expect(readErr).NotTo(HaveOccurred())
824+
825+
var cfg natsauthconfig.AuthorizationConfig
826+
Expect(json.Unmarshal(data, &cfg)).To(Succeed())
827+
828+
subjects := make([]string, 0, len(cfg.Authorization.Users))
829+
for _, u := range cfg.Authorization.Users {
830+
subjects = append(subjects, u.User)
831+
}
832+
Expect(subjects).To(ContainElement(ContainSubstring("director.bosh-internal")))
833+
Expect(subjects).To(ContainElement(ContainSubstring("hm.bosh-internal")))
834+
})
835+
836+
It("overwrites the fooBar token placeholder left by pre-start", func() {
837+
Expect(bootstrapSync.Bootstrap()).To(Succeed())
838+
839+
data, _ := os.ReadFile(natsConfigFilePath)
840+
Expect(string(data)).NotTo(ContainSubstring("f0oBar"))
841+
Expect(string(data)).To(ContainSubstring("users"))
842+
})
843+
844+
It("sends a SIGHUP to reload the NATS server after writing the config", func() {
845+
Expect(bootstrapSync.Bootstrap()).To(Succeed())
846+
847+
Expect(bootstrapCmdCalls).To(HaveLen(1))
848+
Expect(bootstrapCmdCalls[0]).To(ContainSubstring("--signal"))
849+
Expect(bootstrapCmdCalls[0]).To(ContainSubstring("reload="))
850+
})
851+
852+
It("skips the write when neither subject file exists", func() {
853+
bootstrapSync.BoshConfig.DirectorSubjectFile = "/nonexistent"
854+
bootstrapSync.BoshConfig.HMSubjectFile = "/nonexistent"
855+
856+
err := bootstrapSync.Bootstrap()
857+
Expect(err).NotTo(HaveOccurred())
858+
859+
// Config must remain unchanged (no SIGHUP either).
860+
data, _ := os.ReadFile(natsConfigFilePath)
861+
Expect(string(data)).To(ContainSubstring("f0oBar"))
862+
Expect(bootstrapCmdCalls).To(BeEmpty())
863+
})
864+
865+
It("returns an error when the NATS reload fails", func() {
866+
bootstrapCmdErr = fmt.Errorf("reload failed")
867+
868+
err := bootstrapSync.Bootstrap()
869+
Expect(err).To(HaveOccurred())
870+
Expect(err.Error()).To(ContainSubstring("reload failed"))
871+
})
872+
873+
It("includes only the HM user when the director subject file is missing", func() {
874+
bootstrapSync.BoshConfig.DirectorSubjectFile = "/nonexistent"
875+
876+
Expect(bootstrapSync.Bootstrap()).To(Succeed())
877+
878+
data, _ := os.ReadFile(natsConfigFilePath)
879+
var cfg natsauthconfig.AuthorizationConfig
880+
Expect(json.Unmarshal(data, &cfg)).To(Succeed())
881+
Expect(cfg.Authorization.Users).To(HaveLen(1))
882+
Expect(cfg.Authorization.Users[0].User).To(ContainSubstring("hm.bosh-internal"))
883+
})
884+
})
778885
})
779886

780887
// Mirrors Ruby spec: spec/nats_sync/users_sync_spec.rb

0 commit comments

Comments
 (0)