diff --git a/cmd/status/release.go b/cmd/status/release.go index 620418ac..0e76064b 100644 --- a/cmd/status/release.go +++ b/cmd/status/release.go @@ -9,7 +9,9 @@ import ( "sort" "strings" + "github.com/codefly-dev/cli/pkg/gh" "github.com/fatih/color" + "github.com/google/go-github/v89/github" "github.com/spf13/cobra" ) @@ -261,18 +263,44 @@ func createAgentIssue(baseDir string, status AgentStatus) error { status.CoreVer, status.LatestCore, status.Delta, agentPath) - // Use gh to create issue - cmd := exec.CommandContext(context.Background(), - "gh", "issue", "create", - "--title", title, - "--body", body, - "--label", "chore", - "--label", "dependencies") - cmd.Dir = agentPath - - if err := cmd.Run(); err != nil { + owner, repo, err := agentRepository(agentPath) + if err != nil { return err } + client, err := gh.NewClient() + if err != nil { + return err + } + _, _, err = client.Issues.Create(context.Background(), owner, repo, &github.IssueRequest{ + Title: github.Ptr(title), + Body: github.Ptr(body), + Labels: &[]string{"chore", "dependencies"}, + }) + return err +} + +func agentRepository(agentPath string) (string, string, error) { + //nolint:gosec // git is invoked with fixed subcommands; agentPath is a scanned filesystem path, never a shell. + out, err := exec.Command("git", "-C", agentPath, "remote", "get-url", "origin").Output() + if err != nil { + return "", "", fmt.Errorf("resolve %s origin remote: %w", agentPath, err) + } + return parseGitHubRemote(strings.TrimSpace(string(out))) +} - return nil +func parseGitHubRemote(remote string) (string, string, error) { + trimmed := strings.TrimSuffix(remote, ".git") + switch { + case strings.HasPrefix(trimmed, "git@github.com:"): + trimmed = strings.TrimPrefix(trimmed, "git@github.com:") + case strings.HasPrefix(trimmed, "https://github.com/"): + trimmed = strings.TrimPrefix(trimmed, "https://github.com/") + default: + return "", "", fmt.Errorf("unrecognized GitHub remote %q", remote) + } + owner, repo, ok := strings.Cut(trimmed, "/") + if !ok || owner == "" || repo == "" { + return "", "", fmt.Errorf("unrecognized GitHub remote %q", remote) + } + return owner, repo, nil } diff --git a/cmd/status/release_test.go b/cmd/status/release_test.go new file mode 100644 index 00000000..e8230fa0 --- /dev/null +++ b/cmd/status/release_test.go @@ -0,0 +1,73 @@ +package status + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os/exec" + "path/filepath" + "testing" + + "github.com/google/go-github/v89/github" +) + +func TestParseGitHubRemote(t *testing.T) { + cases := []struct { + remote string + owner string + repo string + ok bool + }{ + {"git@github.com:codefly-dev/service-redis.git", "codefly-dev", "service-redis", true}, + {"https://github.com/codefly-dev/service-redis.git", "codefly-dev", "service-redis", true}, + {"https://github.com/codefly-dev/service-redis", "codefly-dev", "service-redis", true}, + {"git@gitlab.com:codefly-dev/service-redis.git", "", "", false}, + {"https://github.com/codefly-dev", "", "", false}, + } + for _, tc := range cases { + owner, repo, err := parseGitHubRemote(tc.remote) + if tc.ok != (err == nil) { + t.Fatalf("%s: err = %v, want ok=%v", tc.remote, err, tc.ok) + } + if tc.ok && (owner != tc.owner || repo != tc.repo) { + t.Fatalf("%s: got %s/%s, want %s/%s", tc.remote, owner, repo, tc.owner, tc.repo) + } + } +} + +func TestCreateAgentIssueCreatesIssueThroughAPI(t *testing.T) { + baseDir := t.TempDir() + agentPath := filepath.Join(baseDir, "service-redis") + if err := exec.Command("git", "init", "-q", agentPath).Run(); err != nil { + t.Fatal(err) + } + if err := exec.Command("git", "-C", agentPath, "remote", "add", "origin", + "git@github.com:codefly-dev/service-redis.git").Run(); err != nil { + t.Fatal(err) + } + + var gotPath string + var payload github.IssueRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &payload) + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"number":7,"html_url":"https://github.com/codefly-dev/service-redis/issues/7"}`)) + })) + defer server.Close() + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GITHUB_API_URL", server.URL) + + status := AgentStatus{Name: "service-redis", CoreVer: "0.3.0", LatestCore: "0.3.5", Delta: 5} + if err := createAgentIssue(baseDir, status); err != nil { + t.Fatalf("createAgentIssue: %v", err) + } + if gotPath != "/api/v3/repos/codefly-dev/service-redis/issues" { + t.Fatalf("request path = %q", gotPath) + } + if payload.Labels == nil || len(*payload.Labels) != 2 { + t.Fatalf("labels = %v, want [chore dependencies]", payload.Labels) + } +} diff --git a/pkg/gh/client.go b/pkg/gh/client.go index 28b35b63..620aa442 100644 --- a/pkg/gh/client.go +++ b/pkg/gh/client.go @@ -25,10 +25,16 @@ func Owner(publisher string) string { // unauthenticated 60/hour rate limit that turns listing many pinned agents // flaky, and it is what lets release publishing write to the API at all. func NewClient() (*github.Client, error) { + var options []github.ClientOptionsFunc if token := Token(); token != "" { - return github.NewClient(github.WithAuthToken(token)) + options = append(options, github.WithAuthToken(token)) } - return github.NewClient() + // GITHUB_API_URL is set by GitHub Actions and against GitHub Enterprise; + // honoring it points the client at the right host (and gives tests an HTTP seam). + if endpoint := strings.TrimSpace(os.Getenv("GITHUB_API_URL")); endpoint != "" { + options = append(options, github.WithEnterpriseURLs(endpoint, endpoint)) + } + return github.NewClient(options...) } // Token resolves a GitHub token from GITHUB_TOKEN/GH_TOKEN, falling back to the diff --git a/pkg/gh/client_test.go b/pkg/gh/client_test.go index 417ec995..c5e08b6a 100644 --- a/pkg/gh/client_test.go +++ b/pkg/gh/client_test.go @@ -78,3 +78,18 @@ func TestTokenEmptyWithoutCredentials(t *testing.T) { t.Fatalf("Token() = %q, want empty when no credential source exists", got) } } + +func TestNewClientHonorsAPIEndpoint(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("PATH", "") + t.Setenv("GITHUB_API_URL", "https://ghe.example.com/api/v3") + + client, err := NewClient() + if err != nil { + t.Fatalf("NewClient: %v", err) + } + if got := client.BaseURL(); got != "https://ghe.example.com/api/v3/" { + t.Fatalf("BaseURL = %q, want trailing-slash normalized endpoint", got) + } +} diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index a6643246..cac19686 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -16,9 +16,11 @@ import ( "strconv" "strings" + "github.com/codefly-dev/cli/pkg/gh" "github.com/codefly-dev/cli/pkg/internal/mutationauthority" "github.com/codefly-dev/cli/pkg/orchestration" "github.com/codefly-dev/core/resources" + "github.com/google/go-github/v89/github" "gopkg.in/yaml.v3" ) @@ -1251,69 +1253,74 @@ func openOrUpdatePullRequest(ctx context.Context, prepared *preparedRepository, commit, ) } - output, err := command(ctx, "", "gh", "pr", "list", - "--repo", prepared.plan.RepositorySlug, "--head", prepared.plan.PromotionBranch, - "--base", prepared.plan.BaseBranch, "--state", "open", "--json", "number,url,headRefOid") + owner, repo, err := splitRepositorySlug(prepared.plan.RepositorySlug) if err != nil { - return "", 0, fmt.Errorf("inspect promotion pull request: %w", err) + return "", 0, err } - var existing []struct { - Number int `json:"number"` - URL string `json:"url"` - HeadRefOID string `json:"headRefOid"` + client, err := gh.NewClient() + if err != nil { + return "", 0, err } - if err := json.Unmarshal([]byte(output), &existing); err != nil { - return "", 0, fmt.Errorf("decode promotion pull request: %w", err) + existing, _, err := client.PullRequests.List(ctx, owner, repo, &github.PullRequestListOptions{ + State: "open", + Head: owner + ":" + prepared.plan.PromotionBranch, + Base: prepared.plan.BaseBranch, + }) + if err != nil { + return "", 0, fmt.Errorf("inspect promotion pull request: %w", err) } if len(existing) > 1 { return "", 0, fmt.Errorf("multiple open promotion pull requests target %s", prepared.plan.PromotionBranch) } if len(existing) == 1 { pr := existing[0] - if pr.HeadRefOID != commit { - return "", 0, fmt.Errorf("pull request head is %s, expected %s", pr.HeadRefOID, commit) - } - if _, err := command(ctx, "", "gh", "pr", "edit", strconv.Itoa(pr.Number), - "--repo", prepared.plan.RepositorySlug, "--title", title, "--body", body); err != nil { - return "", 0, fmt.Errorf("update promotion pull request: %w", err) - } - return pr.URL, pr.Number, nil - } - url, err := command(ctx, "", "gh", "pr", "create", "--repo", prepared.plan.RepositorySlug, - "--base", prepared.plan.BaseBranch, "--head", prepared.plan.PromotionBranch, - "--title", title, "--body", body) + if pr.GetHead().GetSHA() != commit { + return "", 0, fmt.Errorf("pull request head is %s, expected %s", pr.GetHead().GetSHA(), commit) + } + if _, _, editErr := client.PullRequests.Edit(ctx, owner, repo, pr.GetNumber(), &github.PullRequest{ + Title: github.Ptr(title), + Body: github.Ptr(body), + }); editErr != nil { + return "", 0, fmt.Errorf("update promotion pull request: %w", editErr) + } + return pr.GetHTMLURL(), pr.GetNumber(), nil + } + created, _, err := client.PullRequests.Create(ctx, owner, repo, &github.NewPullRequest{ + Title: github.Ptr(title), + Head: github.Ptr(prepared.plan.PromotionBranch), + Base: github.Ptr(prepared.plan.BaseBranch), + Body: github.Ptr(body), + }) if err != nil { return "", 0, fmt.Errorf("open promotion pull request: %w", err) } - return verifyPullRequest(ctx, prepared.plan.RepositorySlug, strings.TrimSpace(url), prepared.plan.BaseBranch, commit) + return verifyPullRequest(ctx, client, owner, repo, created.GetNumber(), prepared.plan.BaseBranch, commit) +} + +func splitRepositorySlug(slug string) (string, string, error) { + owner, repo, ok := strings.Cut(slug, "/") + if !ok || owner == "" || repo == "" { + return "", "", fmt.Errorf("invalid repository %q", slug) + } + return owner, repo, nil } func localReviewRef(promotionBranch, commit string) string { return "refs/codefly/reviews/" + strings.ReplaceAll(promotionBranch, "/", "-") + "/" + commit } -func verifyPullRequest(ctx context.Context, repository, pullRequest, baseBranch, commit string) (string, int, error) { - output, err := command(ctx, "", "gh", "pr", "view", pullRequest, "--repo", repository, - "--json", "number,url,headRefOid,baseRefName") +func verifyPullRequest(ctx context.Context, client *github.Client, owner, repo string, number int, baseBranch, commit string) (string, int, error) { + pr, _, err := client.PullRequests.Get(ctx, owner, repo, number) if err != nil { return "", 0, fmt.Errorf("verify promotion pull request: %w", err) } - var response struct { - Number int `json:"number"` - URL string `json:"url"` - HeadRefOID string `json:"headRefOid"` - BaseRefName string `json:"baseRefName"` - } - if err := json.Unmarshal([]byte(output), &response); err != nil { - return "", 0, fmt.Errorf("decode verified promotion pull request: %w", err) - } - if response.HeadRefOID != commit || response.BaseRefName != baseBranch { + if pr.GetHead().GetSHA() != commit || pr.GetBase().GetRef() != baseBranch { return "", 0, fmt.Errorf( "promotion pull request targets %s at %s, expected %s at %s", - response.BaseRefName, response.HeadRefOID, baseBranch, commit, + pr.GetBase().GetRef(), pr.GetHead().GetSHA(), baseBranch, commit, ) } - return response.URL, response.Number, nil + return pr.GetHTMLURL(), pr.GetNumber(), nil } func resolveGitops(workspace *resources.Workspace, environment string, local bool) (*repositoryConfig, string, string, string, error) { diff --git a/pkg/gitops/publish_test.go b/pkg/gitops/publish_test.go index e94fd755..a8ddabb7 100644 --- a/pkg/gitops/publish_test.go +++ b/pkg/gitops/publish_test.go @@ -3,6 +3,8 @@ package gitops import ( "context" "fmt" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" @@ -16,6 +18,67 @@ import ( var preparedPermit = mutationauthority.NewPreparedPermit() +func TestOpenOrUpdatePullRequestEditsExistingOpenPromotion(t *testing.T) { + const commit = "1234567890123456789012345678901234567890" + prepared := &preparedRepository{plan: PublishPlan{ + RepositorySlug: "codefly-test/manifests", + PromotionBranch: "codefly/promote-payments-aws", + BaseBranch: "main", + }} + + var edited bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/pulls"): + fmt.Fprintf(w, `[{"number":7,"html_url":"https://github.com/codefly-test/manifests/pull/7","head":{"sha":%q}}]`, commit) + case r.Method == http.MethodPatch && strings.HasSuffix(r.URL.Path, "/pulls/7"): + edited = true + fmt.Fprint(w, `{"number":7,"html_url":"https://github.com/codefly-test/manifests/pull/7"}`) + default: + http.Error(w, "unexpected "+r.Method+" "+r.URL.Path, http.StatusInternalServerError) + } + })) + defer server.Close() + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GITHUB_API_URL", server.URL) + + url, number, err := openOrUpdatePullRequest(context.Background(), prepared, &PublishRequest{}, commit) + if err != nil { + t.Fatal(err) + } + if !edited { + t.Fatal("existing pull request was not edited") + } + if url != "https://github.com/codefly-test/manifests/pull/7" || number != 7 { + t.Fatalf("openOrUpdatePullRequest = %q, %d", url, number) + } +} + +func TestOpenOrUpdatePullRequestRejectsHeadCommitDrift(t *testing.T) { + const commit = "1234567890123456789012345678901234567890" + prepared := &preparedRepository{plan: PublishPlan{ + RepositorySlug: "codefly-test/manifests", + PromotionBranch: "codefly/promote-payments-aws", + BaseBranch: "main", + }} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/pulls") { + fmt.Fprint(w, `[{"number":7,"html_url":"https://github.com/codefly-test/manifests/pull/7","head":{"sha":"ffffffffffffffffffffffffffffffffffffffff"}}]`) + return + } + http.Error(w, "unexpected "+r.Method+" "+r.URL.Path, http.StatusInternalServerError) + })) + defer server.Close() + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GITHUB_API_URL", server.URL) + + if _, _, err := openOrUpdatePullRequest(context.Background(), prepared, &PublishRequest{}, commit); err == nil || + !strings.Contains(err.Error(), "pull request head is") { + t.Fatalf("head drift error = %v", err) + } +} + func TestInventoryUnitDirectoriesAreDistinctAndKindChecked(t *testing.T) { dirs, err := inventoryUnitDirectories(&Inventory{Units: []InventoryUnit{ {Kind: UnitKindService, Name: "api", Path: "services/api"}, diff --git a/pkg/gitops/qualification_k3d_test.go b/pkg/gitops/qualification_k3d_test.go index 06d90c4d..609dd287 100644 --- a/pkg/gitops/qualification_k3d_test.go +++ b/pkg/gitops/qualification_k3d_test.go @@ -3,6 +3,8 @@ package gitops import ( "context" "fmt" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" @@ -75,29 +77,32 @@ gitops: if err := os.WriteFile(kubectl, []byte("#!/bin/sh\ntouch \"$CODEFLY_TEST_KUBECTL_CALLED\"\nexit 97\n"), 0o755); err != nil { t.Fatal(err) } - gh := filepath.Join(bin, "gh") - ghScript := `#!/bin/sh -set -eu -if [ "$1 $2" = "pr list" ]; then - printf '%s\n' '[]' - exit 0 -fi -if [ "$1 $2" = "pr create" ]; then - printf '%s\n' 'https://github.com/codefly-test/manifests/pull/1' - exit 0 -fi -if [ "$1 $2" = "pr view" ]; then - revision="$(git --git-dir "$CODEFLY_TEST_REMOTE" rev-parse refs/heads/codefly/promote-payments-aws)" - printf '{"number":1,"url":"https://github.com/codefly-test/manifests/pull/1","headRefOid":"%s","baseRefName":"main"}\n' "$revision" - exit 0 -fi -exit 2 -` - if err := os.WriteFile(gh, []byte(ghScript), 0o755); err != nil { - t.Fatal(err) - } + branchRevision := func() string { + out, err := exec.Command("git", "--git-dir", remote, "rev-parse", "refs/heads/codefly/promote-payments-aws").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) + } + pullRequest := func() string { + return fmt.Sprintf(`{"number":1,"html_url":"https://github.com/codefly-test/manifests/pull/1","head":{"sha":%q},"base":{"ref":"main"}}`, branchRevision()) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/pulls"): + fmt.Fprint(w, `[]`) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/pulls"): + fmt.Fprint(w, pullRequest()) + case strings.HasSuffix(r.URL.Path, "/pulls/1"): + fmt.Fprint(w, pullRequest()) + default: + http.NotFound(w, r) + } + })) + defer server.Close() t.Setenv("CODEFLY_TEST_KUBECTL_CALLED", kubectlCalled) - t.Setenv("CODEFLY_TEST_REMOTE", remote) + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GITHUB_API_URL", server.URL) t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) request := PublishRequest{