From 0b2bbfb468d8a4b205b5617359a8bbcd734a593f Mon Sep 17 00:00:00 2001 From: Felix Moehler <101196374+fmoehler@users.noreply.github.com> Date: Thu, 28 May 2026 12:46:42 +0200 Subject: [PATCH 1/2] Update file_settings_source.go --- infrastructure/file_settings_source.go | 3 +++ infrastructure/file_settings_source_test.go | 29 ++++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/infrastructure/file_settings_source.go b/infrastructure/file_settings_source.go index 9a68c99a4..dd00a8825 100644 --- a/infrastructure/file_settings_source.go +++ b/infrastructure/file_settings_source.go @@ -35,6 +35,9 @@ func NewFileSettingsSource( } func (s *FileSettingsSource) PublicSSHKeyForUsername(string) (string, error) { + if !s.fs.FileExists(s.settingsFilePath) { + return "", bosherr.Error("File for settings source does not exist") + } return "", nil } diff --git a/infrastructure/file_settings_source_test.go b/infrastructure/file_settings_source_test.go index fe8bf27fa..1594997e9 100644 --- a/infrastructure/file_settings_source_test.go +++ b/infrastructure/file_settings_source_test.go @@ -26,10 +26,31 @@ var _ = Describe("FileSettingsSource", func() { }) Describe("PublicSSHKeyForUsername", func() { - It("returns an empty string", func() { - publicKey, err := source.PublicSSHKeyForUsername("fake-username") - Expect(err).ToNot(HaveOccurred()) - Expect(publicKey).To(Equal("")) + Context("when the settings file exists", func() { + BeforeEach(func() { + settingsFileName := "/fake-settings-file-path" + source = infrastructure.NewFileSettingsSource(settingsFileName, fs, logger) + err := fs.WriteFileString(settingsFileName, "{}") + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns an empty string", func() { + publicKey, err := source.PublicSSHKeyForUsername("fake-username") + Expect(err).ToNot(HaveOccurred()) + Expect(publicKey).To(Equal("")) + }) + }) + + Context("when the settings file does not exist", func() { + BeforeEach(func() { + source = infrastructure.NewFileSettingsSource("/missing-settings-file-path", fs, logger) + }) + + It("returns an error", func() { + _, err := source.PublicSSHKeyForUsername("fake-username") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("File for settings source does not exist")) + }) }) }) From f98de311793109944cf6b0827dcd3bcaaa7e8fb4 Mon Sep 17 00:00:00 2001 From: Felix Moehler Date: Fri, 29 May 2026 10:50:47 +0200 Subject: [PATCH 2/2] Add file path to error message and logging for SSH key failures - Include settings file path in PublicSSHKeyForUsername error message for easier debugging - Add warning logs in MultiSettingsSource when SSH key retrieval fails from a source - Update tests to verify error message includes file path and logging behavior --- infrastructure/file_settings_source.go | 2 +- infrastructure/file_settings_source_test.go | 3 ++- infrastructure/multi_settings_source.go | 1 + infrastructure/multi_settings_source_test.go | 10 ++++++++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/infrastructure/file_settings_source.go b/infrastructure/file_settings_source.go index dd00a8825..e38b0bbc0 100644 --- a/infrastructure/file_settings_source.go +++ b/infrastructure/file_settings_source.go @@ -36,7 +36,7 @@ func NewFileSettingsSource( func (s *FileSettingsSource) PublicSSHKeyForUsername(string) (string, error) { if !s.fs.FileExists(s.settingsFilePath) { - return "", bosherr.Error("File for settings source does not exist") + return "", bosherr.Errorf("File for settings source does not exist: '%s'", s.settingsFilePath) } return "", nil } diff --git a/infrastructure/file_settings_source_test.go b/infrastructure/file_settings_source_test.go index 1594997e9..4db223be1 100644 --- a/infrastructure/file_settings_source_test.go +++ b/infrastructure/file_settings_source_test.go @@ -46,10 +46,11 @@ var _ = Describe("FileSettingsSource", func() { source = infrastructure.NewFileSettingsSource("/missing-settings-file-path", fs, logger) }) - It("returns an error", func() { + It("returns an error with the file path", func() { _, err := source.PublicSSHKeyForUsername("fake-username") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("File for settings source does not exist")) + Expect(err.Error()).To(ContainSubstring("/missing-settings-file-path")) }) }) }) diff --git a/infrastructure/multi_settings_source.go b/infrastructure/multi_settings_source.go index 116dd0060..c54703a81 100644 --- a/infrastructure/multi_settings_source.go +++ b/infrastructure/multi_settings_source.go @@ -42,6 +42,7 @@ func (s *MultiSettingsSource) PublicSSHKeyForUsername(username string) (string, s.selectedSSHKeySource = source return publicSSHKey, nil } + s.logger.Warn("multi-settings-source", "Failed to get public SSH key from source: %v", err) } return "", bosherr.WrapErrorf(err, "Getting public SSH key for '%s'", username) diff --git a/infrastructure/multi_settings_source_test.go b/infrastructure/multi_settings_source_test.go index f090ec7fc..ec4777631 100644 --- a/infrastructure/multi_settings_source_test.go +++ b/infrastructure/multi_settings_source_test.go @@ -85,6 +85,11 @@ var _ = Describe("MultiSettingsSource", func() { Expect(err).ToNot(HaveOccurred()) Expect(publicKey).To(Equal("fake-public-key-2")) }) + + It("logs a warning for the first source", func() { + _, _ = source.PublicSSHKeyForUsername("fake-username") //nolint:errcheck + Expect(logBuffer.String()).To(ContainSubstring("fake-public-key-err-1")) + }) }) Context("when both sources fail to get ssh key", func() { @@ -93,6 +98,11 @@ var _ = Describe("MultiSettingsSource", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("fake-public-key-err-2")) }) + + It("logs a warning for the first source", func() { + _, _ = source.PublicSSHKeyForUsername("fake-username") //nolint:errcheck + Expect(logBuffer.String()).To(ContainSubstring("fake-public-key-err-1")) + }) }) })