From eb70397807d1f7140791077009a7b7fcd0ceed8b Mon Sep 17 00:00:00 2001 From: Kent Rancourt Date: Tue, 3 Sep 2024 17:05:52 -0400 Subject: [PATCH] add git-based directives Signed-off-by: Kent Rancourt --- hack/codegen/directive-configs.sh | 4 + internal/directives/copy_directive.go | 5 +- internal/directives/directive.go | 10 +- internal/directives/git_clone_directive.go | 178 +++++- .../directives/git_clone_directive_test.go | 541 ++++++++++-------- internal/directives/git_commit_directive.go | 131 ++++- .../directives/git_commit_directive_test.go | 369 +++++++++--- internal/directives/git_open_pr_directive.go | 258 +++++++++ .../directives/git_open_pr_directive_test.go | 217 +++++++ .../directives/git_overwrite_directive.go | 116 ++++ .../git_overwrite_directive_test.go | 154 +++++ internal/directives/git_push_directive.go | 116 +++- .../directives/git_push_directive_test.go | 311 ++++++---- .../directives/git_wait_for_pr_directive.go | 162 ++++++ .../git_wait_for_pr_directive_test.go | 236 ++++++++ .../directives/schemas/git-clone-config.json | 4 + .../directives/schemas/git-commit-config.json | 28 +- .../schemas/git-open-pr-config.json | 57 ++ .../schemas/git-overwrite-config.json | 19 + .../schemas/git-wait-for-pr-config.json | 47 ++ internal/directives/zz_config_types.go | 61 +- internal/gitprovider/gitprovider.go | 41 ++ 22 files changed, 2588 insertions(+), 477 deletions(-) create mode 100644 internal/directives/git_open_pr_directive.go create mode 100644 internal/directives/git_open_pr_directive_test.go create mode 100644 internal/directives/git_overwrite_directive.go create mode 100644 internal/directives/git_overwrite_directive_test.go create mode 100644 internal/directives/git_wait_for_pr_directive.go create mode 100644 internal/directives/git_wait_for_pr_directive_test.go create mode 100644 internal/directives/schemas/git-open-pr-config.json create mode 100644 internal/directives/schemas/git-overwrite-config.json create mode 100644 internal/directives/schemas/git-wait-for-pr-config.json diff --git a/hack/codegen/directive-configs.sh b/hack/codegen/directive-configs.sh index 8dd4151250..d1b074751f 100755 --- a/hack/codegen/directive-configs.sh +++ b/hack/codegen/directive-configs.sh @@ -21,6 +21,10 @@ printf "${generated_code_warning}$(cat ${out_file})" > ${out_file} # on Linux. So we use -i.bak, which works on both. sed -i.bak 's/\*bool/bool/g' ${out_file} sed -i.bak 's/\*string/string/g' ${out_file} +# As of right now, this transformation is ok, but we can revisit it if we ever +# need nullable numbers or non-int numbers. +sed -i.bak 's/\*float64/int64/g' ${out_file} + rm ${out_file}.bak gofmt -w ${out_file} diff --git a/internal/directives/copy_directive.go b/internal/directives/copy_directive.go index fc4d734535..7b8c694f80 100644 --- a/internal/directives/copy_directive.go +++ b/internal/directives/copy_directive.go @@ -34,16 +34,15 @@ func (d *copyDirective) Name() string { } func (d *copyDirective) Run(ctx context.Context, stepCtx *StepContext) (Result, error) { - failure := Result{Status: StatusFailure} // Validate the configuration against the JSON Schema. if err := validate(d.schemaLoader, gojsonschema.NewGoLoader(stepCtx.Config), d.Name()); err != nil { - return failure, err + return Result{Status: StatusFailure}, err } // Convert the configuration into a typed object. cfg, err := configToStruct[CopyConfig](stepCtx.Config) if err != nil { - return failure, fmt.Errorf("could not convert config into %s config: %w", d.Name(), err) + return Result{Status: StatusFailure}, fmt.Errorf("could not convert config into %s config: %w", d.Name(), err) } return d.run(ctx, stepCtx, cfg) diff --git a/internal/directives/directive.go b/internal/directives/directive.go index 5694589e57..7328d192de 100644 --- a/internal/directives/directive.go +++ b/internal/directives/directive.go @@ -25,6 +25,8 @@ type StepContext struct { Config Config // Project is the Project that the Promotion is associated with. Project string + // Stage is the Stage that the Promotion is targeting. + Stage string // FreightRequests is the list of Freight from various origins that is // requested by the Stage targeted by the Promotion. This information is // sometimes useful to Steps that reference a particular artifact and, in the @@ -122,10 +124,14 @@ func (c Config) DeepCopy() Config { type Status string const ( - // StatusSuccess is the result of a successful directive execution. - StatusSuccess Status = "Success" // StatusFailure is the result of a failed directive execution. StatusFailure Status = "Failure" + // StatusPending is the result of a directive execution that is waiting on + // some external state (such as waiting for an open PR to be merged or + // closed). + StatusPending Status = "Pending" + // StatusSuccess is the result of a successful directive execution. + StatusSuccess Status = "Success" ) // Result represents the outcome of a directive execution, including its status diff --git a/internal/directives/git_clone_directive.go b/internal/directives/git_clone_directive.go index b8d29a9190..1a990ce7b2 100644 --- a/internal/directives/git_clone_directive.go +++ b/internal/directives/git_clone_directive.go @@ -3,8 +3,15 @@ package directives import ( "context" "fmt" + "os" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/xeipuuv/gojsonschema" + + kargoapi "github.com/akuity/kargo/api/v1alpha1" + "github.com/akuity/kargo/internal/controller/freight" + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" ) func init() { @@ -26,9 +33,9 @@ type gitCloneDirective struct { // newGitCloneDirective creates a new git-clone directive. func newGitCloneDirective() Directive { - return &gitCloneDirective{ - schemaLoader: getConfigSchemaLoader("git-clone"), - } + d := &gitCloneDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d } // Name implements the Directive interface. @@ -38,22 +45,163 @@ func (g *gitCloneDirective) Name() string { // Run implements the Directive interface. func (g *gitCloneDirective) Run( - _ context.Context, + ctx context.Context, + stepCtx *StepContext, +) (Result, error) { + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err + } + cfg, err := configToStruct[GitCloneConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("could not convert config into %s config: %w", g.Name(), err) + } + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-clone directive configuration against the JSON +// schema. +func (g *gitCloneDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitCloneDirective) run( + ctx context.Context, stepCtx *StepContext, + cfg GitCloneConfig, ) (Result, error) { - failure := Result{Status: StatusFailure} - // Validate the configuration against the JSON Schema - if err := validate( - g.schemaLoader, - gojsonschema.NewGoLoader(stepCtx.Config), - "git-clone", + var repoCreds *git.RepoCredentials + if creds, found, err := stepCtx.CredentialsDB.Get( + ctx, + stepCtx.Project, + credentials.TypeGit, + cfg.RepoURL, ); err != nil { - return failure, err + return Result{Status: StatusFailure}, fmt.Errorf("error getting credentials for %s: %w", cfg.RepoURL, err) + } else if found { + repoCreds = &git.RepoCredentials{ + Username: creds.Username, + Password: creds.Password, + SSHPrivateKey: creds.SSHPrivateKey, + } + } + repo, err := git.CloneBare( + cfg.RepoURL, + &git.ClientOptions{ + Credentials: repoCreds, + InsecureSkipTLSVerify: cfg.InsecureSkipTLSVerify, + }, + &git.BareCloneOptions{ + BaseDir: stepCtx.WorkDir, + }, + ) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf("error cloning %s: %w", cfg.RepoURL, err) } - if _, err := configToStruct[GitCloneConfig](stepCtx.Config); err != nil { - return failure, - fmt.Errorf("could not convert config into git-clone config: %w", err) + for _, checkout := range cfg.Checkout { + var ref string + switch { + case checkout.Branch != "": + ref = checkout.Branch + if err = ensureRemoteBranch(repo, ref); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error ensuring existence of remote branch %s: %w", ref, err) + } + case checkout.FromFreight: + var desiredOrigin *kargoapi.FreightOrigin + if checkout.FromOrigin == nil { + desiredOrigin = &kargoapi.FreightOrigin{ + Kind: kargoapi.FreightOriginKind(checkout.FromOrigin.Kind), + } + } + var commit *kargoapi.GitCommit + if commit, err = freight.FindCommit( + ctx, + stepCtx.KargoClient, + stepCtx.Project, + stepCtx.FreightRequests, + desiredOrigin, + stepCtx.Freight.References(), + cfg.RepoURL, + ); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error finding commit from repo %s: %w", cfg.RepoURL, err) + } + ref = commit.ID + case checkout.Tag != "": + ref = checkout.Tag + } + path, err := securejoin.SecureJoin(stepCtx.WorkDir, checkout.Path) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + checkout.Path, stepCtx.WorkDir, err, + ) + } + if _, err = repo.AddWorkTree( + path, + &git.AddWorkTreeOptions{Ref: ref}, + ); err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error adding work tree %s to repo %s: %w", + checkout.Path, cfg.RepoURL, err, + ) + } } - // TODO: Add implementation here + // Note: We do NOT defer repo.Close() because we want to keep the repository + // around on the FS for subsequent directives to use. The directive execution + // engine will handle all work dir cleanup. return Result{Status: StatusSuccess}, nil } + +// ensureRemoteBranch ensures the existence of a remote branch. If the branch +// does not exist, an empty orphaned branch is created and pushed to the remote. +func ensureRemoteBranch(repo git.BareRepo, branch string) error { + exists, err := repo.RemoteBranchExists(branch) + if err != nil { + return fmt.Errorf( + "error checking if remote branch %q of repo %s exists: %w", + branch, repo.URL(), err, + ) + } + if exists { + return nil + } + tmpDir, err := os.MkdirTemp("", "repo-") + if err != nil { + return fmt.Errorf("error creating temporary directory: %w", err) + } + workTree, err := repo.AddWorkTree(tmpDir, &git.AddWorkTreeOptions{Orphan: true}) + if err != nil { + return fmt.Errorf( + "error adding temporary working tree for branch %q of repo %s: %w", + branch, repo.URL(), err, + ) + } + defer workTree.Close() + // `git worktree add --orphan some/path` (i.e. the preceding + // repo.AddWorkTree() call) creates a new orphaned branch named "path". We + // have no control over the branch name. It will always be equal to the last + // component of the path. So, we will immediately create _another_ orphaned + // branch with the name we really wanted before making an initial commit and + // pushing it to the remote. + if err = workTree.CreateOrphanedBranch(branch); err != nil { + return err + } + if err = workTree.Commit( + "Initial commit", + &git.CommitOptions{AllowEmpty: true}, + ); err != nil { + return fmt.Errorf( + "error making initial commit to new branch %q of repo %s: %w", + branch, repo.URL(), err, + ) + } + if err = workTree.Push(&git.PushOptions{TargetBranch: branch}); err != nil { + return fmt.Errorf( + "error pushing initial commit to new branch %q to repo %s: %w", + branch, repo.URL(), err, + ) + } + return nil +} diff --git a/internal/directives/git_clone_directive_test.go b/internal/directives/git_clone_directive_test.go index 90cb97d5f6..7dd174ef17 100644 --- a/internal/directives/git_clone_directive_test.go +++ b/internal/directives/git_clone_directive_test.go @@ -2,265 +2,340 @@ package directives import ( "context" + "fmt" + "net/http/httptest" + "os" + "path/filepath" "testing" + "github.com/sosedoff/gitkit" "github.com/stretchr/testify/require" -) - -func TestGitCloneDirective_Run(t *testing.T) { - ctx := context.Background() - d := newGitCloneDirective() + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" +) - t.Run("validations", func(t *testing.T) { - testCases := []struct { - name string - config Config - expectedProblems []string - }{ - { - name: "repoURL not specified", - config: Config{}, - expectedProblems: []string{ - "(root): repoURL is required", - }, +func TestGitCloneDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "repoURL not specified", + config: Config{}, + expectedProblems: []string{ + "(root): repoURL is required", }, - { - name: "repoURL is empty string", - config: Config{ - "repoURL": "", - }, - expectedProblems: []string{ - "repoURL: String length must be greater than or equal to 1", - "repoURL: Does not match format 'uri'", - }, + }, + { + name: "repoURL is empty string", + config: Config{ + "repoURL": "", }, - { - name: "no checkout specified", - config: Config{}, - expectedProblems: []string{ - "(root): checkout is required", - }, + expectedProblems: []string{ + "repoURL: String length must be greater than or equal to 1", + "repoURL: Does not match format 'uri'", }, - { - name: "checkout is an empty array", - config: Config{ - "checkout": []Config{}, - }, - expectedProblems: []string{ - "checkout: Array must have at least 1 items", - }, + }, + { + name: "no checkout specified", + config: Config{}, + expectedProblems: []string{ + "(root): checkout is required", }, - { - name: "checkout path is not specified", - config: Config{ - "checkout": []Config{{}}, - }, - expectedProblems: []string{ - "checkout.0: path is required", - }, + }, + { + name: "checkout is an empty array", + config: Config{ + "checkout": []Config{}, }, - { - name: "checkout path is empty string", - config: Config{ - "checkout": []Config{{ - "path": "", - }}, - }, - expectedProblems: []string{ - "checkout.0.path: String length must be greater than or equal to 1", - }, + expectedProblems: []string{ + "checkout: Array must have at least 1 items", }, - { - name: "neither branch nor fromFreight nor tag specified", - // This is ok. The behavior should be to clone the default branch. - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "path": "/fake/path", - }}, - }, + }, + { + name: "checkout path is not specified", + config: Config{ + "checkout": []Config{{}}, }, - { - name: "branch is empty string, fromFreight is explicitly false, and tag is empty string", - // This is ok. The behavior should be to clone the default branch. - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "branch": "", - "fromFreight": false, - "tag": "", - "path": "/fake/path", - }}, - }, + expectedProblems: []string{ + "checkout.0: path is required", }, - { - name: "just branch is specified", - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "branch": "fake-branch", - "path": "/fake/path", - }}, - }, + }, + { + name: "checkout path is empty string", + config: Config{ + "checkout": []Config{{ + "path": "", + }}, }, - { - name: "branch is specified and fromFreight is true", - // These are meant to be mutually exclusive. - config: Config{ - "checkout": []Config{{ - "branch": "fake-branch", - "fromFreight": true, - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + expectedProblems: []string{ + "checkout.0.path: String length must be greater than or equal to 1", }, - { - name: "branch and fromOrigin are both specified", - // These are not meant to be used together. - config: Config{ - "checkout": []Config{{ - "branch": "fake-branch", - "fromOrigin": Config{}, - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + }, + { + name: "neither branch nor fromFreight nor tag specified", + // This is ok. The behavior should be to clone the default branch. + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "path": "/fake/path", + }}, }, - { - name: "branch and tag are both specified", - // These are meant to be mutually exclusive. - config: Config{ - "checkout": []Config{{ - "branch": "fake-branch", - "tag": "fake-tag", - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + }, + { + name: "branch is empty string, fromFreight is explicitly false, and tag is empty string", + // This is ok. The behavior should be to clone the default branch. + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "branch": "", + "fromFreight": false, + "tag": "", + "path": "/fake/path", + }}, }, - { - name: "just fromFreight is true", - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "fromFreight": true, - "path": "/fake/path", - }}, - }, + }, + { + name: "just branch is specified", + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "branch": "fake-branch", + "path": "/fake/path", + }}, }, - { - name: "fromFreight is true and fromOrigin is specified", - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "fromFreight": true, - "fromOrigin": Config{ - "kind": "Warehouse", - "name": "fake-warehouse", - }, - "path": "/fake/path", - }}, - }, + }, + { + name: "branch is specified and fromFreight is true", + // These are meant to be mutually exclusive. + config: Config{ + "checkout": []Config{{ + "branch": "fake-branch", + "fromFreight": true, + }}, }, - { - name: "fromFreight is true and tag is specified", - // These are meant to be mutually exclusive. - config: Config{ - "checkout": []Config{{ - "fromFreight": true, - "tag": "fake-tag", - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", }, - { - name: "just fromOrigin is specified", - // This is not meant to be used without fromFreight=true. - config: Config{ - "checkout": []Config{{ - "fromOrigin": Config{}, - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + }, + { + name: "branch and fromOrigin are both specified", + // These are not meant to be used together. + config: Config{ + "checkout": []Config{{ + "branch": "fake-branch", + "fromOrigin": Config{}, + }}, }, - { - name: "fromOrigin and tag are both specified", - // These are not meant to be used together. - config: Config{ - "checkout": []Config{{ - "fromOrigin": Config{}, - "tag": "fake-tag", - }}, - }, - expectedProblems: []string{ - "checkout.0: Must validate one and only one schema", - }, + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", }, - { - name: "just tag is specified", - config: Config{ // Should be completely valid - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{{ - "tag": "fake-tag", - "path": "/fake/path", - }}, - }, + }, + { + name: "branch and tag are both specified", + // These are meant to be mutually exclusive. + config: Config{ + "checkout": []Config{{ + "branch": "fake-branch", + "tag": "fake-tag", + }}, }, - { - name: "valid kitchen sink", - config: Config{ - "repoURL": "https://github.com/example/repo.git", - "checkout": []Config{ - { - "path": "/fake/path/0", - }, - { - "branch": "fake-branch", - "path": "/fake/path/1", - }, - { - "fromFreight": true, - "path": "/fake/path/2", - }, - { - "fromFreight": true, - "fromOrigin": Config{ - "kind": "Warehouse", - "name": "fake-warehouse", - }, - "path": "/fake/path/3", - }, - { - "tag": "fake-tag", - "path": "/fake/path/4", + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", + }, + }, + { + name: "just fromFreight is true", + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "fromFreight": true, + "path": "/fake/path", + }}, + }, + }, + { + name: "fromFreight is true and fromOrigin is specified", + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "fromFreight": true, + "fromOrigin": Config{ + "kind": "Warehouse", + "name": "fake-warehouse", + }, + "path": "/fake/path", + }}, + }, + }, + { + name: "fromFreight is true and tag is specified", + // These are meant to be mutually exclusive. + config: Config{ + "checkout": []Config{{ + "fromFreight": true, + "tag": "fake-tag", + }}, + }, + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", + }, + }, + { + name: "just fromOrigin is specified", + // This is not meant to be used without fromFreight=true. + config: Config{ + "checkout": []Config{{ + "fromOrigin": Config{}, + }}, + }, + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", + }, + }, + { + name: "fromOrigin and tag are both specified", + // These are not meant to be used together. + config: Config{ + "checkout": []Config{{ + "fromOrigin": Config{}, + "tag": "fake-tag", + }}, + }, + expectedProblems: []string{ + "checkout.0: Must validate one and only one schema", + }, + }, + { + name: "just tag is specified", + config: Config{ // Should be completely valid + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{{ + "tag": "fake-tag", + "path": "/fake/path", + }}, + }, + }, + { + name: "valid kitchen sink", + config: Config{ + "repoURL": "https://github.com/example/repo.git", + "checkout": []Config{ + { + "path": "/fake/path/0", + }, + { + "branch": "fake-branch", + "path": "/fake/path/1", + }, + { + "fromFreight": true, + "path": "/fake/path/2", + }, + { + "fromFreight": true, + "fromOrigin": Config{ + "kind": "Warehouse", + "name": "fake-warehouse", }, + "path": "/fake/path/3", + }, + { + "tag": "fake-tag", + "path": "/fake/path/4", }, }, }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - stepCtx := &StepContext{ - Config: testCase.config, - } - _, err := d.Run(ctx, stepCtx) - if len(testCase.expectedProblems) == 0 { - require.NoError(t, err) - } else { - for _, problem := range testCase.expectedProblems { - require.ErrorContains(t, err, problem) - } + }, + } + + d := newGitCloneDirective() + dir, ok := d.(*gitCloneDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) } - }) - } - }) + } + }) + } +} + +func TestGitCloneDirective_Run(t *testing.T) { + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // This is the URL of the "remote" repository + testRepoURL := fmt.Sprintf("%s/test.git", server.URL) + + // Create some content and push it to the remote repository's default branch + repo, err := git.Clone(testRepoURL, nil, nil) + require.NoError(t, err) + defer repo.Close() + err = os.WriteFile(filepath.Join(repo.Dir(), "test.txt"), []byte("foo"), 0600) + require.NoError(t, err) + err = repo.AddAllAndCommit("Initial commit") + require.NoError(t, err) + err = repo.Push(nil) + require.NoError(t, err) + + // Now we can proceed to test the git-clone directive... + + d := newGitCloneDirective() + dir, ok := d.(*gitCloneDirective) + require.True(t, ok) + + stepCtx := &StepContext{ + CredentialsDB: &credentials.FakeDB{}, + WorkDir: t.TempDir(), + } + + res, err := dir.run( + context.Background(), + stepCtx, + GitCloneConfig{ + RepoURL: fmt.Sprintf("%s/test.git", server.URL), + Checkout: []Checkout{ + { + // "master" is still the default branch name for a new repository + // unless you configure it otherwise. + Branch: "master", + Path: "master", + }, + { + Branch: "stage/dev", + Path: "dev", + }, + }, + }, + ) + require.NoError(t, err) + require.Equal(t, StatusSuccess, res.Status) + require.DirExists(t, filepath.Join(stepCtx.WorkDir, "master")) + // The checked out master branch should have the content we know is in the + // test remote's master branch. + require.FileExists(t, filepath.Join(stepCtx.WorkDir, "master", "test.txt")) + require.DirExists(t, filepath.Join(stepCtx.WorkDir, "dev")) + // The stage/dev branch is a new orphan branch with a single empty commit. + // It should lack any content. + dirEntries, err := os.ReadDir(filepath.Join(stepCtx.WorkDir, "dev")) + require.NoError(t, err) + require.Len(t, dirEntries, 1) // Just the .git file + require.FileExists(t, filepath.Join(stepCtx.WorkDir, "dev", ".git")) } diff --git a/internal/directives/git_commit_directive.go b/internal/directives/git_commit_directive.go index e4b7d2f7da..2ea439e824 100644 --- a/internal/directives/git_commit_directive.go +++ b/internal/directives/git_commit_directive.go @@ -4,7 +4,10 @@ import ( "context" "fmt" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/controller/git" ) func init() { @@ -20,9 +23,9 @@ type gitCommitDirective struct { // newGitCommitDirective creates a new git-commit directive. func newGitCommitDirective() Directive { - return &gitCommitDirective{ - schemaLoader: getConfigSchemaLoader("git-commit"), - } + d := &gitCommitDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d } // Name implements the Directive interface. @@ -32,22 +35,116 @@ func (g *gitCommitDirective) Name() string { // Run implements the Directive interface. func (g *gitCommitDirective) Run( + ctx context.Context, + stepCtx *StepContext, +) (Result, error) { + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err + } + cfg, err := configToStruct[GitCommitConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("could not convert config into %s config: %w", g.Name(), err) + } + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-commit directive configuration against the JSON +// schema. +func (g *gitCommitDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitCommitDirective) run( _ context.Context, stepCtx *StepContext, + cfg GitCommitConfig, ) (Result, error) { - failure := Result{Status: StatusFailure} - // Validate the configuration against the JSON Schema - if err := validate( - g.schemaLoader, - gojsonschema.NewGoLoader(stepCtx.Config), - "git-commit", - ); err != nil { - return failure, err - } - if _, err := configToStruct[GitCommitConfig](stepCtx.Config); err != nil { - return failure, - fmt.Errorf("could not convert config into git-commit config: %w", err) - } - // TODO: Add implementation here + path, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.Path) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + cfg.Path, stepCtx.WorkDir, err, + ) + } + workTree, err := git.LoadWorkTree(path, nil) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error loading working tree from %s: %w", cfg.Path, err) + } + if err = workTree.AddAll(); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error adding all changes to working tree: %w", err) + } + commitMsg, err := buildCommitMessage(stepCtx.SharedState, cfg) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error building commit message: %w", err) + } + commitOpts := &git.CommitOptions{} + if cfg.Author != nil { + commitOpts.Author = &git.User{} + if cfg.Author.Name != "" { + commitOpts.Author.Name = cfg.Author.Name + } + if cfg.Author.Email != "" { + commitOpts.Author.Email = cfg.Author.Email + } + } + if err = workTree.Commit(commitMsg, commitOpts); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error committing to working tree: %w", err) + } return Result{Status: StatusSuccess}, nil } + +func buildCommitMessage(sharedState State, cfg GitCommitConfig) (string, error) { + var commitMsg string + if cfg.Message != "" { + commitMsg = cfg.Message + } else if len(cfg.MessageFrom) > 0 { + commitMsgParts := make([]string, len(cfg.MessageFrom)) + for i, alias := range cfg.MessageFrom { + stepOutput, exists := sharedState.Get(alias) + if !exists { + return "", fmt.Errorf( + "no output found from step with alias %q; cannot construct commit "+ + "message", + alias, + ) + } + stepOutputState, ok := stepOutput.(State) + if !ok { + return "", fmt.Errorf( + "output from step with alias %q is not a State; cannot construct "+ + "commit message", + alias, + ) + } + commitMsgPart, exists := stepOutputState.Get("commitMessage") + if !exists { + return "", fmt.Errorf( + "no commit message found in output from step with alias %q; cannot "+ + "construct commit message", + alias, + ) + } + if commitMsgParts[i], ok = commitMsgPart.(string); !ok { + return "", fmt.Errorf( + "commit message in output from step with alias %q is not a string; "+ + "cannot construct commit message", + alias, + ) + } + } + if len(commitMsgParts) == 1 { + commitMsg = commitMsgParts[0] + } else { + commitMsg = "Kargo applied multiple changes\n\nIncluding:\n" + for _, commitMsgPart := range commitMsgParts { + commitMsg = fmt.Sprintf("%s\n * %s", commitMsg, commitMsgPart) + } + } + } + return commitMsg, nil +} diff --git a/internal/directives/git_commit_directive_test.go b/internal/directives/git_commit_directive_test.go index ba7d9eab7d..366a7e7ce1 100644 --- a/internal/directives/git_commit_directive_test.go +++ b/internal/directives/git_commit_directive_test.go @@ -2,95 +2,320 @@ package directives import ( "context" + "fmt" + "net/http/httptest" + "os" + "path/filepath" "testing" + "github.com/sosedoff/gitkit" "github.com/stretchr/testify/require" + + "github.com/akuity/kargo/internal/controller/git" ) +func TestGitCommitDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "path not specified", + config: Config{}, + expectedProblems: []string{ + "(root): path is required", + }, + }, + { + name: "path is empty string", + config: Config{ + "path": "", + }, + expectedProblems: []string{ + "path: String length must be greater than or equal to 1", + }, + }, + { + name: "neither message nor messageFrom is specified", + config: Config{}, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "both message and messageFrom are specified", + config: Config{ + "message": "fake commit message", + "messageFrom": []string{"fake-step-alias"}, + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "message is empty string", + config: Config{ + "message": "", + }, + expectedProblems: []string{ + "message: String length must be greater than or equal to 1", + }, + }, + { + name: "messageFrom is empty array", + config: Config{ + "messageFrom": []string{}, + }, + expectedProblems: []string{ + "messageFrom: Array must have at least 1 items", + }, + }, + { + name: "messageFrom array contains an empty string", + config: Config{ + "messageFrom": []string{""}, + }, + expectedProblems: []string{ + "messageFrom.0: String length must be greater than or equal to 1", + }, + }, + { + name: "author is not specified", + config: Config{ // Should be completely valid + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + { + name: "author email is not specified", + config: Config{ // Should be completely valid + "author": Config{}, + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + { + name: "author email is empty string", + config: Config{ // Should be completely valid + "author": Config{ + "email": "", + }, + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + { + name: "author name is not specified", + config: Config{ // Should be completely valid + "author": Config{}, + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + { + name: "author name is empty string", + config: Config{ // Should be completely valid + "author": Config{ + "name": "", + }, + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + { + name: "valid kitchen sink", + config: Config{ + "author": Config{ + "email": "tony@starkindustries.com", + "name": "Tony Stark", + }, + "path": "/tmp/foo", + "message": "fake commit message", + }, + }, + } + + d := newGitCommitDirective() + dir, ok := d.(*gitCommitDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) + } + } + }) + } +} + func TestGitCommitDirective_Run(t *testing.T) { - ctx := context.Background() + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // This is the URL of the "remote" repository + testRepoURL := fmt.Sprintf("%s/test.git", server.URL) + + workDir := t.TempDir() + + // Finagle a local bare repo and working tree into place the way that the + // git-clone directive might have so we can verify the git-commit directive's + // ability to reload the working tree from the file system. + repo, err := git.CloneBare( + testRepoURL, + nil, + &git.BareCloneOptions{ + BaseDir: workDir, + }, + ) + require.NoError(t, err) + defer repo.Close() + // "master" is still the default branch name for a new repository + // unless you configure it otherwise. + workTreePath := filepath.Join(workDir, "master") + workTree, err := repo.AddWorkTree( + workTreePath, + &git.AddWorkTreeOptions{Orphan: true}, + ) + require.NoError(t, err) + // `git worktree add` doesn't give much control over the branch name when you + // create an orphaned working tree, so we have to follow up with this to make + // the branch name look like what we wanted. The git-clone directive does + // this internally as well. + err = workTree.CreateOrphanedBranch("master") + require.NoError(t, err) + + // Write a file. It will be the git-commit directive's job to commit it. + err = os.WriteFile(filepath.Join(workTree.Dir(), "test.txt"), []byte("foo"), 0600) + require.NoError(t, err) + + // Now we can proceed to test the git-commit directive... d := newGitCommitDirective() + dir, ok := d.(*gitCommitDirective) + require.True(t, ok) - t.Run("validations", func(t *testing.T) { - testCases := []struct { - name string - config Config - expectedProblems []string - }{ - { - name: "path not specified", - config: Config{}, - expectedProblems: []string{ - "(root): path is required", - }, + stepCtx := &StepContext{ + WorkDir: workDir, + } + + res, err := dir.run( + context.Background(), + stepCtx, + GitCommitConfig{ + Path: "master", + Message: "Initial commit", + }, + ) + require.NoError(t, err) + require.Equal(t, StatusSuccess, res.Status) + lastCommitMsg, err := workTree.CommitMessage("HEAD") + require.NoError(t, err) + require.Equal(t, "Initial commit", lastCommitMsg) +} + +func TestBuildCommitMessage(t *testing.T) { + testCases := []struct { + name string + sharedState State + cfg GitCommitConfig + assertions func(t *testing.T, msg string, err error) + }{ + { + name: "message is specified", + cfg: GitCommitConfig{Message: "fake commit message"}, + assertions: func(t *testing.T, msg string, err error) { + require.NoError(t, err) + require.Equal(t, "fake commit message", msg) }, - { - name: "path is empty string", - config: Config{ - "path": "", - }, - expectedProblems: []string{ - "path: String length must be greater than or equal to 1", - }, + }, + { + name: "no output from step with alias", + sharedState: State{}, + cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}}, + assertions: func(t *testing.T, _ string, err error) { + require.ErrorContains(t, err, "no output found from step with alias") }, - { - name: "author email is not specified", - config: Config{ // Should be completely valid - "author": Config{}, - "path": "/tmp/foo", - }, + }, + { + name: "unexpected value type from step with alias", + sharedState: State{ + "fake-step-alias": "not a State", }, - { - name: "author email is empty string", - config: Config{ // Should be completely valid - "author": Config{ - "email": "", - }, - "path": "/tmp/foo", - }, + cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}}, + assertions: func(t *testing.T, _ string, err error) { + require.ErrorContains(t, err, "output from step with alias") + require.ErrorContains(t, err, "is not a State") + }, + }, + { + name: "output from step with alias does not contain a commit message", + sharedState: State{ + "fake-step-alias": State{}, }, - { - name: "author name is not specified", - config: Config{ // Should be completely valid - "author": Config{}, - "path": "/tmp/foo", + cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}}, + assertions: func(t *testing.T, _ string, err error) { + require.ErrorContains( + t, err, "no commit message found in output from step with alias", + ) + }, + }, + { + name: "output from step with alias contain a commit message that isn't a string", + sharedState: State{ + "fake-step-alias": State{ + "commitMessage": 42, }, }, - { - name: "author name is empty string", - config: Config{ // Should be completely valid - "author": Config{ - "name": "", - }, - "path": "/tmp/foo", + cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}}, + assertions: func(t *testing.T, _ string, err error) { + require.ErrorContains( + t, err, "commit message in output from step with alias", + ) + require.ErrorContains(t, err, "is not a string") + }, + }, + { + name: "successful message construction", + sharedState: State{ + "fake-step-alias": State{ + "commitMessage": "part one", + }, + "another-fake-step-alias": State{ + "commitMessage": "part two", }, }, - { - name: "valid kitchen sink", - config: Config{ - "author": Config{ - "email": "tony@starkindustries.com", - "name": "Tony Stark", - }, - "path": "/tmp/foo", + cfg: GitCommitConfig{ + MessageFrom: []string{ + "fake-step-alias", + "another-fake-step-alias", }, }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - stepCtx := &StepContext{ - Config: testCase.config, - } - _, err := d.Run(ctx, stepCtx) - if len(testCase.expectedProblems) == 0 { - require.NoError(t, err) - } else { - for _, problem := range testCase.expectedProblems { - require.ErrorContains(t, err, problem) - } - } - }) - } - }) + assertions: func(t *testing.T, msg string, err error) { + require.NoError(t, err) + require.Contains(t, msg, "Kargo applied multiple changes") + require.Contains(t, msg, "part one") + require.Contains(t, msg, "part two") + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + commitMsg, err := buildCommitMessage(testCase.sharedState, testCase.cfg) + testCase.assertions(t, commitMsg, err) + }) + } } diff --git a/internal/directives/git_open_pr_directive.go b/internal/directives/git_open_pr_directive.go new file mode 100644 index 0000000000..2ef87c6890 --- /dev/null +++ b/internal/directives/git_open_pr_directive.go @@ -0,0 +1,258 @@ +package directives + +import ( + "context" + "fmt" + + "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" + "github.com/akuity/kargo/internal/gitprovider" +) + +const prNumberKey = "prNumber" + +func init() { + // Register the git-open-pr directive with the builtins registry. + builtins.RegisterDirective( + newGitOpenPRDirective(), + &DirectivePermissions{AllowCredentialsDB: true}, + ) +} + +// gitOpenPRDirective is a directive that opens a pull request. +type gitOpenPRDirective struct { + schemaLoader gojsonschema.JSONLoader +} + +// newGitOpenPRDirective creates a new git-open-pr directive. +func newGitOpenPRDirective() Directive { + d := &gitOpenPRDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d +} + +// Name implements the Directive interface. +func (g *gitOpenPRDirective) Name() string { + return "git-open-pr" +} + +// Run implements the Directive interface. +func (g *gitOpenPRDirective) Run( + ctx context.Context, + stepCtx *StepContext, +) (Result, error) { + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err + } + cfg, err := configToStruct[GitOpenPRConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("could not convert config into git-open-pr config: %w", err) + } + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-open-pr directive configuration against the JSON +// schema. +func (g *gitOpenPRDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitOpenPRDirective) run( + ctx context.Context, + stepCtx *StepContext, + cfg GitOpenPRConfig, +) (Result, error) { + sourceBranch, err := getSourceBranch(stepCtx.SharedState, cfg) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error determining source branch: %w", err) + } + + var repoCreds *git.RepoCredentials + creds, found, err := stepCtx.CredentialsDB.Get( + ctx, + stepCtx.Project, + credentials.TypeGit, + cfg.RepoURL, + ) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf("error getting credentials for %s: %w", cfg.RepoURL, err) + } + if found { + repoCreds = &git.RepoCredentials{ + Username: creds.Username, + Password: creds.Password, + SSHPrivateKey: creds.SSHPrivateKey, + } + } + + // Note: Strictly speaking, you don't need to clone a repo to check if a + // remote branch exists, but our options for authenticating to a remote + // repository are really only applied when cloning. + // + // We could have the user provide the path to a working tree, load it, and use + // that to check for the existence of the remote branch, but that feels + // limiting, as it would only enable workflows wherein the user HAS a working + // tree at their disposal. It is theoretically possible that the PR the user + // is opening isn't related to anything that occurred in a previous step. i.e. + // We cannot assume that they have already cloned the repository and have a + // working tree. Beyond this, if we detect the need to CREATE and push the + // remote branch, it is preferable that we do not mess with the state of the + // user's working tree in the process of doing so. + // + // For all the reasons above, we ask the user to provide a URL instead of a + // path and we perform a shallow clone to check for the existence of the + // remote branch. + repo, err := git.Clone( + cfg.RepoURL, + &git.ClientOptions{ + Credentials: repoCreds, + InsecureSkipTLSVerify: cfg.InsecureSkipTLSVerify, + }, + &git.CloneOptions{ + Depth: 1, + Branch: sourceBranch, + }, + ) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error cloning %s: %w", cfg.RepoURL, err) + } + defer repo.Close() + + // Get the title from the commit message of the head of the source branch + // BEFORE we move on to ensuring the existence of the target branch because + // that may involve creating a new branch and committing to it. + title, err := repo.CommitMessage(sourceBranch) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error getting commit message from head of branch %s: %w", + sourceBranch, err, + ) + } + + if err = ensureRemoteTargetBranch( + repo, + cfg.TargetBranch, + cfg.CreateTargetBranch, + ); err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error ensuring existence of remote branch %s: %w", + cfg.TargetBranch, err, + ) + } + + gpOpts := &gitprovider.GitProviderOptions{ + InsecureSkipTLSVerify: cfg.InsecureSkipTLSVerify, + } + if repoCreds != nil { + gpOpts.Token = repoCreds.Password + } + if cfg.Provider != nil { + gpOpts.Name = string(*cfg.Provider) + } + gitProviderSvc, err := gitprovider.NewGitProviderService(cfg.RepoURL, gpOpts) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error creating git provider service: %w", err) + } + + pr, err := gitProviderSvc.CreatePullRequest( + ctx, + gitprovider.CreatePullRequestOpts{ + Head: cfg.SourceBranch, + Base: cfg.TargetBranch, + Title: title, + }, + ) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error creating pull request: %w", err) + } + return Result{ + Status: StatusSuccess, + Output: State{ + prNumberKey: pr.Number, + }, + }, nil +} + +func getSourceBranch(sharedState State, cfg GitOpenPRConfig) (string, error) { + sourceBranch := cfg.SourceBranch + if cfg.SourceBranchFromPush != "" { + stepOutput, exists := sharedState.Get(cfg.SourceBranchFromPush) + if !exists { + return "", fmt.Errorf( + "no output found from step with alias %q", + cfg.SourceBranchFromPush, + ) + } + stepOutputState, ok := stepOutput.(State) + if !ok { + return "", fmt.Errorf( + "output from step with alias %q is not a State", + cfg.SourceBranchFromPush, + ) + } + sourceBranchAny, exists := stepOutputState.Get(branchKey) + if !exists { + return "", fmt.Errorf( + "no branch found in output from step with alias %q", + cfg.SourceBranchFromPush, + ) + } + if sourceBranch, ok = sourceBranchAny.(string); !ok { + return "", fmt.Errorf( + "branch name in output from step with alias %q is not a string", + cfg.SourceBranchFromPush, + ) + } + } + return sourceBranch, nil +} + +// ensureRemoteTargetBranch ensures the existence of a remote branch. If the +// branch does not exist, an empty orphaned branch is created and pushed to the +// remote. +func ensureRemoteTargetBranch(repo git.Repo, branch string, create bool) error { + exists, err := repo.RemoteBranchExists(branch) + if err != nil { + return fmt.Errorf( + "error checking if remote branch %q of repo %s exists: %w", + branch, repo.URL(), err, + ) + } + if exists { + return nil + } + if !create { + return fmt.Errorf( + "remote branch %q does not exist in repo %s", branch, repo.URL(), + ) + } + if err = repo.CreateOrphanedBranch(branch); err != nil { + return fmt.Errorf( + "error creating orphaned branch %q in repo %s: %w", + branch, repo.URL(), err, + ) + } + if err = repo.Commit( + "Initial commit", + &git.CommitOptions{AllowEmpty: true}, + ); err != nil { + return fmt.Errorf( + "error making initial commit to new branch %q of repo %s: %w", + branch, repo.URL(), err, + ) + } + if err = repo.Push(&git.PushOptions{TargetBranch: branch}); err != nil { + return fmt.Errorf( + "error pushing initial commit to new branch %q to repo %s: %w", + branch, repo.URL(), err, + ) + } + return nil +} diff --git a/internal/directives/git_open_pr_directive_test.go b/internal/directives/git_open_pr_directive_test.go new file mode 100644 index 0000000000..eedf5441e0 --- /dev/null +++ b/internal/directives/git_open_pr_directive_test.go @@ -0,0 +1,217 @@ +package directives + +import ( + "context" + "fmt" + "net/http/httptest" + "testing" + + "github.com/sosedoff/gitkit" + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" + "github.com/akuity/kargo/internal/gitprovider" +) + +func TestGitOpenPRDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "repoURL not specified", + config: Config{}, + expectedProblems: []string{ + "(root): repoURL is required", + }, + }, + { + name: "repoURL is empty string", + config: Config{ + "repoURL": "", + }, + expectedProblems: []string{ + "repoURL: String length must be greater than or equal to 1", + }, + }, + { + name: "targetBranch not specified", + config: Config{}, + expectedProblems: []string{ + "(root): targetBranch is required", + }, + }, + { + name: "targetBranch is empty string", + config: Config{ + "targetBranch": "", + }, + expectedProblems: []string{ + "targetBranch: String length must be greater than or equal to 1", + }, + }, + { + name: "neither sourceBranch nor sourceBranchFromPush specified", + config: Config{}, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "both sourceBranch and sourceBranchFromPush specified", + config: Config{ + "sourceBranch": "main", + "sourceBranchFromPush": "push", + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "provider is an invalid value", + config: Config{ + "provider": "bogus", + }, + expectedProblems: []string{ + "provider: provider must be one of the following:", + }, + }, + { + name: "valid without explicit provider", + config: Config{ + "repoURL": "https://github.com/example/repo.git", + "sourceBranch": "fake-branch", + "targetBranch": "another-fake-branch", + }, + }, + { + name: "valid with explicit provider", + config: Config{ + "provider": "github", + "repoURL": "https://github.com/example/repo.git", + "sourceBranch": "fake-branch", + "targetBranch": "another-fake-branch", + }, + }, + { + name: "valid with source branch from push", + config: Config{ + "provider": "github", + "repoURL": "https://github.com/example/repo.git", + "sourceBranchFromPush": "fake-step", + "targetBranch": "fake-branch", + }, + }, + } + + d := newGitOpenPRDirective() + dir, ok := d.(*gitOpenPRDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) + } + } + }) + } +} + +func TestGitOpenPRDirective_Run(t *testing.T) { + const testSourceBranch = "source" + const testTargetBranch = "target" + + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // This is the URL of the "remote" repository + testRepoURL := fmt.Sprintf("%s/test.git", server.URL) + + workDir := t.TempDir() + + repo, err := git.Clone(testRepoURL, nil, nil) + require.NoError(t, err) + defer repo.Close() + err = repo.CreateOrphanedBranch(testSourceBranch) + require.NoError(t, err) + err = repo.Commit("Initial commit", &git.CommitOptions{AllowEmpty: true}) + require.NoError(t, err) + err = repo.Push(nil) + require.NoError(t, err) + + // Set up a fake git provider + const fakeGitProviderName = "fake" + const testPRNumber int64 = 42 + gitprovider.RegisterProvider( + fakeGitProviderName, + gitprovider.ProviderRegistration{ + NewService: func( + string, + *gitprovider.GitProviderOptions, + ) (gitprovider.GitProviderService, error) { + return &gitprovider.FakeGitProviderService{ + CreatePullRequestFn: func( + context.Context, + gitprovider.CreatePullRequestOpts, + ) (*gitprovider.PullRequest, error) { + return &gitprovider.PullRequest{Number: testPRNumber}, nil + }, + }, nil + }, + }, + ) + + // Now we can proceed to test the git-open-pr directive... + + d := newGitOpenPRDirective() + dir, ok := d.(*gitOpenPRDirective) + require.True(t, ok) + + res, err := dir.run( + context.Background(), + &StepContext{ + Project: "fake-project", + Stage: "fake-stage", + WorkDir: workDir, + CredentialsDB: &credentials.FakeDB{}, + SharedState: State{ + "fake-step": State{ + branchKey: testSourceBranch, + }, + }, + }, + GitOpenPRConfig{ + RepoURL: testRepoURL, + // We get slightly better coverage by using this option + SourceBranchFromPush: "fake-step", + TargetBranch: testTargetBranch, + CreateTargetBranch: true, + Provider: ptr.To(Provider(fakeGitProviderName)), + }, + ) + require.NoError(t, err) + prNumber, ok := res.Output.Get(prNumberKey) + require.True(t, ok) + require.Equal(t, testPRNumber, prNumber) + + // Assert that the target branch, which didn't already exist, was created + exists, err := repo.RemoteBranchExists(testTargetBranch) + require.NoError(t, err) + require.True(t, exists) +} diff --git a/internal/directives/git_overwrite_directive.go b/internal/directives/git_overwrite_directive.go new file mode 100644 index 0000000000..a142f150d4 --- /dev/null +++ b/internal/directives/git_overwrite_directive.go @@ -0,0 +1,116 @@ +package directives + +import ( + "context" + "fmt" + "os" + + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/otiai10/copy" + "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/logging" +) + +func init() { + // Register the git-overwrite directive with the builtins registry. + builtins.RegisterDirective(newGitOverwriteDirective(), nil) +} + +// gitOverwriteDirective is a directive that overwrites the content of a Git +// working tree with the content from another directory. +type gitOverwriteDirective struct { + schemaLoader gojsonschema.JSONLoader +} + +// newGitOverwriteDirective creates a new git-overwrite directive. +func newGitOverwriteDirective() Directive { + d := &gitOverwriteDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d +} + +// Name implements the Directive interface. +func (g *gitOverwriteDirective) Name() string { + return "git-overwrite" +} + +// Run implements the Directive interface. +func (g *gitOverwriteDirective) Run( + ctx context.Context, + stepCtx *StepContext, +) (Result, error) { + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err + } + cfg, err := configToStruct[GitOverwriteConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("could not convert config into %s config: %w", g.Name(), err) + } + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-overwrite directive configuration against the JSON +// schema. +func (g *gitOverwriteDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitOverwriteDirective) run( + ctx context.Context, + stepCtx *StepContext, + cfg GitOverwriteConfig, +) (Result, error) { + inPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.InPath) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + cfg.InPath, stepCtx.WorkDir, err, + ) + } + outPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.OutPath) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + cfg.OutPath, stepCtx.WorkDir, err, + ) + } + workTree, err := git.LoadWorkTree(outPath, nil) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error loading working tree from %s: %w", cfg.OutPath, err) + } + // workTree.Clear() won't remove any files that aren't indexed. This is a bit + // of a hack to ensure that we don't have any untracked files in the working + // tree so that workTree.Clear() will remove everything. + if err = workTree.AddAll(); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error adding all files to working tree at %s: %w", cfg.OutPath, err) + } + if err = workTree.Clear(); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error clearing working tree at %s: %w", cfg.OutPath, err) + } + if err = copy.Copy( + inPath, + outPath, + copy.Options{ + Skip: func(srcFI os.FileInfo, _, _ string) (bool, error) { + return srcFI.IsDir() && srcFI.Name() == ".git", nil + }, + OnSymlink: func(src string) copy.SymlinkAction { + logging.LoggerFromContext(ctx).Trace("ignoring symlink", "src", src) + return copy.Skip + }, + OnError: func(_, _ string, err error) error { + return sanitizePathError(err, stepCtx.WorkDir) + }, + }, + ); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("failed to copy %q to %q: %w", cfg.InPath, cfg.OutPath, err) + } + return Result{Status: StatusSuccess}, nil +} diff --git a/internal/directives/git_overwrite_directive_test.go b/internal/directives/git_overwrite_directive_test.go new file mode 100644 index 0000000000..9b5809993e --- /dev/null +++ b/internal/directives/git_overwrite_directive_test.go @@ -0,0 +1,154 @@ +package directives + +import ( + "context" + "fmt" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/sosedoff/gitkit" + "github.com/stretchr/testify/require" + + "github.com/akuity/kargo/internal/controller/git" +) + +func TestGitOverwriteDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "inPath not specified", + config: Config{}, + expectedProblems: []string{ + "(root): inPath is required", + }, + }, + { + name: "inPath is empty string", + config: Config{ + "inPath": "", + }, + expectedProblems: []string{ + "inPath: String length must be greater than or equal to 1", + }, + }, + { + name: "outPath not specified", + config: Config{}, + expectedProblems: []string{ + "(root): outPath is required", + }, + }, + { + name: "outPath is empty string", + config: Config{ + "outPath": "", + }, + expectedProblems: []string{ + "outPath: String length must be greater than or equal to 1", + }, + }, + } + + d := newGitOverwriteDirective() + dir, ok := d.(*gitOverwriteDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) + } + } + }) + } +} + +func TestGitOverwriteDirective_Run(t *testing.T) { + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // This is the URL of the "remote" repository + testRepoURL := fmt.Sprintf("%s/test.git", server.URL) + + workDir := t.TempDir() + + // Finagle a local bare repo and working tree into place the way that the + // git-clone directive might have so we can verify the git-push directive's + // ability to reload the working tree from the file system. + repo, err := git.CloneBare( + testRepoURL, + nil, + &git.BareCloneOptions{ + BaseDir: workDir, + }, + ) + require.NoError(t, err) + defer repo.Close() + // "master" is still the default branch name for a new repository + // unless you configure it otherwise. + workTreePath := filepath.Join(workDir, "master") + workTree, err := repo.AddWorkTree( + workTreePath, + &git.AddWorkTreeOptions{Orphan: true}, + ) + require.NoError(t, err) + + // Write a file. Later, we will expect to see this has been deleted. + err = os.WriteFile(filepath.Join(workTree.Dir(), "original.txt"), []byte("foo"), 0600) + require.NoError(t, err) + + // Write another file to a different directory. This will be the source + // directory for the git-overwrite directive. + srcDir := filepath.Join(workDir, "src") + err = os.Mkdir(srcDir, 0700) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "new.txt"), []byte("bar"), 0600) + require.NoError(t, err) + + d := newGitOverwriteDirective() + dir, ok := d.(*gitOverwriteDirective) + require.True(t, ok) + + res, err := dir.run( + context.Background(), + &StepContext{ + Project: "fake-project", + Stage: "fake-stage", + WorkDir: workDir, + }, + GitOverwriteConfig{ + InPath: "src", + OutPath: "master", + }, + ) + require.NoError(t, err) + require.Equal(t, StatusSuccess, res.Status) + + // Make sure old files are gone + _, err = os.Stat(filepath.Join(workTree.Dir(), "original.txt")) + require.Error(t, err) + require.True(t, os.IsNotExist(err)) + // Make sure new files are present + _, err = os.Stat(filepath.Join(workTree.Dir(), "new.txt")) + require.NoError(t, err) + // Make sure the .git directory is still there + _, err = os.Stat(filepath.Join(workTree.Dir(), ".git")) + require.NoError(t, err) +} diff --git a/internal/directives/git_push_directive.go b/internal/directives/git_push_directive.go index 1115fed91d..c55eb267bf 100644 --- a/internal/directives/git_push_directive.go +++ b/internal/directives/git_push_directive.go @@ -4,12 +4,21 @@ import ( "context" "fmt" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" ) +const branchKey = "branch" + func init() { // Register the git-push directive with the builtins registry. - builtins.RegisterDirective(newGitPushDirective(), nil) + builtins.RegisterDirective( + newGitPushDirective(), + &DirectivePermissions{AllowCredentialsDB: true}, + ) } // gitPushDirective is a directive that pushes commits from a local Git @@ -20,9 +29,9 @@ type gitPushDirective struct { // newGitPushDirective creates a new git-push directive. func newGitPushDirective() Directive { - return &gitPushDirective{ - schemaLoader: getConfigSchemaLoader("git-push"), - } + d := &gitPushDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d } // Name implements the Directive interface. @@ -32,22 +41,95 @@ func (g *gitPushDirective) Name() string { // Run implements the Directive interface. func (g *gitPushDirective) Run( - _ context.Context, + ctx context.Context, stepCtx *StepContext, ) (Result, error) { - failure := Result{Status: StatusFailure} - // Validate the configuration against the JSON Schema - if err := validate( - g.schemaLoader, - gojsonschema.NewGoLoader(stepCtx.Config), - "git-push", - ); err != nil { - return failure, err + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err } - if _, err := configToStruct[GitPushConfig](stepCtx.Config); err != nil { - return failure, + cfg, err := configToStruct[GitPushConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf("could not convert config into git-push config: %w", err) } - // TODO: Add implementation here - return Result{Status: StatusSuccess}, nil + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-push directive configuration against the JSON +// schema. +func (g *gitPushDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), "git-push") +} + +func (g *gitPushDirective) run( + ctx context.Context, + stepCtx *StepContext, + cfg GitPushConfig, +) (Result, error) { + // This is kind of hacky, but we needed to load the working tree to get the + // URL of the repository. With that in hand, we can look for applicable + // credentials and, if found, reload the work tree with the credentials. + path, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.Path) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + cfg.Path, stepCtx.WorkDir, err, + ) + } + loadOpts := &git.LoadWorkTreeOptions{} + workTree, err := git.LoadWorkTree(path, loadOpts) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error loading working tree from %s: %w", cfg.Path, err) + } + var creds credentials.Credentials + var found bool + if creds, found, err = stepCtx.CredentialsDB.Get( + ctx, + stepCtx.Project, + credentials.TypeGit, + workTree.URL(), + ); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error getting credentials for %s: %w", workTree.URL(), err) + } else if found { + loadOpts.Credentials = &git.RepoCredentials{ + Username: creds.Username, + Password: creds.Password, + SSHPrivateKey: creds.SSHPrivateKey, + } + } + if workTree, err = git.LoadWorkTree(path, loadOpts); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error loading working tree from %s: %w", cfg.Path, err) + } + pushOpts := &git.PushOptions{ + // Start with whatever was specified in the config, which may be empty + TargetBranch: cfg.TargetBranch, + } + // If we're supposed to generate a target branch name, do so + if cfg.GenerateTargetBranch { + pushOpts.TargetBranch = fmt.Sprintf("kargo/%s/%s/promotion", stepCtx.Project, stepCtx.Stage) + pushOpts.Force = true + } + targetBranch := pushOpts.TargetBranch + if targetBranch == "" { + // If retBranch is still empty, we want to set it to the current branch + // because we will want to return the branch that was pushed to, but we + // don't want to mess with the options any further. + if targetBranch, err = workTree.CurrentBranch(); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error getting current branch: %w", err) + } + } + if err := workTree.Push(pushOpts); err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error pushing commits to remote: %w", err) + } + return Result{ + Status: StatusSuccess, + Output: State{ + branchKey: targetBranch, + }, + }, nil } diff --git a/internal/directives/git_push_directive_test.go b/internal/directives/git_push_directive_test.go index 0e7f6e7dad..69551d7139 100644 --- a/internal/directives/git_push_directive_test.go +++ b/internal/directives/git_push_directive_test.go @@ -2,128 +2,209 @@ package directives import ( "context" + "fmt" + "net/http/httptest" + "os" + "path/filepath" "testing" + "github.com/sosedoff/gitkit" "github.com/stretchr/testify/require" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" ) -func TestGitPushDirective_Run(t *testing.T) { - ctx := context.Background() +func TestGitPushDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "path not specified", + config: Config{}, + expectedProblems: []string{ + "(root): path is required", + }, + }, + { + name: "path is empty string", + config: Config{ + "path": "", + }, + expectedProblems: []string{ + "path: String length must be greater than or equal to 1", + }, + }, + { + name: "just generateTargetBranch is true", + config: Config{ // Should be completely valid + "generateTargetBranch": true, + "path": "/fake/path", + }, + }, + { + name: "generateTargetBranch is true and targetBranch is empty string", + config: Config{ // Should be completely valid + "generateTargetBranch": true, + "path": "/fake/path", + "targetBranch": "", + }, + }, + { + name: "generateTargetBranch is true and targetBranch is specified", + // These are meant to be mutually exclusive. + config: Config{ + "generateTargetBranch": true, + "targetBranch": "fake-branch", + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "generateTargetBranch not specified and targetBranch not specified", + config: Config{}, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "generateTargetBranch not specified and targetBranch is empty string", + config: Config{ + "targetBranch": "", + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "generateTargetBranch not specified and targetBranch is specified", + config: Config{ // Should be completely valid + "path": "/fake/path", + "targetBranch": "fake-branch", + }, + }, + { + name: "just generateTargetBranch is false", + config: Config{ + "generateTargetBranch": false, + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "generateTargetBranch is false and targetBranch is empty string", + config: Config{ + "generateTargetBranch": false, + "targetBranch": "", + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "generateTargetBranch is false and targetBranch is specified", + config: Config{ // Should be completely valid + "path": "/fake/path", + "targetBranch": "fake-branch", + }, + }, + } d := newGitPushDirective() + dir, ok := d.(*gitPushDirective) + require.True(t, ok) - t.Run("validations", func(t *testing.T) { - testCases := []struct { - name string - config Config - expectedProblems []string - }{ - { - name: "path not specified", - config: Config{}, - expectedProblems: []string{ - "(root): path is required", - }, - }, - { - name: "path is empty string", - config: Config{ - "path": "", - }, - expectedProblems: []string{ - "path: String length must be greater than or equal to 1", - }, - }, - { - name: "just generateTargetBranch is true", - config: Config{ // Should be completely valid - "generateTargetBranch": true, - "path": "/fake/path", - }, - }, - { - name: "generateTargetBranch is true and targetBranch is empty string", - config: Config{ // Should be completely valid - "generateTargetBranch": true, - "path": "/fake/path", - "targetBranch": "", - }, - }, - { - name: "generateTargetBranch is true and targetBranch is specified", - // These are meant to be mutually exclusive. - config: Config{ - "generateTargetBranch": true, - "targetBranch": "fake-branch", - }, - expectedProblems: []string{ - "(root): Must validate one and only one schema", - }, - }, - { - name: "generateTargetBranch not specified and targetBranch not specified", - config: Config{}, - expectedProblems: []string{ - "(root): Must validate one and only one schema", - }, - }, - { - name: "generateTargetBranch not specified and targetBranch is empty string", - config: Config{ - "targetBranch": "", - }, - expectedProblems: []string{ - "(root): Must validate one and only one schema", - }, - }, - { - name: "generateTargetBranch not specified and targetBranch is specified", - config: Config{ // Should be completely valid - "path": "/fake/path", - "targetBranch": "fake-branch", - }, - }, - { - name: "just generateTargetBranch is false", - config: Config{ - "generateTargetBranch": false, - }, - expectedProblems: []string{ - "(root): Must validate one and only one schema", - }, - }, - { - name: "generateTargetBranch is false and targetBranch is empty string", - config: Config{ - "generateTargetBranch": false, - "targetBranch": "", - }, - expectedProblems: []string{ - "(root): Must validate one and only one schema", - }, - }, - { - name: "generateTargetBranch is false and targetBranch is specified", - config: Config{ // Should be completely valid - "path": "/fake/path", - "targetBranch": "fake-branch", - }, - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - stepCtx := &StepContext{ - Config: testCase.config, + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) } - _, err := d.Run(ctx, stepCtx) - if len(testCase.expectedProblems) == 0 { - require.NoError(t, err) - } else { - for _, problem := range testCase.expectedProblems { - require.ErrorContains(t, err, problem) - } - } - }) - } - }) + } + }) + } +} + +func TestGitPushDirective_Run(t *testing.T) { + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // This is the URL of the "remote" repository + testRepoURL := fmt.Sprintf("%s/test.git", server.URL) + + workDir := t.TempDir() + + // Finagle a local bare repo and working tree into place the way that the + // git-clone directive might have so we can verify the git-push directive's + // ability to reload the working tree from the file system. + repo, err := git.CloneBare( + testRepoURL, + nil, + &git.BareCloneOptions{ + BaseDir: workDir, + }, + ) + require.NoError(t, err) + defer repo.Close() + // "master" is still the default branch name for a new repository + // unless you configure it otherwise. + workTreePath := filepath.Join(workDir, "master") + workTree, err := repo.AddWorkTree( + workTreePath, + &git.AddWorkTreeOptions{Orphan: true}, + ) + require.NoError(t, err) + // `git worktree add` doesn't give much control over the branch name when you + // create an orphaned working tree, so we have to follow up with this to make + // the branch name look like what we wanted. The git-clone directive does + // this internally as well. + err = workTree.CreateOrphanedBranch("master") + require.NoError(t, err) + + // Write a file. It will be the git-commit directive's job to commit it. + err = os.WriteFile(filepath.Join(workTree.Dir(), "test.txt"), []byte("foo"), 0600) + require.NoError(t, err) + + // Commit the changes similarly to how the git-commit directive would. + err = workTree.AddAllAndCommit("Initial commit") + require.NoError(t, err) + + // Now we can proceed to test the git-push directive... + + d := newGitPushDirective() + dir, ok := d.(*gitPushDirective) + require.True(t, ok) + + res, err := dir.run( + context.Background(), + &StepContext{ + Project: "fake-project", + Stage: "fake-stage", + WorkDir: workDir, + CredentialsDB: &credentials.FakeDB{}, + }, + GitPushConfig{ + Path: "master", + GenerateTargetBranch: true, + }, + ) + require.NoError(t, err) + branchName, ok := res.Output.Get(branchKey) + require.True(t, ok) + require.Equal(t, "kargo/fake-project/fake-stage/promotion", branchName) } diff --git a/internal/directives/git_wait_for_pr_directive.go b/internal/directives/git_wait_for_pr_directive.go new file mode 100644 index 0000000000..44b47fa8e7 --- /dev/null +++ b/internal/directives/git_wait_for_pr_directive.go @@ -0,0 +1,162 @@ +package directives + +import ( + "context" + "fmt" + + "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/controller/git" + "github.com/akuity/kargo/internal/credentials" + "github.com/akuity/kargo/internal/gitprovider" +) + +func init() { + // Register the git-wait-for-pr directive with the builtins registry. + builtins.RegisterDirective( + newGitWaitForPRDirective(), + &DirectivePermissions{AllowCredentialsDB: true}, + ) +} + +// gitWaitForPRDirective is a directive that waits for a pull request to be +// merged or closed unmerged. +type gitWaitForPRDirective struct { + schemaLoader gojsonschema.JSONLoader +} + +// newGitWaitForPRDirective creates a new git-wait-for-pr directive. +func newGitWaitForPRDirective() Directive { + d := &gitWaitForPRDirective{} + d.schemaLoader = getConfigSchemaLoader(d.Name()) + return d +} + +// Name implements the Directive interface. +func (g *gitWaitForPRDirective) Name() string { + return "git-wait-for-pr" +} + +// Run implements the Directive interface. +func (g *gitWaitForPRDirective) Run( + ctx context.Context, + stepCtx *StepContext, +) (Result, error) { + if err := g.validate(stepCtx.Config); err != nil { + return Result{Status: StatusFailure}, err + } + cfg, err := configToStruct[GitWaitForPRConfig](stepCtx.Config) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("could not convert config into git-wait-for-pr config: %w", err) + } + return g.run(ctx, stepCtx, cfg) +} + +// validate validates the git-wait-for-pr directive configuration against the +// JSON schema. +func (g *gitWaitForPRDirective) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitWaitForPRDirective) run( + ctx context.Context, + stepCtx *StepContext, + cfg GitWaitForPRConfig, +) (Result, error) { + prNumber, err := getPRNumber(stepCtx.SharedState, cfg) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error getting PR number: %w", err) + } + + var repoCreds *git.RepoCredentials + creds, found, err := stepCtx.CredentialsDB.Get( + ctx, + stepCtx.Project, + credentials.TypeGit, + cfg.RepoURL, + ) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf("error getting credentials for %s: %w", cfg.RepoURL, err) + } + if found { + repoCreds = &git.RepoCredentials{ + Username: creds.Username, + Password: creds.Password, + SSHPrivateKey: creds.SSHPrivateKey, + } + } + + gpOpts := &gitprovider.GitProviderOptions{ + InsecureSkipTLSVerify: cfg.InsecureSkipTLSVerify, + } + if repoCreds != nil { + gpOpts.Token = repoCreds.Password + } + if cfg.Provider != nil { + gpOpts.Name = string(*cfg.Provider) + } + gitProviderSvc, err := gitprovider.NewGitProviderService(cfg.RepoURL, gpOpts) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error creating git provider service: %w", err) + } + + pr, err := gitProviderSvc.GetPullRequest(ctx, prNumber) + if err != nil { + return Result{Status: StatusFailure}, + fmt.Errorf("error getting pull request %d: %w", prNumber, err) + } + if pr.IsOpen() { + return Result{Status: StatusPending}, nil + } + + merged, err := gitProviderSvc.IsPullRequestMerged(ctx, prNumber) + if err != nil { + return Result{Status: StatusFailure}, fmt.Errorf( + "error checking if pull request %d was merged: %w", + prNumber, err, + ) + } + if !merged { + return Result{Status: StatusFailure}, + fmt.Errorf("pull request %d was closed without being merged", prNumber) + } + + return Result{Status: StatusSuccess}, nil +} + +func getPRNumber(sharedState State, cfg GitWaitForPRConfig) (int64, error) { + prNumber := cfg.PRNumber + if cfg.PRNumberFromOpen != "" { + stepOutput, exists := sharedState.Get(cfg.PRNumberFromOpen) + if !exists { + return 0, fmt.Errorf( + "no output found from step with alias %q", + cfg.PRNumberFromOpen, + ) + } + stepOutputState, ok := stepOutput.(State) + if !ok { + return 0, fmt.Errorf( + "output from step with alias %q is not a State", + cfg.PRNumberFromOpen, + ) + } + prNumberAny, exists := stepOutputState.Get(branchKey) + if !exists { + return 0, fmt.Errorf( + "no PR number found in output from step with alias %q", + cfg.PRNumberFromOpen, + ) + } + if prNumber, ok = prNumberAny.(int64); !ok { + return 0, fmt.Errorf( + "PR number in output from step with alias %q is not an int64", + cfg.PRNumberFromOpen, + ) + } + } + return prNumber, nil +} diff --git a/internal/directives/git_wait_for_pr_directive_test.go b/internal/directives/git_wait_for_pr_directive_test.go new file mode 100644 index 0000000000..a52535bde8 --- /dev/null +++ b/internal/directives/git_wait_for_pr_directive_test.go @@ -0,0 +1,236 @@ +package directives + +import ( + "context" + "errors" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" + + "github.com/akuity/kargo/internal/credentials" + "github.com/akuity/kargo/internal/gitprovider" +) + +func TestGitWaitForPRDirective_Validate(t *testing.T) { + testCases := []struct { + name string + config Config + expectedProblems []string + }{ + { + name: "repoURL not specified", + config: Config{}, + expectedProblems: []string{ + "(root): repoURL is required", + }, + }, + { + name: "repoURL is empty string", + config: Config{ + "repoURL": "", + }, + expectedProblems: []string{ + "repoURL: String length must be greater than or equal to 1", + }, + }, + { + name: "neither prNumber nor prNumberFromOpen specified", + config: Config{}, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "both prNumber and prNumberFromOpen specified", + config: Config{ + "prNumber": 42, + "prNumberFromOpen": "fake-step", + }, + expectedProblems: []string{ + "(root): Must validate one and only one schema", + }, + }, + { + name: "provider is an invalid value", + config: Config{ + "provider": "bogus", + }, + expectedProblems: []string{ + "provider: provider must be one of the following:", + }, + }, + { + name: "valid without explicit provider", + config: Config{ + "prNumber": 42, + "repoURL": "https://github.com/example/repo.git", + }, + }, + { + name: "valid with explicit provider", + config: Config{ + "provider": "github", + "prNumber": 42, + "repoURL": "https://github.com/example/repo.git", + }, + }, + } + + d := newGitWaitForPRDirective() + dir, ok := d.(*gitWaitForPRDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := dir.validate(testCase.config) + if len(testCase.expectedProblems) == 0 { + require.NoError(t, err) + } else { + for _, problem := range testCase.expectedProblems { + require.ErrorContains(t, err, problem) + } + } + }) + } +} + +func TestGitWaitForPRDirective__Run(t *testing.T) { + testCases := []struct { + name string + provider gitprovider.GitProviderService + assertions func(*testing.T, Result, error) + }{ + { + name: "error finding PR", + provider: &gitprovider.FakeGitProviderService{ + GetPullRequestFn: func( + context.Context, + int64, + ) (*gitprovider.PullRequest, error) { + return nil, errors.New("something went wrong") + }, + }, + assertions: func(t *testing.T, res Result, err error) { + require.ErrorContains(t, err, "error getting pull request") + require.ErrorContains(t, err, "something went wrong") + require.Equal(t, StatusFailure, res.Status) + }, + }, + { + name: "PR is open", + provider: &gitprovider.FakeGitProviderService{ + GetPullRequestFn: func( + context.Context, + int64, + ) (*gitprovider.PullRequest, error) { + return &gitprovider.PullRequest{ + State: gitprovider.PullRequestStateOpen, + }, nil + }, + }, + assertions: func(t *testing.T, res Result, err error) { + require.NoError(t, err) + require.Equal(t, StatusPending, res.Status) + }, + }, + { + name: "error checking if PR was merged", + provider: &gitprovider.FakeGitProviderService{ + GetPullRequestFn: func( + context.Context, + int64, + ) (*gitprovider.PullRequest, error) { + return &gitprovider.PullRequest{ + State: gitprovider.PullRequestStateClosed, + }, nil + }, + IsPullRequestMergedFn: func(context.Context, int64) (bool, error) { + return false, errors.New("something went wrong") + }, + }, + assertions: func(t *testing.T, res Result, err error) { + require.ErrorContains(t, err, "error checking if pull request") + require.ErrorContains(t, err, "was merged") + require.ErrorContains(t, err, "something went wrong") + require.Equal(t, StatusFailure, res.Status) + }, + }, + { + name: "PR is closed and not merged", + provider: &gitprovider.FakeGitProviderService{ + GetPullRequestFn: func( + context.Context, + int64, + ) (*gitprovider.PullRequest, error) { + return &gitprovider.PullRequest{ + State: gitprovider.PullRequestStateClosed, + }, nil + }, + IsPullRequestMergedFn: func(context.Context, int64) (bool, error) { + return false, nil + }, + }, + assertions: func(t *testing.T, res Result, err error) { + require.ErrorContains(t, err, "was closed without being merged") + require.Equal(t, StatusFailure, res.Status) + }, + }, + { + name: "PR is closed and merged", + provider: &gitprovider.FakeGitProviderService{ + GetPullRequestFn: func( + context.Context, + int64, + ) (*gitprovider.PullRequest, error) { + return &gitprovider.PullRequest{ + State: gitprovider.PullRequestStateClosed, + }, nil + }, + IsPullRequestMergedFn: func(context.Context, int64) (bool, error) { + return true, nil + }, + }, + assertions: func(t *testing.T, res Result, err error) { + require.NoError(t, err) + require.Equal(t, StatusSuccess, res.Status) + }, + }, + } + + d := newGitWaitForPRDirective() + dir, ok := d.(*gitWaitForPRDirective) + require.True(t, ok) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + // Cannot register multiple providers with the same name, so this takes + // care of that problem + testGitProviderName := uuid.NewString() + + gitprovider.RegisterProvider( + testGitProviderName, + gitprovider.ProviderRegistration{ + NewService: func( + string, + *gitprovider.GitProviderOptions, + ) (gitprovider.GitProviderService, error) { + return testCase.provider, nil + }, + }, + ) + + res, err := dir.run( + context.Background(), + &StepContext{ + CredentialsDB: &credentials.FakeDB{}, + }, + GitWaitForPRConfig{ + Provider: ptr.To(Provider(testGitProviderName)), + }, + ) + testCase.assertions(t, res, err) + }) + } +} diff --git a/internal/directives/schemas/git-clone-config.json b/internal/directives/schemas/git-clone-config.json index 306f66c967..cb7aedfb9c 100644 --- a/internal/directives/schemas/git-clone-config.json +++ b/internal/directives/schemas/git-clone-config.json @@ -28,6 +28,10 @@ "type": "string", "description": "The branch to checkout. Mutually exclusive with 'tag' and 'fromFreight=true'. If none of these is specified, the default branch is checked out." }, + "create": { + "type": "boolean", + "description": "Indicates whether a new, empty orphan branch should be created if the branch does not already exist. Default is false." + }, "fromFreight": { "type": "boolean", "description": "Indicates whether the ID of a commit to check out may be obtained from Freight. A value of 'true' is mutually exclusive with 'branch' and 'tag'. If none of these is specified, the default branch is checked out." diff --git a/internal/directives/schemas/git-commit-config.json b/internal/directives/schemas/git-commit-config.json index 486a3c06b8..f033771164 100644 --- a/internal/directives/schemas/git-commit-config.json +++ b/internal/directives/schemas/git-commit-config.json @@ -30,12 +30,36 @@ }, "message": { "type": "string", - "description": "The commit message." + "description": "The commit message. Mutually exclusive with 'messageFrom'.", + "minLength": 1 + }, + "messageFrom": { + "type": "array", + "description": "TODO", + "minItems": 1, + "items": { + "type": "string", + "minLength": 1 + } }, "path": { "type": "string", "description": "The path to a working directory of a local repository.", "minLength": 1 } - } + }, + "oneOf": [ + { + "required": ["message"], + "properties": { + "messageFrom": { "enum": [null] } + } + }, + { + "required": ["messageFrom"], + "properties": { + "message": { "enum": [null] } + } + } + ] } diff --git a/internal/directives/schemas/git-open-pr-config.json b/internal/directives/schemas/git-open-pr-config.json new file mode 100644 index 0000000000..e276e6ed3d --- /dev/null +++ b/internal/directives/schemas/git-open-pr-config.json @@ -0,0 +1,57 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "GitOpenPRConfig", + "type": "object", + "additionalProperties": false, + "required": ["repoURL", "targetBranch"], + "properties": { + "createTargetBranch": { + "type": "boolean", + "description": "Indicates whether a new, empty orphan branch should be created and pushed to the remote if the target branch does not already exist there. Default is false." + }, + "insecureSkipTLSVerify" : { + "type": "boolean", + "description": "Indicates whether to skip TLS verification when cloning the repository. Default is false." + }, + "provider": { + "type": "string", + "description": "The name of the Git provider to use. Currently only 'github' and 'gitlab' are supported. Kargo will try to infer the provider if it is not explicitly specified.", + "enum": ["github", "gitlab"] + }, + "repoURL": { + "type": "string", + "description": "The URL of a remote Git repository to clone.", + "minLength": 1, + "format": "uri" + }, + "sourceBranch": { + "type": "string", + "description": "The branch containing the changes to be merged. This branch must already exist and be up to date on the remote.", + "minLength": 1 + }, + "sourceBranchFromPush": { + "type": "string", + "description": "References a previous push step by alias and will use the branch written to by that step as the source branch.", + "minLength": 1 + }, + "targetBranch": { + "type": "string", + "description": "The branch to which the changes should be merged. This branch must already exist and be up to date on the remote.", + "minLength": 1 + } + }, + "oneOf": [ + { + "required": ["sourceBranch"], + "properties": { + "sourceBranchFromPush": { "enum": ["", null] } + } + }, + { + "required": ["sourceBranchFromPush"], + "properties": { + "sourceBranch": { "enum": ["", null] } + } + } + ] +} diff --git a/internal/directives/schemas/git-overwrite-config.json b/internal/directives/schemas/git-overwrite-config.json new file mode 100644 index 0000000000..df9ed69761 --- /dev/null +++ b/internal/directives/schemas/git-overwrite-config.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "GitOverwriteConfig", + "type": "object", + "additionalProperties": false, + "required": ["inPath", "outPath"], + "properties": { + "inPath": { + "type": "string", + "description": "A path to a directory from which to copy all contents, excluding the .git/ directory, if one exists.", + "minLength": 1 + }, + "outPath": { + "type": "string", + "description": "A path to a git working tree which will be cleared of all existing content before receiving a copy of all content specified by inPath.", + "minLength": 1 + } + } +} diff --git a/internal/directives/schemas/git-wait-for-pr-config.json b/internal/directives/schemas/git-wait-for-pr-config.json new file mode 100644 index 0000000000..28e4ec6520 --- /dev/null +++ b/internal/directives/schemas/git-wait-for-pr-config.json @@ -0,0 +1,47 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "GitWaitForPRConfig", + "type": "object", + "additionalProperties": false, + "required": ["repoURL"], + "properties": { + "insecureSkipTLSVerify" : { + "type": "boolean", + "description": "Indicates whether to skip TLS verification when cloning the repository. Default is false." + }, + "provider": { + "type": "string", + "description": "The name of the Git provider to use. Currently only 'github' and 'gitlab' are supported. Kargo will try to infer the provider if it is not explicitly specified.", + "enum": ["github", "gitlab"] + }, + "prNumber": { + "type": "number", + "description": "The number of the pull request to wait for." + }, + "prNumberFromOpen": { + "type": "string", + "description": "References a previous open step by alias and will use the PR number opened by that step.", + "minLength": 1 + }, + "repoURL": { + "type": "string", + "description": "The URL of a remote Git repository to clone.", + "minLength": 1, + "format": "uri" + } + }, + "oneOf": [ + { + "required": ["prNumber"], + "properties": { + "prNumberFromOpen": { "enum": ["", null] } + } + }, + { + "required": ["prNumberFromOpen"], + "properties": { + "prNumber": { "enum": [0, null] } + } + } + ] +} diff --git a/internal/directives/zz_config_types.go b/internal/directives/zz_config_types.go index 8677b8369b..73ea0972bc 100644 --- a/internal/directives/zz_config_types.go +++ b/internal/directives/zz_config_types.go @@ -25,6 +25,9 @@ type Checkout struct { // The branch to checkout. Mutually exclusive with 'tag' and 'fromFreight=true'. If none of // these is specified, the default branch is checked out. Branch string `json:"branch,omitempty"` + // Indicates whether a new, empty orphan branch should be created if the branch does not + // already exist. Default is false. + Create bool `json:"create,omitempty"` // Indicates whether the ID of a commit to check out may be obtained from Freight. A value // of 'true' is mutually exclusive with 'branch' and 'tag'. If none of these is specified, // the default branch is checked out. @@ -47,8 +50,10 @@ type CheckoutFromOrigin struct { type GitCommitConfig struct { // The author of the commit. Author *Author `json:"author,omitempty"` - // The commit message. + // The commit message. Mutually exclusive with 'messageFrom'. Message string `json:"message,omitempty"` + // TODO + MessageFrom []string `json:"messageFrom,omitempty"` // The path to a working directory of a local repository. Path string `json:"path"` } @@ -61,6 +66,37 @@ type Author struct { Name string `json:"name,omitempty"` } +type GitOpenPRConfig struct { + // Indicates whether a new, empty orphan branch should be created and pushed to the remote + // if the target branch does not already exist there. Default is false. + CreateTargetBranch bool `json:"createTargetBranch,omitempty"` + // Indicates whether to skip TLS verification when cloning the repository. Default is false. + InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` + // The name of the Git provider to use. Currently only 'github' and 'gitlab' are supported. + // Kargo will try to infer the provider if it is not explicitly specified. + Provider *Provider `json:"provider,omitempty"` + // The URL of a remote Git repository to clone. + RepoURL string `json:"repoURL"` + // The branch containing the changes to be merged. This branch must already exist and be up + // to date on the remote. + SourceBranch string `json:"sourceBranch,omitempty"` + // References a previous push step by alias and will use the branch written to by that step + // as the source branch. + SourceBranchFromPush string `json:"sourceBranchFromPush,omitempty"` + // The branch to which the changes should be merged. This branch must already exist and be + // up to date on the remote. + TargetBranch string `json:"targetBranch"` +} + +type GitOverwriteConfig struct { + // A path to a directory from which to copy all contents, excluding the .git/ directory, if + // one exists. + InPath string `json:"inPath"` + // A path to a git working tree which will be cleared of all existing content before + // receiving a copy of all content specified by inPath. + OutPath string `json:"outPath"` +} + type GitPushConfig struct { // Indicates whether to push to a new remote branch. A value of 'true' is mutually exclusive // with 'targetBranch'. If neither of these is provided, the target branch will be the @@ -73,6 +109,20 @@ type GitPushConfig struct { TargetBranch string `json:"targetBranch,omitempty"` } +type GitWaitForPRConfig struct { + // Indicates whether to skip TLS verification when cloning the repository. Default is false. + InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` + // The number of the pull request to wait for. + PRNumber int64 `json:"prNumber,omitempty"` + // References a previous open step by alias and will use the PR number opened by that step. + PRNumberFromOpen string `json:"prNumberFromOpen,omitempty"` + // The name of the Git provider to use. Currently only 'github' and 'gitlab' are supported. + // Kargo will try to infer the provider if it is not explicitly specified. + Provider *Provider `json:"provider,omitempty"` + // The URL of a remote Git repository to clone. + RepoURL string `json:"repoURL"` +} + type HelmTemplateConfig struct { // APIVersions allows a manual set of supported API Versions to be passed when rendering the // manifests. @@ -170,6 +220,15 @@ const ( Warehouse Kind = "Warehouse" ) +// The name of the Git provider to use. Currently only 'github' and 'gitlab' are supported. +// Kargo will try to infer the provider if it is not explicitly specified. +type Provider string + +const ( + Github Provider = "github" + Gitlab Provider = "gitlab" +) + // Specifies the new value for the specified key in the Helm values file. type Value string diff --git a/internal/gitprovider/gitprovider.go b/internal/gitprovider/gitprovider.go index 8c42d82cd9..e41e930b20 100644 --- a/internal/gitprovider/gitprovider.go +++ b/internal/gitprovider/gitprovider.go @@ -71,3 +71,44 @@ type PullRequest struct { func (pr *PullRequest) IsOpen() bool { return pr.State == PullRequestStateOpen } + +type FakeGitProviderService struct { + CreatePullRequestFn func( + context.Context, + CreatePullRequestOpts, + ) (*PullRequest, error) + GetPullRequestFn func(context.Context, int64) (*PullRequest, error) + ListPullRequestsFn func( + context.Context, + ListPullRequestOpts, + ) ([]*PullRequest, error) + IsPullRequestMergedFn func(context.Context, int64) (bool, error) +} + +func (f *FakeGitProviderService) CreatePullRequest( + ctx context.Context, + opts CreatePullRequestOpts, +) (*PullRequest, error) { + return f.CreatePullRequestFn(ctx, opts) +} + +func (f *FakeGitProviderService) GetPullRequest( + ctx context.Context, + number int64, +) (*PullRequest, error) { + return f.GetPullRequestFn(ctx, number) +} + +func (f *FakeGitProviderService) ListPullRequests( + ctx context.Context, + opts ListPullRequestOpts, +) ([]*PullRequest, error) { + return f.ListPullRequestsFn(ctx, opts) +} + +func (f *FakeGitProviderService) IsPullRequestMerged( + ctx context.Context, + number int64, +) (bool, error) { + return f.IsPullRequestMergedFn(ctx, number) +}