refactor(stacks): extract and align git auth resolution for stack redeploy (#3073)

api/http/handler/stacks/stack_update_git_redeploy.go

@@ -26,12 +26,14 @@ import (
 )
 
 type stackGitRedeployPayload struct {
-	RepositoryReferenceName  string
+	RepositoryReferenceName string
+	// When true and RepositoryPassword is non-empty, stored credentials are replaced.
 	RepositoryAuthentication bool
 	RepositoryUsername       string
-	RepositoryPassword       string
-	Env                      []portainer.Pair
-	Prune                    *bool
+	// Non-empty value (with RepositoryAuthentication=true) replaces stored credentials; leave blank to keep them.
+	RepositoryPassword string
+	Env                []portainer.Pair
+	Prune              *bool
 	// RepullImageAndRedeploy indicates whether to force repulling images and redeploying the stack
 	RepullImageAndRedeploy bool
 
@@ -175,25 +177,14 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request)
 		stack.Name = payload.StackName
 	}
 
-	repositoryUsername := ""
-	repositoryPassword := ""
-	if payload.RepositoryAuthentication {
-		repositoryPassword = payload.RepositoryPassword
-
-		// When the existing stack is using the custom username/password and the password is not updated,
-		// the stack should keep using the saved username/password
-		if repositoryPassword == "" && gitConfig.Authentication != nil {
-			repositoryPassword = gitConfig.Authentication.Password
-		}
-		repositoryUsername = payload.RepositoryUsername
-	}
+	auth := resolveGitAuthFromRedeployPayload(gitConfig, payload)
 
 	cloneOptions := git.CloneOptions{
 		ProjectPath:   stack.ProjectPath,
 		URL:           gitConfig.URL,
 		ReferenceName: gitConfig.ReferenceName,
-		Username:      repositoryUsername,
-		Password:      repositoryPassword,
+		Username:      auth.Username,
+		Password:      auth.Password,
 		TLSSkipVerify: gitConfig.TLSSkipVerify,
 	}
 
@@ -204,7 +195,7 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request)
 
 	defer clean()
 
-	newHash, err := handler.GitService.LatestCommitID(context.TODO(), gitConfig.URL, gitConfig.ReferenceName, repositoryUsername, repositoryPassword, gitConfig.TLSSkipVerify)
+	newHash, err := handler.GitService.LatestCommitID(context.TODO(), gitConfig.URL, gitConfig.ReferenceName, auth.Username, auth.Password, gitConfig.TLSSkipVerify)
 	if err != nil {
 		return httperror.InternalServerError("Unable get latest commit id", errors.WithMessagef(err, "failed to fetch latest commit id of the stack %v", stack.ID))
 	}
@@ -282,6 +273,20 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request)
 	return response.JSON(w, stack)
 }
 
+func resolveGitAuthFromRedeployPayload(gitConfig *gittypes.RepoConfig, payload stackGitRedeployPayload) gittypes.GitAuthentication {
+	auth := gittypes.GitAuthentication{}
+	if gitConfig.Authentication != nil {
+		auth = *gitConfig.Authentication
+	}
+
+	// only overload source auth if provided
+	if payload.RepositoryAuthentication && payload.RepositoryPassword != "" {
+		auth.Username = payload.RepositoryUsername
+		auth.Password = payload.RepositoryPassword
+	}
+	return auth
+}
+
 func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pullImage bool, endpoint *portainer.Endpoint, gate *deployGate, postDeploy postDeployFunc) *httperror.HandlerError {
 	var deploymentConfiger deployments.StackDeploymentConfiger
 

api/http/handler/stacks/stack_update_git_redeploy_test.go

@@ -0,0 +1,82 @@
+package stacks
+
+import (
+	"testing"
+
+	gittypes "github.com/portainer/portainer/api/git/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestResolveGitAuthFromRedeployPayload(t *testing.T) {
+	t.Parallel()
+
+	existing := &gittypes.GitAuthentication{
+		Username: "existing-user",
+		Password: "existing-pass",
+	}
+
+	tests := []struct {
+		name    string
+		auth    *gittypes.GitAuthentication
+		payload stackGitRedeployPayload
+		want    gittypes.GitAuthentication
+	}{
+		{
+			name:    "no existing auth, flag off, no creds",
+			auth:    nil,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: false},
+			want:    gittypes.GitAuthentication{},
+		},
+		{
+			name:    "no existing auth, flag off, creds provided",
+			auth:    nil,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: false, RepositoryPassword: "pass"},
+			want:    gittypes.GitAuthentication{},
+		},
+		{
+			name:    "no existing auth, flag on, empty password",
+			auth:    nil,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: true, RepositoryUsername: "user"},
+			want:    gittypes.GitAuthentication{},
+		},
+		{
+			name:    "no existing auth, flag on, password set",
+			auth:    nil,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: true, RepositoryUsername: "user", RepositoryPassword: "pass"},
+			want:    gittypes.GitAuthentication{Username: "user", Password: "pass"},
+		},
+		{
+			name:    "no existing auth, flag on, password set but no username",
+			auth:    nil,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: true, RepositoryPassword: "pass"},
+			want:    gittypes.GitAuthentication{Username: "", Password: "pass"},
+		},
+		{
+			name:    "existing auth, flag off",
+			auth:    existing,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: false},
+			want:    *existing,
+		},
+		{
+			name:    "existing auth, flag on, empty password",
+			auth:    existing,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: true, RepositoryUsername: "new-user"},
+			want:    *existing,
+		},
+		{
+			name:    "existing auth, flag on, password set",
+			auth:    existing,
+			payload: stackGitRedeployPayload{RepositoryAuthentication: true, RepositoryUsername: "new-user", RepositoryPassword: "new-pass"},
+			want:    gittypes.GitAuthentication{Username: "new-user", Password: "new-pass"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			cfg := &gittypes.RepoConfig{Authentication: tc.auth}
+			got := resolveGitAuthFromRedeployPayload(cfg, tc.payload)
+			require.Equal(t, tc.want, got)
+		})
+	}
+}