Skip to content

fix(server): change git clone to clone with context for potential bug and add global ctx to taskManager #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ssr-zna wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(server): change git clone to clone with context for potential bug and add global ctx to taskManager #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions server/path_publish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"context"
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -106,15 +107,15 @@ func (b *Backend) pathPublish(ctx context.Context, req *logical.Request, fields
return nil, fmt.Errorf("error getting publisher repository: %w", err)
}

taskUUID, err := b.TasksManager.RunTask(context.Background(), req.Storage, func(ctx context.Context, storage logical.Storage) error {
taskUUID, err := b.TasksManager.RunTask(ctx, req.Storage, func(ctx context.Context, storage logical.Storage) error {
logboek.Context(ctx).Default().LogF("Started task\n")
b.Logger().Debug("Started task")

logboek.Context(ctx).Default().LogF("Cloning git repo\n")
b.Logger().Debug("Cloning git repo")

gitBranch := cfg.GitTrdlChannelsBranch
gitRepo, err := cloneGitRepositoryBranch(cfg.GitRepoUrl, gitBranch, gitUsername, gitPassword)
gitRepo, err := cloneGitRepositoryBranch(ctx, cfg.GitRepoUrl, gitBranch, gitUsername, gitPassword)
if err != nil {
return fmt.Errorf("unable to clone git repository: %w", err)
}
Expand Down Expand Up @@ -203,7 +204,7 @@ func (b *Backend) pathPublish(ctx context.Context, req *logical.Request, fields
return nil
})
if err != nil {
if err == tasks_manager.ErrBusy {
if errors.Is(err, tasks_manager.ErrBusy) {
return logical.ErrorResponse("busy"), nil
}

Expand Down Expand Up @@ -306,7 +307,7 @@ func NewErrIncorrectChannelName(chnl string) error {
return fmt.Errorf(`got incorrect channel name %q: expected "dev", "alpha", "beta", "ea", "stable" or "rock-solid"`, chnl)
}

func cloneGitRepositoryBranch(url, gitBranch, username, password string) (*git.Repository, error) {
func cloneGitRepositoryBranch(ctx context.Context, url, gitBranch, username, password string) (*git.Repository, error) {
cloneGitOptions := trdlGit.CloneOptions{
BranchName: gitBranch,
RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
Expand All @@ -319,7 +320,7 @@ func cloneGitRepositoryBranch(url, gitBranch, username, password string) (*git.R
}
}

gitRepo, err := trdlGit.CloneInMemory(url, cloneGitOptions)
gitRepo, err := trdlGit.CloneInMemory(ctx, url, cloneGitOptions)
if err != nil {
return nil, err
}
Expand Down
11 changes: 6 additions & 5 deletions server/path_release.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"archive/tar"
"context"
"errors"
"fmt"
"io"
"strings"
Expand Down Expand Up @@ -113,14 +114,14 @@ func (b *Backend) pathRelease(ctx context.Context, req *logical.Request, fields
return nil, fmt.Errorf("error getting publisher repository: %w", err)
}

taskUUID, err := b.TasksManager.RunTask(context.Background(), req.Storage, func(ctx context.Context, storage logical.Storage) error {
taskUUID, err := b.TasksManager.RunTask(ctx, req.Storage, func(ctx context.Context, storage logical.Storage) error {
logboek.Context(ctx).Default().LogF("Started task\n")
b.Logger().Debug("Started task")

logboek.Context(ctx).Default().LogF("Cloning git repo\n")
b.Logger().Debug("Cloning git repo")

gitRepo, err := cloneGitRepositoryTag(cfg.GitRepoUrl, gitTag, gitUsername, gitPassword)
gitRepo, err := cloneGitRepositoryTag(ctx, cfg.GitRepoUrl, gitTag, gitUsername, gitPassword)
if err != nil {
return fmt.Errorf("unable to clone git repository: %w", err)
}
Expand Down Expand Up @@ -214,7 +215,7 @@ func (b *Backend) pathRelease(ctx context.Context, req *logical.Request, fields
return nil
})
if err != nil {
if err == tasks_manager.ErrBusy {
if errors.Is(err, tasks_manager.ErrBusy) {
return logical.ErrorResponse("busy"), nil
}

Expand All @@ -228,7 +229,7 @@ func (b *Backend) pathRelease(ctx context.Context, req *logical.Request, fields
}, nil
}

func cloneGitRepositoryTag(url, gitTag, username, password string) (*git.Repository, error) {
func cloneGitRepositoryTag(ctx context.Context, url, gitTag, username, password string) (*git.Repository, error) {
cloneGitOptions := trdlGit.CloneOptions{
TagName: gitTag,
RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
Expand All @@ -241,7 +242,7 @@ func cloneGitRepositoryTag(url, gitTag, username, password string) (*git.Reposit
}
}

gitRepo, err := trdlGit.CloneInMemory(url, cloneGitOptions)
gitRepo, err := trdlGit.CloneInMemory(ctx, url, cloneGitOptions)
if err != nil {
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions server/pkg/git/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package git

import (
"archive/tar"
"context"
"fmt"
"io"
"io/ioutil"
Expand All @@ -24,7 +25,7 @@ type CloneOptions struct {
Auth transport.AuthMethod
}

func CloneInMemory(url string, opts CloneOptions) (*git.Repository, error) {
func CloneInMemory(ctx context.Context, url string, opts CloneOptions) (*git.Repository, error) {
storage := memory.NewStorage()
fs := memfs.New()

Expand All @@ -50,7 +51,7 @@ func CloneInMemory(url string, opts CloneOptions) (*git.Repository, error) {
}
}

return git.Clone(storage, fs, cloneOptions)
return git.CloneContext(ctx, storage, fs, cloneOptions)
}

func AddWorktreeFilesToTar(tw *tar.Writer, gitRepo *git.Repository) error {
Expand Down
6 changes: 5 additions & 1 deletion server/pkg/git/signatures_loader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package git

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand All @@ -10,7 +12,9 @@ var _ = XDescribe("Signatures loader from git NOTES", func() {
It("successfully loads werf signatures", func() {
tagName := "v1.2.84+fix1"

repo, err := CloneInMemory("https://github.com/werf/werf.git", CloneOptions{TagName: tagName})
ctx := context.Background()

repo, err := CloneInMemory(ctx, "https://github.com/werf/werf.git", CloneOptions{TagName: tagName})
Expect(err).To(Succeed())

tref, err := repo.Tag(tagName)
Expand Down
7 changes: 5 additions & 2 deletions server/pkg/git/signatures_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package git

import (
"context"
_ "embed"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -119,8 +120,10 @@ var _ = Describe("VerifyTagSignatures and VerifyCommitSignatures", func() {
expectedErrMsg string
}

ctx := context.Background()

tableItBodyTagFunc := func(entry tableEntry) {
repo, err := CloneInMemory(testDir, CloneOptions{})
repo, err := CloneInMemory(ctx, testDir, CloneOptions{})
Expect(err).ShouldNot(HaveOccurred())

err = VerifyTagSignatures(
Expand All @@ -140,7 +143,7 @@ var _ = Describe("VerifyTagSignatures and VerifyCommitSignatures", func() {
}

tableItBodyCommitFunc := func(entry tableEntry) {
repo, err := CloneInMemory(testDir, CloneOptions{})
repo, err := CloneInMemory(ctx, testDir, CloneOptions{})
Expect(err).ShouldNot(HaveOccurred())

head, err := repo.Head()
Expand Down
2 changes: 1 addition & 1 deletion server/pkg/tasks_manager/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (m *Manager) RunTask(ctx context.Context, reqStorage logical.Storage, taskF
func (m *Manager) AddOptionalTask(ctx context.Context, reqStorage logical.Storage, taskFunc func(context.Context, logical.Storage) error) (string, bool, error) {
taskUUID, err := m.RunTask(ctx, reqStorage, taskFunc)
if err != nil {
if err == ErrBusy {
if errors.Is(err, ErrBusy) {
return taskUUID, false, nil
}

Expand Down