From f47063522670f779ddcce8432b2a4545970e8559 Mon Sep 17 00:00:00 2001 From: Michele Palazzi Date: Sat, 24 Feb 2024 16:43:28 +0100 Subject: [PATCH] fix: prs are now getting created Signed-off-by: Michele Palazzi --- README.md | 2 +- action.yml | 2 +- src/argoaction/git.go | 30 +++++-- src/argoaction/git_test.go | 117 ++++++++++---------------- src/argoaction/process_files.go | 2 +- src/argoaction/process_files_test.go | 63 -------------- src/go.mod | 2 +- src/internal/action_interface_test.go | 8 -- src/internal/git_ops.go | 22 ++++- src/internal/git_ops_test.go | 8 +- 10 files changed, 93 insertions(+), 163 deletions(-) diff --git a/README.md b/README.md index 02cb56a..70f4687 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ jobs: fetch-depth: '0' - name: Check updates for ArgoCD Apps - uses: ironashram/argocd-apps-action@v0.0.4 + uses: ironashram/argocd-apps-action@v0.0.5 with: target_branch: main create_pr: true diff --git a/action.yml b/action.yml index 057385d..3f3a0de 100644 --- a/action.yml +++ b/action.yml @@ -18,7 +18,7 @@ runs: steps: - name: Download ArgoCD Apps Action Binary env: - ACTION_VERSION: "v0.0.4" + ACTION_VERSION: "v0.0.5" shell: bash run: | wget https://github.com/ironashram/argocd-apps-action/releases/download/${ACTION_VERSION}/argocd-apps-action-${ACTION_VERSION}-linux-amd64.tar.gz diff --git a/src/argoaction/git.go b/src/argoaction/git.go index 507fc4d..0e4cffd 100644 --- a/src/argoaction/git.go +++ b/src/argoaction/git.go @@ -18,7 +18,19 @@ import ( "github.com/google/go-github/v59/github" ) -var createNewBranch = func(gitOps internal.GitOperations, branchName string) error { +var createNewBranch = func(gitOps internal.GitOperations, baseBranch, branchName string) error { + worktree, err := gitOps.Worktree() + if err != nil { + return err + } + + err = worktree.Checkout(&git.CheckoutOptions{ + Branch: plumbing.NewBranchReferenceName(baseBranch), + }) + if err != nil { + return err + } + headRef, err := gitOps.Head() if err != nil { return err @@ -31,7 +43,7 @@ var createNewBranch = func(gitOps internal.GitOperations, branchName string) err return fmt.Errorf("failed to create new branch: %w", err) } - worktree, err := gitOps.Worktree() + worktree, err = gitOps.Worktree() if err != nil { return err } @@ -52,11 +64,11 @@ var commitChanges = func(gitOps internal.GitOperations, path string, commitMessa return fmt.Errorf("failed to commit changes: %w", err) } - realWorktree, ok := worktree.(*git.Worktree) - if !ok { - return fmt.Errorf("failed to assert worktree type") - } - basePath := realWorktree.Filesystem.Root() + basePath, err := worktree.Root() + if err != nil { + return fmt.Errorf("failed to get worktree root: %w", err) + } + relativePath, err := filepath.Rel(basePath, path) if err != nil { return fmt.Errorf("failed to get relative path: %w", err) @@ -86,7 +98,7 @@ var pushChanges = func(gitOps internal.GitOperations, branchName string, cfg *mo Username: "github-actions[bot]", Password: cfg.Token, }, - RefSpecs: []config.RefSpec{config.RefSpec(branchName + ":" + branchName)}, + RefSpecs: []config.RefSpec{config.RefSpec("refs/heads/" + branchName + ":refs/heads/" + branchName)}, }) if err != nil { return fmt.Errorf("failed to push changes: %w", err) @@ -117,7 +129,7 @@ var createPullRequest = func(githubClient internal.GitHubClient, baseBranch stri return errors.New("action is nil") } - _, _, err := pullRequests.Create(context.Background(), cfg.Owner, cfg.Repo, newPR) + _, _, err := pullRequests.Create(context.Background(), cfg.Owner, cfg.Name, newPR) if err != nil { return err } diff --git a/src/argoaction/git_test.go b/src/argoaction/git_test.go index bb30d44..fa790c1 100644 --- a/src/argoaction/git_test.go +++ b/src/argoaction/git_test.go @@ -5,13 +5,8 @@ import ( "errors" "fmt" "testing" - "time" - "github.com/go-git/go-billy/v5/memfs" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/go-git/go-git/v5/storage/memory" "github.com/google/go-github/v59/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -24,7 +19,7 @@ func TestCreatePullRequest(t *testing.T) { cfg := &models.Config{ CreatePr: true, TargetBranch: "main", - Repo: "your-github-repo", + Name: "your-github-repo", Owner: "your-github-owner", Token: "your-github-token", } @@ -49,9 +44,9 @@ func TestCreatePullRequest(t *testing.T) { mockClient = &internal.MockGithubClient{ PullRequestsService: &internal.MockPullRequestsService{ - CreateFunc: func(ctx context.Context, owner string, repo string, newPR *github.NewPullRequest) (*github.PullRequest, *github.Response, error) { + CreateFunc: func(ctx context.Context, owner string, name string, newPR *github.NewPullRequest) (*github.PullRequest, *github.Response, error) { assert.Equal(t, cfg.Owner, owner) - assert.Equal(t, cfg.Repo, repo) + assert.Equal(t, cfg.Name, name) assert.Equal(t, expectedPR, newPR) return nil, nil, nil }, @@ -80,7 +75,7 @@ func TestCreatePullRequest_Error(t *testing.T) { cfg := &models.Config{ CreatePr: true, TargetBranch: "main", - Repo: "your-github-repo", + Name: "your-github-repo", Owner: "your-github-owner", Token: "your-github-token", } @@ -106,9 +101,9 @@ func TestCreatePullRequest_Error(t *testing.T) { expectedError := errors.New("failed to create pull request") mockClient = &internal.MockGithubClient{ PullRequestsService: &internal.MockPullRequestsService{ - CreateFunc: func(ctx context.Context, owner string, repo string, newPR *github.NewPullRequest) (*github.PullRequest, *github.Response, error) { + CreateFunc: func(ctx context.Context, owner string, name string, newPR *github.NewPullRequest) (*github.PullRequest, *github.Response, error) { assert.Equal(t, cfg.Owner, owner) - assert.Equal(t, cfg.Repo, repo) + assert.Equal(t, cfg.Name, name) assert.Equal(t, expectedPR, newPR) return nil, nil, expectedError }, @@ -120,73 +115,40 @@ func TestCreatePullRequest_Error(t *testing.T) { assert.EqualError(t, err, expectedError.Error()) } -func setupRepoAndInitialCommit() (*git.Repository, error) { - repo, err := git.Init(memory.NewStorage(), memfs.New()) - if err != nil { - return nil, err - } - - wt, err := repo.Worktree() - if err != nil { - return nil, err - } - - file, err := wt.Filesystem.Create("test.txt") - if err != nil { - return nil, err - } - - _, err = file.Write([]byte("This is a test file")) - if err != nil { - return nil, err - } - - file.Close() - - _, err = wt.Add("test.txt") - if err != nil { - return nil, err - } - - _, err = wt.Commit("Initial commit", &git.CommitOptions{ - Author: &object.Signature{ - Name: "Test", - Email: "test@test.com", - When: time.Now(), - }, - }) - - if err != nil { - return nil, err - } +func TestCreateNewBranch(t *testing.T) { + gitOps := new(internal.MockGitRepo) + worktree := new(internal.MockWorktree) - return repo, nil -} + headRef := plumbing.NewHashReference(plumbing.HEAD, plumbing.ZeroHash) + gitOps.On("Worktree").Return(worktree, nil) + gitOps.On("Head").Return(headRef, nil) + gitOps.On("SetReference", mock.Anything, mock.Anything).Return(nil) -func TestCreateNewBranch(t *testing.T) { - repo, err := setupRepoAndInitialCommit() - assert.NoError(t, err) - gitOps := &internal.GitRepo{Repo: repo} + worktree.On("Checkout", mock.Anything).Return(nil) - err = createNewBranch(gitOps, "test-branch") - assert.NoError(t, err) + err := createNewBranch(gitOps, "base", "new-branch") - ref, err := repo.Reference(plumbing.NewBranchReferenceName("test-branch"), true) + gitOps.AssertExpectations(t) + worktree.AssertExpectations(t) assert.NoError(t, err) - assert.NotNil(t, ref) } func TestCommitChanges(t *testing.T) { - repo, err := setupRepoAndInitialCommit() - assert.NoError(t, err) - gitOps := &internal.GitRepo{Repo: repo} + mockRepo := new(internal.MockGitRepo) + worktree := new(internal.MockWorktree) - err = commitChanges(gitOps, ".", "Test commit") - assert.NoError(t, err) + mockRepo.On("Worktree").Return(worktree, nil) + hash := plumbing.NewHash("0000000000000000000000000000000000000000") + worktree.On("Add", ".").Return(hash, nil) + commitHash := plumbing.NewHash("0000000000000000000000000000000000000001") + worktree.On("Commit", "Test commit", mock.Anything).Return(commitHash, nil) + worktree.On("Root").Return("/valid/path", nil) + + err := commitChanges(mockRepo, "/valid/path", "Test commit") - ref, _ := repo.Head() - commit, _ := repo.CommitObject(ref.Hash()) - assert.Equal(t, "Test commit", commit.Message) + assert.NoError(t, err) + mockRepo.AssertExpectations(t) + worktree.AssertExpectations(t) } func TestPushChanges(t *testing.T) { @@ -199,22 +161,29 @@ func TestPushChanges(t *testing.T) { func TestCreateNewBranch_Error(t *testing.T) { expectedError := fmt.Errorf("failed to create new branch: %w", errors.New("some error")) - mockRepo := &internal.MockGitRepo{} - dummyRef := &plumbing.Reference{} - mockRepo.On("Head").Return(dummyRef, expectedError) - err := createNewBranch(mockRepo, "test-branch") + mockRepo := new(internal.MockGitRepo) + worktree := new(internal.MockWorktree) + + mockRepo.On("Worktree").Return(worktree, nil) + + worktree.On("Checkout", mock.Anything).Return(expectedError) + + err := createNewBranch(mockRepo, "main", "test-branch") + assert.EqualError(t, err, expectedError.Error()) mockRepo.AssertExpectations(t) + worktree.AssertExpectations(t) } func TestCommitChanges_Error(t *testing.T) { expectedError := fmt.Errorf("failed to commit changes: %w", errors.New("some error")) - mockRepo := &internal.MockGitRepo{} - mockWorktree := &git.Worktree{} + mockRepo := new(internal.MockGitRepo) + mockWorktree := new(internal.MockWorktree) mockRepo.On("Worktree").Return(mockWorktree, expectedError) err := commitChanges(mockRepo, ".", "Test commit") assert.EqualError(t, err, fmt.Errorf("failed to commit changes: %w", expectedError).Error()) mockRepo.AssertExpectations(t) + mockWorktree.AssertExpectations(t) } func TestPushChanges_Error(t *testing.T) { diff --git a/src/argoaction/process_files.go b/src/argoaction/process_files.go index f5503e8..78744b9 100644 --- a/src/argoaction/process_files.go +++ b/src/argoaction/process_files.go @@ -116,7 +116,7 @@ var processFile = func(path string, gitOps internal.GitOperations, githubClient if cfg.CreatePr { branchName := "update-" + chart - err = createNewBranch(gitOps, branchName) + err = createNewBranch(gitOps, cfg.TargetBranch, branchName) if err != nil { action.Debugf("Error creating new branch: %v\n", err) return err diff --git a/src/argoaction/process_files_test.go b/src/argoaction/process_files_test.go index 5db628e..eff3634 100644 --- a/src/argoaction/process_files_test.go +++ b/src/argoaction/process_files_test.go @@ -1,11 +1,9 @@ package argoaction import ( - "errors" "net/http" "testing" - "github.com/go-git/go-git/v5/plumbing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "sigs.k8s.io/yaml" @@ -91,65 +89,4 @@ func TestProcessFile(t *testing.T) { mockOSInterface.AssertExpectations(t) mockGitHubClient.AssertExpectations(t) }) - - t.Run("Error creating new branch", func(t *testing.T) { - cfg.CreatePr = true - mockRepo := new(internal.MockGitRepo) - mockAction.On("Debugf", "Checking %s from %s, current version is %s\n", mock.AnythingOfType("[]interface {}")).Once() - mockAction.On("Infof", "There is a newer %s version: %s\n", mock.AnythingOfType("[]interface {}")).Once() - mockAction.On("Debugf", "Error creating new branch: %v\n", mock.AnythingOfType("[]interface {}")).Once() - - mockRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName("master"), plumbing.NewHash("e5bd3914e2e596debea16f433f57875b5b90bcd6")) - mockRepo.On("Head").Return(mockRef, nil) - mockRepo.On("SetReference", mock.AnythingOfType("string"), mock.AnythingOfType("*plumbing.Reference")).Return(nil) - - mockWorktree := new(internal.MockWorktree) - mockWorktree.On("Checkout", mock.AnythingOfType("*git.CheckoutOptions")).Return(errors.New("error creating new branch")).Once() - mockRepo.On("Worktree").Return(mockWorktree, nil) - - fileContent := []byte("spec:\n source:\n chart: chart1\n repoURL: https://test.local\n targetRevision: 0.1.2 \n") - mockOSInterface.On("ReadFile", mock.Anything).Return([]byte(fileContent), nil).Once() - - err := processFile("valid.yaml", mockRepo, mockGitHubClient, cfg, mockAction, mockOSInterface) - - assert.Error(t, err) - mockAction.AssertExpectations(t) - mockOSInterface.AssertExpectations(t) - mockRepo.AssertExpectations(t) - }) - - t.Run("Error creating pull request", func(t *testing.T) { - cfg.CreatePr = true - mockRepo := new(internal.MockGitRepo) - mockAction.On("Debugf", "Checking %s from %s, current version is %s\n", mock.AnythingOfType("[]interface {}")).Once() - mockAction.On("Infof", "There is a newer %s version: %s\n", mock.AnythingOfType("[]interface {}")).Once() - // mockAction.On("Debugf", "Error marshaling app: %v\n", mock.AnythingOfType("[]interface {}")).Once() - mockAction.On("Debugf", "Error creating pull request: %v\n", mock.Anything).Return().Once() - - // mockRepo.On("CreateNewBranch", mock.AnythingOfType("string")).Return(nil).Once() - - mockWorktree := new(internal.MockWorktree) - mockWorktree.On("Checkout", mock.AnythingOfType("*git.CheckoutOptions")).Return(nil) - mockWorktree.On("Add", mock.AnythingOfType("string")).Return(plumbing.NewHash("0000000000000000000000000000000000000000"), nil) - mockWorktree.On("Commit", mock.AnythingOfType("string"), mock.AnythingOfType("*git.CommitOptions")).Return(plumbing.NewHash("0000000000000000000000000000000000000000"), nil) - mockRepo.On("Worktree").Return(mockWorktree, nil) - - mockRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName("master"), plumbing.NewHash("e5bd3914e2e596debea16f433f57875b5b90bcd6")) - mockRepo.On("Head").Return(mockRef, nil) - - mockRepo.On("SetReference", mock.AnythingOfType("string"), mock.Anything).Return(nil) - mockRepo.On("Push", mock.AnythingOfType("*git.PushOptions")).Return(nil) - - fileContent := []byte("spec:\n source:\n chart: chart1\n repoURL: https://test.local\n targetRevision: 0.1.2 \n") - mockOSInterface.On("ReadFile", mock.Anything).Return([]byte(fileContent), nil).Once() - mockOSInterface.On("WriteFile", "valid.yaml", []byte("spec:\n source:\n chart: chart1\n repoURL: https://test.local\n targetRevision: 1.7.0\n"), mock.AnythingOfType("fs.FileMode")).Return(nil) - - err := processFile("valid.yaml", mockRepo, mockGitHubClient, cfg, mockAction, mockOSInterface) - - assert.Error(t, err) - mockAction.AssertExpectations(t) - mockOSInterface.AssertExpectations(t) - mockRepo.AssertExpectations(t) - mockWorktree.AssertExpectations(t) - }) } diff --git a/src/go.mod b/src/go.mod index 142550c..4a09e2b 100644 --- a/src/go.mod +++ b/src/go.mod @@ -4,7 +4,6 @@ go 1.22.0 require ( github.com/Masterminds/semver v1.5.0 - github.com/go-git/go-billy/v5 v5.5.0 github.com/go-git/go-git/v5 v5.11.0 github.com/google/go-github/v59 v59.0.0 github.com/jarcoal/httpmock v1.3.1 @@ -23,6 +22,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/emirpasic/gods v1.18.1 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect + github.com/go-git/go-billy/v5 v5.5.0 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/go-querystring v1.1.0 // indirect diff --git a/src/internal/action_interface_test.go b/src/internal/action_interface_test.go index 848da46..3511a7e 100644 --- a/src/internal/action_interface_test.go +++ b/src/internal/action_interface_test.go @@ -7,7 +7,6 @@ import ( ) func TestActionInterface_GetInput(t *testing.T) { - // Test cases testCases := []struct { name string action *MockActionInterface @@ -38,7 +37,6 @@ func TestActionInterface_GetInput(t *testing.T) { }, } - // Run tests for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { result := tc.action.GetInput(tc.input) @@ -50,7 +48,6 @@ func TestActionInterface_GetInput(t *testing.T) { } func TestActionInterface_Getenv(t *testing.T) { - // Test cases testCases := []struct { name string action *MockActionInterface @@ -81,7 +78,6 @@ func TestActionInterface_Getenv(t *testing.T) { }, } - // Run tests for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { result := tc.action.Getenv(tc.env) @@ -93,7 +89,6 @@ func TestActionInterface_Getenv(t *testing.T) { } func TestActionInterface_Debugf(t *testing.T) { - // Test cases testCases := []struct { name string action *MockActionInterface @@ -120,7 +115,6 @@ func TestActionInterface_Debugf(t *testing.T) { }, } - // Run tests for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { tc.action.On("Debugf", tc.format, mock.Anything).Once() @@ -130,7 +124,6 @@ func TestActionInterface_Debugf(t *testing.T) { } func TestActionInterface_Fatalf(t *testing.T) { - // Test cases testCases := []struct { name string action *MockActionInterface @@ -157,7 +150,6 @@ func TestActionInterface_Fatalf(t *testing.T) { }, } - // Run tests for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { tc.action.On("Fatalf", tc.format, mock.Anything).Once() diff --git a/src/internal/git_ops.go b/src/internal/git_ops.go index 0fbfc83..1812b1c 100644 --- a/src/internal/git_ops.go +++ b/src/internal/git_ops.go @@ -2,6 +2,7 @@ package internal import ( "io" + "path/filepath" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" @@ -37,7 +38,11 @@ func (r *GitRepo) Worktree() (WorktreeOperations, error) { if err != nil { return nil, err } - return worktree, nil + root, err := filepath.Abs(worktree.Filesystem.Root()) + if err != nil { + return nil, err + } + return &WorktreeWithRoot{worktree, root}, nil } func (r *GitRepo) Head() (*plumbing.Reference, error) { @@ -138,6 +143,7 @@ type WorktreeOperations interface { Checkout(opts *git.CheckoutOptions) error Add(path string) (plumbing.Hash, error) Commit(message string, opts *git.CommitOptions) (plumbing.Hash, error) + Root() (string, error) } type MockWorktree struct { @@ -158,3 +164,17 @@ func (m *MockWorktree) Commit(message string, opts *git.CommitOptions) (plumbing args := m.Called(message, opts) return args.Get(0).(plumbing.Hash), args.Error(1) } + +func (m *MockWorktree) Root() (string, error) { + args := m.Called() + return args.String(0), args.Error(1) +} + +type WorktreeWithRoot struct { + *git.Worktree + root string +} + +func (w *WorktreeWithRoot) Root() (string, error) { + return w.root, nil +} diff --git a/src/internal/git_ops_test.go b/src/internal/git_ops_test.go index 43fd3c1..c4a3346 100644 --- a/src/internal/git_ops_test.go +++ b/src/internal/git_ops_test.go @@ -37,10 +37,9 @@ func TestGitRepo_SetReference(t *testing.T) { } func TestGitRepo_Worktree(t *testing.T) { - mockRepo := &MockGitRepo{} - repo, _ := git.Init(memory.NewStorage(), nil) - worktree, _ := repo.Worktree() - mockRepo.On("Worktree").Return(worktree, nil) + mockRepo := new(MockGitRepo) + mockWorktree := new(MockWorktree) + mockRepo.On("Worktree").Return(mockWorktree, nil) _, err := mockRepo.Worktree() if err != nil { @@ -48,6 +47,7 @@ func TestGitRepo_Worktree(t *testing.T) { } mockRepo.AssertExpectations(t) + mockWorktree.AssertExpectations(t) } func TestGitRepo_Head(t *testing.T) {