From 5324908e6e1145bec5f2f0ab80b312a809ad1744 Mon Sep 17 00:00:00 2001 From: bjee19 <139261241+bjee19@users.noreply.github.com> Date: Fri, 13 Oct 2023 11:22:58 -0700 Subject: [PATCH] Fix NGF fails to recover if conf files are unexpectedly removed (#1132) Problem: When the http.conf file did not exist, any updates to NGF would cause NGF to error as it could not "replace" that file as it didn't exist. Solution: Added a check to see if the error returned from trying to remove a conf file was a IsNotExist, if so, continue and act as if the file has been deleted. --- internal/mode/static/nginx/file/manager.go | 12 +++++++++-- .../mode/static/nginx/file/manager_test.go | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/file/manager.go b/internal/mode/static/nginx/file/manager.go index 560ab878af..632a3af34f 100644 --- a/internal/mode/static/nginx/file/manager.go +++ b/internal/mode/static/nginx/file/manager.go @@ -88,10 +88,18 @@ func NewManagerImpl(logger logr.Logger, osFileManager OSFileManager) *ManagerImp func (m *ManagerImpl) ReplaceFiles(files []File) error { for _, path := range m.lastWrittenPaths { if err := m.osFileManager.Remove(path); err != nil { + if os.IsNotExist(err) { + m.logger.Info( + "File not found when attempting to delete", + "path", path, + "error", err, + ) + continue + } return fmt.Errorf("failed to delete file %q: %w", path, err) } - m.logger.Info("deleted file", "path", path) + m.logger.Info("Deleted file", "path", path) } // In some cases, NGINX reads files in runtime, like a JWK. If you remove such file, NGINX will fail @@ -106,7 +114,7 @@ func (m *ManagerImpl) ReplaceFiles(files []File) error { } m.lastWrittenPaths = append(m.lastWrittenPaths, file.Path) - m.logger.Info("wrote file", "path", file.Path) + m.logger.Info("Wrote file", "path", file.Path) } return nil diff --git a/internal/mode/static/nginx/file/manager_test.go b/internal/mode/static/nginx/file/manager_test.go index c6f52a526c..07f0478ae9 100644 --- a/internal/mode/static/nginx/file/manager_test.go +++ b/internal/mode/static/nginx/file/manager_test.go @@ -116,6 +116,26 @@ var _ = Describe("EventHandler", func() { }) }) + When("file does not exist", func() { + It("should not error", func() { + fakeOSMgr := &filefakes.FakeOSFileManager{} + mgr := file.NewManagerImpl(zap.New(), fakeOSMgr) + + files := []file.File{ + { + Type: file.TypeRegular, + Path: "regular-1.conf", + Content: []byte("regular-1"), + }, + } + + Expect(mgr.ReplaceFiles(files)).ToNot(HaveOccurred()) + + fakeOSMgr.RemoveReturns(os.ErrNotExist) + Expect(mgr.ReplaceFiles(files)).ToNot(HaveOccurred()) + }) + }) + When("file type is not supported", func() { It("should panic", func() { mgr := file.NewManagerImpl(zap.New(), nil)