From d3265145e9ee6e04c1d9ef1182ff7af0183ddd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Fri, 18 Oct 2024 04:08:00 +0200 Subject: [PATCH] CLeanup bumper code --- cmd/image-autobumper/Dockerfile | 2 +- pkg/github/bumper/bumper.go | 127 ------------------------------- pkg/github/bumper/bumper_test.go | 70 ----------------- pkg/github/bumper/utils.go | 43 ----------- pkg/github/bumper/utils_test.go | 100 ------------------------ 5 files changed, 1 insertion(+), 341 deletions(-) delete mode 100644 pkg/github/bumper/utils.go delete mode 100644 pkg/github/bumper/utils_test.go diff --git a/cmd/image-autobumper/Dockerfile b/cmd/image-autobumper/Dockerfile index 6b58658b1a97..4b8ae7219e14 100644 --- a/cmd/image-autobumper/Dockerfile +++ b/cmd/image-autobumper/Dockerfile @@ -22,6 +22,6 @@ LABEL io.kyma-project.source=github.com/kyma-project/test-infra/cmd/image-autobu # Copy the built Go app from the builder stage COPY --from=builder /app/cmd/image-autobumper/main /image-autobumper -RUN apk add --no-cache ca-certificates git && \ +RUN apk add --no-cache ca-certificates && \ chmod +x /image-autobumper ENTRYPOINT ["/image-autobumper"] \ No newline at end of file diff --git a/pkg/github/bumper/bumper.go b/pkg/github/bumper/bumper.go index 9b5fdeef6e10..54faf2b26835 100644 --- a/pkg/github/bumper/bumper.go +++ b/pkg/github/bumper/bumper.go @@ -1,14 +1,9 @@ package bumper import ( - "bytes" "context" - "errors" "fmt" - "io" "os" - "os/exec" - "strings" "time" "github.com/go-git/go-git/v5" @@ -77,75 +72,6 @@ type PRHandler interface { PRTitleBody() (string, string, error) } -func Call(stdout, stderr io.Writer, cmd string, args ...string) error { - (&logrus.Logger{ - Out: stderr, - Formatter: logrus.StandardLogger().Formatter, - Hooks: logrus.StandardLogger().Hooks, - Level: logrus.StandardLogger().Level, - }).WithField("cmd", cmd). - // The default formatting uses a space as separator, which is hard to read if an arg contains a space - WithField("args", fmt.Sprintf("['%s']", strings.Join(args, "', '"))). - Info("running command") - - c := exec.Command(cmd, args...) - c.Stdout = stdout - c.Stderr = stderr - return c.Run() -} - -func getTreeRef(stderr io.Writer, refname string) (string, error) { - revParseStdout := &bytes.Buffer{} - if err := Call(revParseStdout, stderr, gitCmd, "rev-parse", refname+":"); err != nil { - return "", fmt.Errorf("parse ref: %w", err) - } - fields := strings.Fields(revParseStdout.String()) - if n := len(fields); n < 1 { - return "", errors.New("got no otput when trying to rev-parse") - } - return fields[0], nil -} - -func gitStatus(stdout io.Writer, stderr io.Writer) (string, error) { - tmpRead, tmpWrite, err := os.Pipe() - if err != nil { - return "", err - } - - if err := Call(tmpWrite, stderr, gitCmd, "status"); err != nil { - return "", fmt.Errorf("git status: %w", err) - } - tmpWrite.Close() - output, err := io.ReadAll(tmpRead) - if err != nil { - return "", err - } - stdout.Write(output) - return string(output), nil -} - -func gitAdd(files []string, stdout, stderr io.Writer) error { - if err := Call(stdout, stderr, gitCmd, "add", strings.Join(files, " ")); err != nil { - return fmt.Errorf("git add: %w", err) - } - - return nil -} - -func gitCommit(name, email, message string, stdout, stderr io.Writer, signoff bool) error { - commitArgs := []string{"commit", "-m", message} - if name != "" && email != "" { - commitArgs = append(commitArgs, "--author", fmt.Sprintf("%s <%s>", name, email)) - } - if signoff { - commitArgs = append(commitArgs, "--signoff") - } - if err := Call(stdout, stderr, gitCmd, commitArgs...); err != nil { - return fmt.Errorf("git commit: %w", err) - } - return nil -} - func gitPush(remote, remoteBranch string, repo *git.Repository, auth transport.AuthMethod, dryrun bool) error { if _, err := repo.CreateRemote(&config.RemoteConfig{ Name: forkRemoteName, @@ -153,9 +79,6 @@ func gitPush(remote, remoteBranch string, repo *git.Repository, auth transport.A }); err != nil && err != git.ErrRemoteExists { return fmt.Errorf("create remote: %w", err) } - // if err := Call(stdout, stderr, gitCmd, "remote", "add", forkRemoteName, remote); err != nil { - // return fmt.Errorf("add remote: %w", err) - // } remoteRef, err := repo.Reference(plumbing.ReferenceName(fmt.Sprintf("refs/remotes/%s/%s", forkRemoteName, remoteBranch)), true) if err != nil && err != plumbing.ErrReferenceNotFound { @@ -167,31 +90,12 @@ func gitPush(remote, remoteBranch string, repo *git.Repository, auth transport.A remoteTreeRef = remoteRef.Hash().String() } - // fetchStderr := &bytes.Buffer{} - // var remoteTreeRef string - // if err := Call(stdout, fetchStderr, gitCmd, "fetch", forkRemoteName, remoteBranch); err != nil { - // logrus.Info("fetchStderr is : ", fetchStderr.String()) - // if !strings.Contains(strings.ToLower(fetchStderr.String()), fmt.Sprintf("couldn't find remote ref %s", remoteBranch)) { - // return fmt.Errorf("fetch from fork: %w", err) - // } - // } else { - // var err error - // remoteTreeRef, err = getTreeRef(stderr, fmt.Sprintf("refs/remotes/%s/%s", forkRemoteName, remoteBranch)) - // if err != nil { - // return fmt.Errorf("get remote tree ref: %w", err) - // } - // } - localRef, err := repo.Head() if err != nil { return fmt.Errorf("get local ref: %w", err) } localTreeRef := localRef.Hash().String() - // localTreeRef, err := getTreeRef(stderr, "HEAD") - // if err != nil { - // return fmt.Errorf("get local tree ref: %w", err) - // } if dryrun { logrus.Info("[Dryrun] Skip git push with: ") @@ -209,29 +113,12 @@ func gitPush(remote, remoteBranch string, repo *git.Repository, auth transport.A }); err != nil { return fmt.Errorf("push to remote: %w", err) } - // if err := GitPush(forkRemoteName, remoteBranch, stdout, stderr, ""); err != nil { - // return err - // } } else { logrus.Info("Not pushing as up-to-date remote branch already exists") } return nil } -// GitPush push the changes to the given remote and branch. -func GitPush(remote, remoteBranch string, stdout, stderr io.Writer, workingDir string) error { - logrus.Info("Pushing to remote...") - gc := GitCommand{ - baseCommand: gitCmd, - args: []string{"push", "-f", remote, fmt.Sprintf("HEAD:%s", remoteBranch)}, - workingDir: workingDir, - } - if err := gc.Call(stdout, stderr); err != nil { - return fmt.Errorf("%s: %w", gc.getCommand(), err) - } - return nil -} - // UpdatePullRequestWithLabels updates with github client "gc" the PR of github repo org/repo // with "title" and "body" of PR matching author and headBranch from "source" to "baseBranch" with labels func UpdatePullRequestWithLabels(gc github.Client, org, repo, title, body, source, baseBranch, @@ -375,14 +262,6 @@ func processGitHub(o *Options, prh PRHandler) error { logrus.Info("No changes, quitting.") return nil } - // resp, err := gitStatus(stdout, stderr) - // if err != nil { - // return fmt.Errorf("git status: %w", err) - // } - // if strings.Contains(resp, "nothing to commit, working tree clean") { - // fmt.Println("No changes, quitting.") - // return nil - // } logrus.Info("Adding changes to the stage") for _, file := range filesToBeAdded { @@ -397,9 +276,6 @@ func processGitHub(o *Options, prh PRHandler) error { return fmt.Errorf("add file %s to the worktree: %w", file, err) } } - // if err := gitAdd(filesToBeAdded, stdout, stderr); err != nil { - // return fmt.Errorf("add changes to commit %w", err) - // } logrus.Infof("Committing changes with message: %s", commitMsg) commitOpts := &git.CommitOptions{ @@ -412,9 +288,6 @@ func processGitHub(o *Options, prh PRHandler) error { if _, err := workTree.Commit(commitMsg, commitOpts); err != nil { return fmt.Errorf("commit changes to the remote branch: %w", err) } - // if err := gitCommit(o.GitName, o.GitEmail, commitMsg, stdout, stderr, o.Signoff); err != nil { - // return fmt.Errorf("commit changes to the remote branch: %w", err) - // } } remote := fmt.Sprintf("https://%s:%s@%s/%s/%s.git", o.GitHubLogin, string(secret.GetTokenGenerator(o.GitHubToken)()), githubHost, o.GitHubLogin, o.RemoteName) diff --git a/pkg/github/bumper/bumper_test.go b/pkg/github/bumper/bumper_test.go index 8b78f9f4f209..5c4a3c3465d5 100644 --- a/pkg/github/bumper/bumper_test.go +++ b/pkg/github/bumper/bumper_test.go @@ -18,11 +18,8 @@ package bumper import ( "os" - "path/filepath" "strings" "testing" - - "k8s.io/test-infra/prow/config/secret" ) func TestValidateOptions(t *testing.T) { @@ -119,73 +116,6 @@ func writeToFile(t *testing.T, path, content string) { } } -func TestCallWithWriter(t *testing.T) { - dir := t.TempDir() - - file1 := filepath.Join(dir, "secret1") - file2 := filepath.Join(dir, "secret2") - - writeToFile(t, file1, "abc") - writeToFile(t, file2, "xyz") - - if err := secret.Add(file1, file2); err != nil { - t.Errorf("failed to start secrets agent; %v", err) - } - - var fakeOut fakeWriter - var fakeErr fakeWriter - - stdout := CensoredWriter{Delegate: &fakeOut, Censor: secret.Censor} - stderr := CensoredWriter{Delegate: &fakeErr, Censor: secret.Censor} - - testCases := []struct { - description string - command string - args []string - expectedOut string - expectedErr string - }{ - { - description: "no secret in stdout are working well", - command: "echo", - args: []string{"-n", "aaa: 123"}, - expectedOut: "aaa: 123", - }, - { - description: "secret in stdout are censored", - command: "echo", - args: []string{"-n", "abc: 123"}, - expectedOut: "XXX: 123", - }, - { - description: "secret in stderr are censored", - command: "ls", - args: []string{"/tmp/file-not-exist/abc/xyz/file-not-exist"}, - expectedErr: "/tmp/file-not-exist/XXX/XXX/file-not-exist", - }, - { - description: "no secret in stderr are working well", - command: "ls", - args: []string{"/tmp/file-not-exist/aaa/file-not-exist"}, - expectedErr: "/tmp/file-not-exist/aaa/file-not-exist", - }, - } - - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - fakeOut.result = []byte{} - fakeErr.result = []byte{} - _ = Call(stdout, stderr, tc.command, tc.args...) - if full, want := string(fakeOut.result), tc.expectedOut; !strings.Contains(full, want) { - t.Errorf("stdout does not contain %q, got %q", full, want) - } - if full, want := string(fakeErr.result), tc.expectedErr; !strings.Contains(full, want) { - t.Errorf("stderr does not contain %q, got %q", full, want) - } - }) - } -} - func TestGetAssignment(t *testing.T) { cases := []struct { description string diff --git a/pkg/github/bumper/utils.go b/pkg/github/bumper/utils.go deleted file mode 100644 index c76887b82783..000000000000 --- a/pkg/github/bumper/utils.go +++ /dev/null @@ -1,43 +0,0 @@ -package bumper - -import ( - "fmt" - "io" - "strings" -) - -// GitCommand is used to pass the various components of the git command which needs to be executed -type GitCommand struct { - baseCommand string - args []string - workingDir string -} - -func (gc *GitCommand) Call(stdout, stderr io.Writer) error { - return Call(stdout, stderr, gc.baseCommand, gc.buildCommand()...) -} - -func (gc *GitCommand) buildCommand() []string { - var args []string - if gc.workingDir != "" { - args = append(args, "-C", gc.workingDir) - } - args = append(args, gc.args...) - - return args -} - -func (gc *GitCommand) getCommand() string { - return fmt.Sprintf("%s %s", gc.baseCommand, strings.Join(gc.buildCommand(), " ")) -} - -// CensoredWriter is wrapper for io.writer which will censor secrets using provided censor -type CensoredWriter struct { - Delegate io.Writer - Censor func(content []byte) []byte -} - -func (w CensoredWriter) Write(content []byte) (int, error) { - censored := w.Censor(content) - return w.Delegate.Write(censored) -} diff --git a/pkg/github/bumper/utils_test.go b/pkg/github/bumper/utils_test.go deleted file mode 100644 index ec1a8a1d6afd..000000000000 --- a/pkg/github/bumper/utils_test.go +++ /dev/null @@ -1,100 +0,0 @@ -package bumper - -import ( - "reflect" - "testing" -) - -func Test_buildCommand(t *testing.T) { - tc := []struct { - name string - command GitCommand - expected []string - }{ - { - name: "simple command without working directory", - command: GitCommand{ - baseCommand: "git", - args: []string{"add", "test.txt"}, - }, - expected: []string{"add", "test.txt"}, - }, - { - name: "command with working directory", - command: GitCommand{ - baseCommand: "git", - args: []string{"add", "test.txt"}, - workingDir: "test/directory", - }, - expected: []string{"-C", "test/directory", "add", "test.txt"}, - }, - } - - for _, c := range tc { - t.Run(c.name, func(t *testing.T) { - command := c.command.buildCommand() - - if !reflect.DeepEqual(command, c.expected) { - t.Errorf("buildCommand(): got %v, expected %v", command, c.expected) - } - }) - } -} - -type fakeWriter struct { - result []byte -} - -func (w *fakeWriter) Write(content []byte) (int, error) { - w.result = append(w.result, content...) - return len(content), nil -} - -func TestWrite(t *testing.T) { - tc := []struct { - name string - content []byte - expected []byte - censor func(content []byte) []byte - }{ - { - name: "censor don't change output", - content: []byte("some string without secrets"), - expected: []byte("some string without secrets"), - censor: func(content []byte) []byte { - return content - }, - }, - { - name: "censored secret", - content: []byte("some secret string"), - expected: []byte("some XXX string"), - censor: func(content []byte) []byte { - return []byte("some XXX string") - }, - }, - } - - for _, c := range tc { - t.Run(c.name, func(t *testing.T) { - writer := fakeWriter{} - cw := CensoredWriter{ - Delegate: &writer, - Censor: c.censor, - } - - written, err := cw.Write(c.content) - if err != nil { - t.Errorf("got unexpected error: %s", err) - } - - if written != len(c.expected) { - t.Errorf("written bytes doesn't match expected: got %d, expected %d", written, len(c.expected)) - } - - if string(writer.result) != string(c.expected) { - t.Errorf("expected %s, got %s", string(c.expected), string(writer.result)) - } - }) - } -}