From 2cd35f0b22666729cbef85fcfbb079312ebc1664 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:11:19 -0300 Subject: [PATCH] fix(gitops): resolve shared git credentials in git operations everywhere BE-13282 (#3330) --- api/gitops/workflows/git_phases.go | 127 ------------------- api/gitops/workflows/git_phases_test.go | 162 ------------------------ 2 files changed, 289 deletions(-) delete mode 100644 api/gitops/workflows/git_phases.go delete mode 100644 api/gitops/workflows/git_phases_test.go diff --git a/api/gitops/workflows/git_phases.go b/api/gitops/workflows/git_phases.go deleted file mode 100644 index 4d0e5c2769..0000000000 --- a/api/gitops/workflows/git_phases.go +++ /dev/null @@ -1,127 +0,0 @@ -package workflows - -import ( - "context" - "fmt" - "path" - "slices" - - portainer "github.com/portainer/portainer/api" - gittypes "github.com/portainer/portainer/api/git/types" -) - -// ListRefsFunc lists all git refs for a repository. -type ListRefsFunc func(ctx context.Context) ([]string, error) - -// ListFilesFunc lists files in a repository branch filtered by extension. -type ListFilesFunc func(ctx context.Context, exts []string, dirOnly bool) ([]string, error) - -// GitEntries represents a git entry which can be either a file or a directory. -type GitEntries struct { - Name string - IsFile bool -} - -// ComputeGitPhasesForConfig computes source and artifact phases from a RepoConfig and a GitService. -func ComputeGitPhasesForConfig(ctx context.Context, gitSvc portainer.GitService, cfg *gittypes.RepoConfig) (source, artifact WorkflowPhaseStatus) { - if gitSvc == nil || cfg == nil { - return WorkflowPhaseStatus{Status: StatusUnknown}, WorkflowPhaseStatus{Status: StatusUnknown} - } - - username, password := gitCredentials(cfg) - return ComputeGitPhases(ctx, cfg.ReferenceName, []GitEntries{{Name: cfg.ConfigFilePath, IsFile: true}}, - func(ctx context.Context) ([]string, error) { - return gitSvc.ListRefs(ctx, cfg.URL, username, password, false, cfg.TLSSkipVerify) - }, - func(ctx context.Context, exts []string, dirOnly bool) ([]string, error) { - return gitSvc.ListFiles(ctx, cfg.URL, cfg.ReferenceName, username, password, dirOnly, false, exts, cfg.TLSSkipVerify) - }, - ) -} - -func gitCredentials(cfg *gittypes.RepoConfig) (username, password string) { - if cfg.Authentication != nil { - return cfg.Authentication.Username, cfg.Authentication.Password - } - return "", "" -} - -// ComputeGitPhases checks source (ref reachability) and artifact (config file presence). -// If source fails, artifact is returned as unknown without making a network call. -func ComputeGitPhases(ctx context.Context, referenceName string, configFilePath []GitEntries, listRefs ListRefsFunc, listFiles ListFilesFunc) (source, artifact WorkflowPhaseStatus) { - source = computeSourcePhase(ctx, referenceName, listRefs) - if source.Status == StatusError { - return source, WorkflowPhaseStatus{Status: StatusUnknown} - } - return source, computeArtifactPhase(ctx, configFilePath, listFiles) -} - -func computeSourcePhase(ctx context.Context, referenceName string, listRefs ListRefsFunc) WorkflowPhaseStatus { - refs, err := listRefs(ctx) - if err != nil { - return WorkflowPhaseStatus{Status: StatusError, Error: err.Error()} - } - if referenceName == "" { - return WorkflowPhaseStatus{Status: StatusHealthy} - } - if !slices.Contains(refs, referenceName) { - return WorkflowPhaseStatus{Status: StatusError, Error: fmt.Sprintf("ref %q not found", referenceName)} - } - return WorkflowPhaseStatus{Status: StatusHealthy} -} - -func computeArtifactPhase(ctx context.Context, gitEntries []GitEntries, listFiles ListFilesFunc) WorkflowPhaseStatus { - if len(gitEntries) == 0 { - return WorkflowPhaseStatus{Status: StatusError, Error: "no config file path specified"} - } - - var ( - exts []string - fileEntries []string - dirEntries []string - ) - for _, gitEntry := range gitEntries { - if gitEntry.IsFile { - ext := path.Ext(gitEntry.Name) - if len(ext) > 0 { - ext = ext[1:] - exts = append(exts, ext) - } - - fileEntries = append(fileEntries, gitEntry.Name) - continue - } - - dirEntries = append(dirEntries, gitEntry.Name) - } - - // Check file entries - if len(fileEntries) > 0 { - files, err := listFiles(ctx, exts, false) - if err != nil { - return WorkflowPhaseStatus{Status: StatusError, Error: err.Error()} - } - - for _, fileEntry := range fileEntries { - if !slices.Contains(files, fileEntry) { - return WorkflowPhaseStatus{Status: StatusError, Error: fmt.Sprintf("file %q not found", fileEntry)} - } - } - } - - // Check directory entries - if len(dirEntries) > 0 { - dirs, err := listFiles(ctx, nil, true) - if err != nil { - return WorkflowPhaseStatus{Status: StatusError, Error: err.Error()} - } - - for _, dirEntry := range dirEntries { - if !slices.Contains(dirs, dirEntry) { - return WorkflowPhaseStatus{Status: StatusError, Error: fmt.Sprintf("directory %q not found", dirEntry)} - } - } - } - - return WorkflowPhaseStatus{Status: StatusHealthy} -} diff --git a/api/gitops/workflows/git_phases_test.go b/api/gitops/workflows/git_phases_test.go deleted file mode 100644 index 956625cecb..0000000000 --- a/api/gitops/workflows/git_phases_test.go +++ /dev/null @@ -1,162 +0,0 @@ -package workflows - -import ( - "context" - "errors" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestComputeGitPhases(t *testing.T) { - t.Parallel() - - okRefs := func(_ context.Context) ([]string, error) { - return []string{"refs/heads/main"}, nil - } - okFiles := func(_ context.Context, _ []string, _ bool) ([]string, error) { - return []string{"docker-compose.yml"}, nil - } - errRefs := func(_ context.Context) ([]string, error) { - return nil, errors.New("connection refused") - } - errFiles := func(_ context.Context, _ []string, _ bool) ([]string, error) { - return nil, errors.New("connection refused") - } - - cases := []struct { - name string - referenceName string - configFilePath []GitEntries - listRefs ListRefsFunc - listFiles ListFilesFunc - expectedSource Status - expectedArtifact Status - }{ - { - name: "listRefs errors: source error, artifact unknown", - referenceName: "refs/heads/main", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: errRefs, - listFiles: okFiles, - expectedSource: StatusError, - expectedArtifact: StatusUnknown, - }, - { - name: "ref not in list: source error, artifact unknown", - referenceName: "refs/heads/missing", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: func(_ context.Context) ([]string, error) { - return []string{"refs/heads/main"}, nil - }, - listFiles: okFiles, - expectedSource: StatusError, - expectedArtifact: StatusUnknown, - }, - { - name: "empty configFilePath: artifact error", - referenceName: "refs/heads/main", - configFilePath: []GitEntries{}, - listRefs: okRefs, - listFiles: okFiles, - expectedSource: StatusHealthy, - expectedArtifact: StatusError, - }, - { - name: "listFiles errors: artifact error", - referenceName: "refs/heads/main", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: okRefs, - listFiles: errFiles, - expectedSource: StatusHealthy, - expectedArtifact: StatusError, - }, - { - name: "file not in list: artifact error", - referenceName: "refs/heads/main", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: okRefs, - listFiles: func(_ context.Context, _ []string, _ bool) ([]string, error) { - return []string{"other.yml"}, nil - }, - expectedSource: StatusHealthy, - expectedArtifact: StatusError, - }, - { - name: "both healthy", - referenceName: "refs/heads/main", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: okRefs, - listFiles: okFiles, - expectedSource: StatusHealthy, - expectedArtifact: StatusHealthy, - }, - { - name: "empty referenceName: source healthy (default HEAD)", - referenceName: "", - configFilePath: []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, - listRefs: okRefs, - listFiles: okFiles, - expectedSource: StatusHealthy, - expectedArtifact: StatusHealthy, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - source, artifact := ComputeGitPhases(t.Context(), tc.referenceName, tc.configFilePath, tc.listRefs, tc.listFiles) - assert.Equal(t, tc.expectedSource, source.Status) - assert.Equal(t, tc.expectedArtifact, artifact.Status) - }) - } -} - -func TestComputeArtifactPhase_ExtensionFilter(t *testing.T) { - t.Parallel() - - cases := []struct { - configPath string - wantExts []string - }{ - {"docker-compose.yml", []string{"yml"}}, - {"stack.yaml", []string{"yaml"}}, - {"subdir/compose.yml", []string{"yml"}}, - {"Makefile", nil}, - {"archive.tar.gz", []string{"gz"}}, - } - - for _, tc := range cases { - t.Run(tc.configPath, func(t *testing.T) { - t.Parallel() - var capturedExts []string - ComputeGitPhases( - t.Context(), - "", - []GitEntries{{Name: tc.configPath, IsFile: true}}, - func(_ context.Context) ([]string, error) { return nil, nil }, - func(_ context.Context, exts []string, dirOnly bool) ([]string, error) { - capturedExts = exts - return []string{tc.configPath}, nil - }, - ) - assert.Equal(t, tc.wantExts, capturedExts) - }) - } -} - -func TestComputeGitPhases_ArtifactNotCalledOnSourceError(t *testing.T) { - t.Parallel() - - listFilesCalled := false - listRefs := func(_ context.Context) ([]string, error) { - return nil, errors.New("repo unreachable") - } - listFiles := func(_ context.Context, _ []string, _ bool) ([]string, error) { - listFilesCalled = true - return nil, nil - } - - ComputeGitPhases(t.Context(), "refs/heads/main", []GitEntries{{Name: "docker-compose.yml", IsFile: true}}, listRefs, listFiles) - - assert.False(t, listFilesCalled, "listFiles must not be called when source fails") -}