From e0934bb7fa91e40d516c22d4e3b3cbb1300b6d67 Mon Sep 17 00:00:00 2001 From: Oscar Zhou <100548325+oscarzhou-portainer@users.noreply.github.com> Date: Tue, 11 Jun 2024 22:11:08 +1200 Subject: [PATCH] fix(customtemplate): duplicated error handling [EE-7197] (#11912) --- .../customtemplate_git_fetch.go | 3 +- .../customtemplate_git_fetch_test.go | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/api/http/handler/customtemplates/customtemplate_git_fetch.go b/api/http/handler/customtemplates/customtemplate_git_fetch.go index 64132e56e..59da73b1f 100644 --- a/api/http/handler/customtemplates/customtemplate_git_fetch.go +++ b/api/http/handler/customtemplates/customtemplate_git_fetch.go @@ -70,8 +70,7 @@ func (handler *Handler) customTemplateGitFetch(w http.ResponseWriter, r *http.Re if err != nil { log.Warn().Err(err).Msg("failed to download git repository") - if err != nil { - rbErr := rollbackCustomTemplate(backupPath, customTemplate.ProjectPath) + if rbErr := rollbackCustomTemplate(backupPath, customTemplate.ProjectPath); rbErr != nil { return httperror.InternalServerError("Failed to rollback the custom template folder", rbErr) } diff --git a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go index 736897de6..83cad9b2b 100644 --- a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go +++ b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go @@ -2,6 +2,7 @@ package customtemplates import ( "bytes" + "errors" "io" "io/fs" "net/http" @@ -19,6 +20,7 @@ import ( "github.com/portainer/portainer/api/internal/authorization" "github.com/portainer/portainer/api/internal/testhelpers" "github.com/portainer/portainer/api/jwt" + httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/segmentio/encoding/json" "github.com/stretchr/testify/assert" @@ -49,6 +51,19 @@ func (f *TestFileService) GetFileContent(projectPath, configFilePath string) ([] return os.ReadFile(filepath.Join(projectPath, configFilePath)) } +type InvalidTestGitService struct { + portainer.GitService + targetFilePath string +} + +func (g *InvalidTestGitService) CloneRepository(dest, repoUrl, refName, username, password string, tlsSkipVerify bool) error { + return errors.New("simulate network error") +} + +func (g *InvalidTestGitService) LatestCommitID(repositoryURL, referenceName, username, password string, tlsSkipVerify bool) (string, error) { + return "", nil +} + func createTestFile(targetPath string) error { f, err := os.Create(targetPath) if err != nil { @@ -174,4 +189,28 @@ func Test_customTemplateGitFetch(t *testing.T) { singleAPIRequest(h, jwt2, is, "gfedcba") }) + + t.Run("restore git repository if it is failed to download the new git repository", func(t *testing.T) { + invalidGitService := &InvalidTestGitService{ + targetFilePath: filepath.Join(template1.ProjectPath, template1.GitConfig.ConfigFilePath), + } + h := NewHandler(requestBouncer, store, fileService, invalidGitService) + + req := httptest.NewRequest(http.MethodPut, "/custom_templates/1/git_fetch", bytes.NewBuffer([]byte("{}"))) + testhelpers.AddTestSecurityCookie(req, jwt1) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + is.Equal(http.StatusInternalServerError, rr.Code) + + var errResp httperror.HandlerError + err = json.NewDecoder(rr.Body).Decode(&errResp) + assert.NoError(t, err, "failed to parse error body") + + assert.FileExists(t, gitService.targetFilePath, "previous git repository is not restored") + fileContent, err := os.ReadFile(gitService.targetFilePath) + assert.NoError(t, err, "failed to read target file") + assert.Equal(t, "gfedcba", string(fileContent)) + }) }