diff --git a/cmd/agents/versions.go b/cmd/agents/versions.go index a116b6f4..6e99b9cf 100644 --- a/cmd/agents/versions.go +++ b/cmd/agents/versions.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "os" - "os/exec" "path/filepath" "slices" "sort" @@ -16,6 +15,7 @@ import ( "github.com/blang/semver" "github.com/codefly-dev/cli/cmd/common" "github.com/codefly-dev/cli/pkg/cli" + "github.com/codefly-dev/cli/pkg/gh" "github.com/codefly-dev/core/resources" "github.com/google/go-github/v89/github" "github.com/spf13/cobra" @@ -416,7 +416,7 @@ func pinnedVersions(ctx context.Context, agent *resources.Agent) []string { } func fetchReleasesFromGitHub(ctx context.Context, agent *resources.Agent) ([]releaseInfo, error) { - client, err := newGitHubClient() + client, err := gh.NewClient() if err != nil { return nil, err } @@ -452,7 +452,7 @@ func fetchReleasesFromGitHub(ctx context.Context, agent *resources.Agent) ([]rel } func fetchTagsFromGitHub(ctx context.Context, agent *resources.Agent) ([]string, error) { - client, err := newGitHubClient() + client, err := gh.NewClient() if err != nil { return nil, err } @@ -478,37 +478,7 @@ func fetchTagsFromGitHub(ctx context.Context, agent *resources.Agent) ([]string, // githubSource mirrors manager.toGithubSource (unexported): the publisher's // dots become dashes and the repo is service-. func githubSource(agent *resources.Agent) (owner, repo string) { - return strings.ReplaceAll(agent.Publisher, ".", "-"), "service-" + agent.Name -} - -// newGitHubClient returns a client authenticated with GITHUB_TOKEN/GH_TOKEN -// when either is set. Listing every version of every pinned agent multiplies -// requests fast, and the unauthenticated 60/hour limit turns this diagnostic -// flaky exactly when a workspace has many pins to check. -func newGitHubClient() (*github.Client, error) { - if token := githubToken(); token != "" { - return github.NewClient(github.WithAuthToken(token)) - } - return github.NewClient() -} - -// githubToken resolves a GitHub token from GITHUB_TOKEN/GH_TOKEN, falling back -// to the `gh` CLI's stored credential. Without the `gh` fallback, `agent list`/ -// `versions` runs unauthenticated (60 req/hour) and reports resolvable versions -// as "-" the moment a workspace has several pins to check — a confusing false -// negative on a machine that is in fact fully authenticated via `gh`. -func githubToken() string { - if t := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); t != "" { - return t - } - if t := strings.TrimSpace(os.Getenv("GH_TOKEN")); t != "" { - return t - } - out, err := exec.Command("gh", "auth", "token").Output() - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) + return gh.Owner(agent.Publisher), "service-" + agent.Name } func localCacheVersions(ctx context.Context, agent *resources.Agent) []string { diff --git a/cmd/agents/versions_test.go b/cmd/agents/versions_test.go index 6b0e47ba..0251cf69 100644 --- a/cmd/agents/versions_test.go +++ b/cmd/agents/versions_test.go @@ -303,42 +303,6 @@ func TestLocalCacheVersionsScansAgentDir(t *testing.T) { } } -func TestNewGitHubClientAddsAuthorization(t *testing.T) { - t.Setenv("GITHUB_TOKEN", "secret") - var got string - server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - got = r.Header.Get("Authorization") - })) - defer server.Close() - - client, err := newGitHubClient() - if err != nil { - t.Fatalf("newGitHubClient: %v", err) - } - resp, err := client.Client().Get(server.URL) - if err != nil { - t.Fatal(err) - } - resp.Body.Close() - if got != "Bearer secret" { - t.Fatalf("Authorization = %q, want %q", got, "Bearer secret") - } -} - -func TestNewGitHubClientUnauthenticated(t *testing.T) { - t.Setenv("GITHUB_TOKEN", "") - t.Setenv("GH_TOKEN", "") - t.Setenv("PATH", "") // no `gh` on PATH: force the tokenless path - - client, err := newGitHubClient() - if err != nil { - t.Fatalf("newGitHubClient: %v", err) - } - if client == nil { - t.Fatal("newGitHubClient returned a nil client") - } -} - func TestSummarizeWorkspaceAgentsCachesAndFlagsResolvability(t *testing.T) { restoreReleases, restoreTags, restoreOCI := fetchReleases, fetchTags, fetchOCITags defer func() { fetchReleases, fetchTags, fetchOCITags = restoreReleases, restoreTags, restoreOCI }() diff --git a/cmd/publish/agent_release.go b/cmd/publish/agent_release.go index 0a3b89b3..48d6b1ed 100644 --- a/cmd/publish/agent_release.go +++ b/cmd/publish/agent_release.go @@ -16,8 +16,10 @@ import ( "strings" "time" + "github.com/codefly-dev/cli/pkg/gh" "github.com/codefly-dev/core/agents/manager" "github.com/codefly-dev/core/resources" + "github.com/google/go-github/v89/github" "gopkg.in/yaml.v3" ) @@ -42,14 +44,15 @@ var loaderPlatforms = []platform{ // checkAgentReleasePreconditions fails fast on the two things that would // otherwise only surface AFTER the expensive CI run (or, in `publish // all`, after earlier repos already shipped): a host that can't build -// every loader platform, and a missing gh CLI. Both are deterministic and +// every loader platform, and the absence of any GitHub credential to +// authenticate the release API calls. Both are deterministic and // side-effect free, so they are safe to run during the validate phase. func checkAgentReleasePreconditions() error { if err := hostBuildsLoaderPlatforms(); err != nil { return err } - if _, err := exec.LookPath("gh"); err != nil { - return fmt.Errorf("the gh CLI is required to upload agent release assets but is not on PATH: %w", err) + if gh.Token() == "" { + return fmt.Errorf("a GitHub token is required to publish agent release assets; set GITHUB_TOKEN or GH_TOKEN, or authenticate the gh CLI (gh auth login)") } return nil } @@ -104,7 +107,7 @@ func loaderSBOMName(reg *resources.AgentKindRegistration, name, version string, // (the resolver only knows the host platform). Consistency with the real // resolver is asserted in verifyReleaseAssets for the host target. func loaderDownloadURL(reg *resources.AgentKindRegistration, publisher, name, version string, p platform) string { - owner := strings.ReplaceAll(publisher, ".", "-") + owner := gh.Owner(publisher) return fmt.Sprintf("https://github.com/%s/%s/releases/download/v%s/%s", owner, reg.GitHubRepository(name), version, loaderArchiveName(reg, name, version, p)) } @@ -281,47 +284,77 @@ func runReleaseAgentCI(ctx context.Context, self, agentDir, output string, nativ } // createAndUploadRelease publishes every staged loader archive and SBOM to -// the GitHub release for tag. gh runs from workDir so it resolves the -// repository from the origin remote. +// the GitHub release for tag in owner/repo. // // Idempotent by design: it creates the release on the first publish, or // uploads into an existing one (clobbering same-named assets) on a retry // or `re-tag`. Without this a re-run after a partial upload would error on // the already-existing release, stranding a half-uploaded release. -func createAndUploadRelease(ctx context.Context, workDir, tag string, assets []loaderAsset) error { - files := make([]string, 0, len(assets)*2) +func createAndUploadRelease(ctx context.Context, client *github.Client, owner, repo, tag string, assets []loaderAsset) error { + release, err := getOrCreateRelease(ctx, client, owner, repo, tag) + if err != nil { + return err + } + existing := map[string]int64{} + for _, asset := range release.Assets { + existing[asset.GetName()] = asset.GetID() + } for _, asset := range assets { - files = append(files, asset.archivePath) + files := []string{asset.archivePath} if asset.sbomPath != "" { files = append(files, asset.sbomPath) } + for _, file := range files { + if err := uploadReleaseAsset(ctx, client, owner, repo, release.GetID(), file, existing); err != nil { + return err + } + } } - var args []string - if releaseExists(ctx, workDir, tag) { - args = append([]string{"release", "upload", tag}, files...) - args = append(args, "--clobber") - } else { - args = append([]string{"release", "create", tag, "--title", tag, "--notes", "Release " + tag}, files...) + return nil +} + +// getOrCreateRelease returns the existing release for tag, or creates one when +// none exists yet. A non-404 lookup error is surfaced rather than masked as a +// missing release, so a transient API failure can't silently spawn a duplicate. +func getOrCreateRelease(ctx context.Context, client *github.Client, owner, repo, tag string) (*github.RepositoryRelease, error) { + release, resp, err := client.Repositories.GetReleaseByTag(ctx, owner, repo, tag) + if err == nil { + return release, nil } - cmd := exec.CommandContext(ctx, "gh", args...) - cmd.Dir = workDir - cmd.Env = os.Environ() - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("publish GitHub release %s: %w", tag, err) + if resp == nil || resp.StatusCode != http.StatusNotFound { + return nil, fmt.Errorf("look up GitHub release %s: %w", tag, err) } - return nil + created, _, err := client.Repositories.CreateRelease(ctx, owner, repo, github.CreateReleaseRequest{ + TagName: tag, + Name: github.Ptr(tag), + Body: github.Ptr("Release " + tag), + }) + if err != nil { + return nil, fmt.Errorf("create GitHub release %s: %w", tag, err) + } + return created, nil } -// releaseExists reports whether a GitHub release already exists for tag. -func releaseExists(ctx context.Context, workDir, tag string) bool { - cmd := exec.CommandContext(ctx, "gh", "release", "view", tag) - cmd.Dir = workDir - cmd.Env = os.Environ() - cmd.Stdout = io.Discard - cmd.Stderr = io.Discard - return cmd.Run() == nil +// uploadReleaseAsset uploads path into the release, replicating `gh --clobber`: +// an already-present asset of the same name is deleted first, since the GitHub +// API rejects uploading a duplicate name into a release. +func uploadReleaseAsset(ctx context.Context, client *github.Client, owner, repo string, releaseID int64, path string, existing map[string]int64) error { + name := filepath.Base(path) + if id, ok := existing[name]; ok { + if _, err := client.Repositories.DeleteReleaseAsset(ctx, owner, repo, id); err != nil { + return fmt.Errorf("replace existing release asset %s: %w", name, err) + } + delete(existing, name) + } + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("open release asset %s: %w", path, err) + } + defer file.Close() + if _, _, err := client.Repositories.UploadReleaseAsset(ctx, owner, repo, releaseID, &github.UploadOptions{Name: name}, file); err != nil { + return fmt.Errorf("upload release asset %s: %w", name, err) + } + return nil } // verifyReleaseAssets confirms every uploaded loader archive resolves @@ -387,7 +420,6 @@ func assertAssetReachable(ctx context.Context, url string) error { type agentReleaser struct { self string agentDir string - workDir string reg *resources.AgentKindRegistration skipConformance bool publisher string @@ -428,14 +460,14 @@ var sourceTagKinds = map[string]bool{ } // newAgentReleaseGate selects release behavior from the manifest kind. -func newAgentReleaseGate(agentDir, workDir string) (releaseGate, error) { +func newAgentReleaseGate(agentDir string) (releaseGate, error) { identity, err := readAgentIdentity(filepath.Join(agentDir, "agent.codefly.yaml")) if err != nil { return nil, err } switch { case loaderAssetKinds[identity.Kind]: - return newAgentReleaser(agentDir, workDir) + return newAgentReleaser(agentDir) case sourceTagKinds[identity.Kind]: return newSourceTagReleaser(agentDir, identity.Kind == string(resources.ModuleAgent)) default: @@ -477,7 +509,7 @@ func unsupportedReleaseKindError(kind string) error { return fmt.Errorf("publish supports %s; got %q", strings.Join(supported, ", "), kind) } -func newAgentReleaser(agentDir, workDir string) (*agentReleaser, error) { +func newAgentReleaser(agentDir string) (*agentReleaser, error) { if err := checkAgentReleasePreconditions(); err != nil { return nil, err } @@ -509,7 +541,6 @@ func newAgentReleaser(agentDir, workDir string) (*agentReleaser, error) { return &agentReleaser{ self: self, agentDir: agentDir, - workDir: workDir, reg: ®, skipConformance: reg.Resource != resources.ServiceAgent, publisher: identity.Publisher, @@ -544,7 +575,13 @@ func (r *agentReleaser) beforeCommit(ctx context.Context, newTag string) error { func (r *agentReleaser) afterPush(ctx context.Context, newTag string) error { version := strings.TrimPrefix(newTag, "v") - if err := createAndUploadRelease(ctx, r.workDir, newTag, r.assets); err != nil { + owner := gh.Owner(r.publisher) + repo := r.reg.GitHubRepository(r.name) + client, err := gh.NewClient() + if err != nil { + return err + } + if err := createAndUploadRelease(ctx, client, owner, repo, newTag, r.assets); err != nil { return err } return verifyReleaseAssets(ctx, r.reg, r.publisher, r.name, version, r.assets) diff --git a/cmd/publish/agent_release_test.go b/cmd/publish/agent_release_test.go index 561cc603..2ab7a485 100644 --- a/cmd/publish/agent_release_test.go +++ b/cmd/publish/agent_release_test.go @@ -143,7 +143,7 @@ func TestModuleAndProviderSelectSourceTagGateWithoutLoaderAssets(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "agent.codefly.yaml"), manifest, 0o644)) require.NoError(t, checkAgentReleasePreconditionsForManifest(filepath.Join(dir, "agent.codefly.yaml"))) - gate, err := newAgentReleaseGate(dir, dir) + gate, err := newAgentReleaseGate(dir) require.NoError(t, err) defer gate.cleanup() releaser, ok := gate.(*sourceTagReleaser) @@ -156,8 +156,8 @@ func TestModuleAndProviderSelectSourceTagGateWithoutLoaderAssets(t *testing.T) { func TestLoaderAssetGateSelectsRegistrationAndConformance(t *testing.T) { // Loader-asset publishing requires a host that can build every loader - // platform plus gh — the same gate a real service/toolbox publish hits. - // Skip where that can't be exercised. + // platform plus a resolvable GitHub token — the same gate a real + // service/toolbox publish hits. Skip where that can't be exercised. if err := checkAgentReleasePreconditions(); err != nil { t.Skipf("host cannot exercise loader-asset publishing: %v", err) } @@ -174,7 +174,7 @@ func TestLoaderAssetGateSelectsRegistrationAndConformance(t *testing.T) { manifest := []byte("publisher: codefly.dev\nkind: " + tc.kind + "\nname: web\nversion: 0.0.14\n") require.NoError(t, os.WriteFile(filepath.Join(dir, "agent.codefly.yaml"), manifest, 0o644)) - gate, err := newAgentReleaseGate(dir, dir) + gate, err := newAgentReleaseGate(dir) require.NoError(t, err) defer gate.cleanup() releaser, ok := gate.(*agentReleaser) @@ -202,7 +202,7 @@ func TestUnsupportedAgentKindFailsClosedWithActionableError(t *testing.T) { manifest := []byte("publisher: codefly.dev\nkind: codefly:job\nname: batch\nversion: 0.0.1\n") require.NoError(t, os.WriteFile(filepath.Join(dir, "agent.codefly.yaml"), manifest, 0o644)) - _, err := newAgentReleaseGate(dir, dir) + _, err := newAgentReleaseGate(dir) require.ErrorContains(t, err, "publish supports") require.ErrorContains(t, err, "codefly:service") require.ErrorContains(t, err, `got "codefly:job"`) diff --git a/cmd/publish/all.go b/cmd/publish/all.go index fd18976d..8cffc631 100644 --- a/cmd/publish/all.go +++ b/cmd/publish/all.go @@ -160,7 +160,7 @@ func runAll(c *cobra.Command, args []string) error { timeout := 120 * time.Second var releaser releaseGate if t.Manifest.Mode == ModeAgent { - releaser, err = newAgentReleaseGate(filepath.Dir(t.Manifest.Path), t.Dir) + releaser, err = newAgentReleaseGate(filepath.Dir(t.Manifest.Path)) if err != nil { return fmt.Errorf("prepare agent release for %s: %w", relOrBase(root, t.Dir), err) } diff --git a/cmd/publish/cmd.go b/cmd/publish/cmd.go index 0481269e..b71b2ca4 100644 --- a/cmd/publish/cmd.go +++ b/cmd/publish/cmd.go @@ -97,7 +97,8 @@ func run(c *cobra.Command, args []string) error { // bare tag push, so they get a generous timeout. timeout := 60 * time.Second if manifest.Mode == ModeAgent && !dryRun { - releaser, err := newAgentReleaseGate(filepath.Dir(manifest.Path), workDir) + var releaser releaseGate + releaser, err = newAgentReleaseGate(filepath.Dir(manifest.Path)) if err != nil { return err } @@ -169,7 +170,8 @@ func runReTag(c *cobra.Command, _ []string) error { // but failed to upload. Same generous timeout as publish. timeout := 60 * time.Second if manifest.Mode == ModeAgent && !dryRun { - releaser, err := newAgentReleaseGate(filepath.Dir(manifest.Path), workDir) + var releaser releaseGate + releaser, err = newAgentReleaseGate(filepath.Dir(manifest.Path)) if err != nil { return err } diff --git a/cmd/publish/release_upload_test.go b/cmd/publish/release_upload_test.go new file mode 100644 index 00000000..55bd5583 --- /dev/null +++ b/cmd/publish/release_upload_test.go @@ -0,0 +1,157 @@ +package publish + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/google/go-github/v89/github" + "github.com/stretchr/testify/require" +) + +// fakeGitHub records the release-API calls createAndUploadRelease makes so the +// create vs. clobber behavior can be asserted without reaching GitHub. +type fakeGitHub struct { + uploaded []string // asset names uploaded, in order + deleted []int64 // asset ids deleted (the clobber step) + created bool // whether a release was created +} + +// server stands up an httptest server speaking enough of the releases API and +// returns a client pointed at it. existingAssets seeds a release already +// present for the tag (nil means the tag has no release yet). +func (f *fakeGitHub) server(t *testing.T, tag string, existingAssets map[string]int64) *github.Client { + t.Helper() + const releaseID = 42 + mux := http.NewServeMux() + + mux.HandleFunc("/repos/codefly-dev/service-go/releases/tags/"+tag, func(w http.ResponseWriter, _ *http.Request) { + if existingAssets == nil { + http.Error(w, "not found", http.StatusNotFound) + return + } + writeRelease(w, releaseID, existingAssets) + }) + mux.HandleFunc("/repos/codefly-dev/service-go/releases", func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + f.created = true + writeRelease(w, releaseID, nil) + }) + mux.HandleFunc(fmt.Sprintf("/repos/codefly-dev/service-go/releases/%d/assets", releaseID), func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + f.uploaded = append(f.uploaded, r.URL.Query().Get("name")) + w.WriteHeader(http.StatusCreated) + fmt.Fprint(w, `{}`) + }) + for name, id := range existingAssets { + mux.HandleFunc(fmt.Sprintf("/repos/codefly-dev/service-go/releases/assets/%d", id), func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodDelete, r.Method, "existing asset %q must be deleted before re-upload", name) + f.deleted = append(f.deleted, id) + w.WriteHeader(http.StatusNoContent) + }) + } + + ts := httptest.NewServer(mux) + t.Cleanup(ts.Close) + base := ts.URL + "/" + client, err := github.NewClient(github.WithURLs(&base, &base)) + require.NoError(t, err) + return client +} + +func writeRelease(w http.ResponseWriter, id int64, assets map[string]int64) { + var parts []string + for name, aid := range assets { + parts = append(parts, fmt.Sprintf(`{"id":%d,"name":%q}`, aid, name)) + } + fmt.Fprintf(w, `{"id":%d,"assets":[%s]}`, id, strings.Join(parts, ",")) +} + +func stageAssets(t *testing.T, names ...string) []loaderAsset { + t.Helper() + dir := t.TempDir() + assets := make([]loaderAsset, 0, len(names)) + for _, name := range names { + assets = append(assets, loaderAsset{archivePath: stageFile(t, dir, name)}) + } + return assets +} + +// stageAssetWithSBOM builds a single loader asset carrying both an archive and +// an SBOM, so the SBOM upload branch of createAndUploadRelease is exercised. +func stageAssetWithSBOM(t *testing.T, archiveName, sbomName string) loaderAsset { + t.Helper() + dir := t.TempDir() + return loaderAsset{ + archivePath: stageFile(t, dir, archiveName), + sbomPath: stageFile(t, dir, sbomName), + } +} + +func stageFile(t *testing.T, dir, name string) string { + t.Helper() + path := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(path, []byte("payload-"+name), 0o644)) + return path +} + +func TestCreateAndUploadRelease_CreatesWhenAbsent(t *testing.T) { + f := &fakeGitHub{} + client := f.server(t, "v0.0.16", nil) + assets := stageAssets(t, "service-go_0.0.16_darwin_arm64.tar.gz", "service-go_0.0.16_linux_amd64.tar.gz") + + require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) + + require.True(t, f.created, "a release must be created when the tag has none") + require.ElementsMatch(t, []string{ + "service-go_0.0.16_darwin_arm64.tar.gz", + "service-go_0.0.16_linux_amd64.tar.gz", + }, f.uploaded) + require.Empty(t, f.deleted, "nothing to clobber on a fresh release") +} + +func TestCreateAndUploadRelease_ClobbersExistingAsset(t *testing.T) { + name := "service-go_0.0.16_linux_amd64.tar.gz" + f := &fakeGitHub{} + client := f.server(t, "v0.0.16", map[string]int64{name: 7}) + assets := stageAssets(t, name) + + require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) + + require.False(t, f.created, "an existing release must be reused, not recreated") + require.Equal(t, []int64{7}, f.deleted, "the same-named asset must be deleted before re-upload") + require.Equal(t, []string{name}, f.uploaded) +} + +func TestCreateAndUploadRelease_UploadsSBOMAlongsideArchive(t *testing.T) { + archive := "service-go_0.0.16_linux_amd64.tar.gz" + sbom := "service-go_0.0.16_linux_amd64.cdx.json" + f := &fakeGitHub{} + client := f.server(t, "v0.0.16", nil) + assets := []loaderAsset{stageAssetWithSBOM(t, archive, sbom)} + + require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) + + require.ElementsMatch(t, []string{archive, sbom}, f.uploaded, + "both the archive and its SBOM must be uploaded") +} + +func TestCreateAndUploadRelease_ClobbersSBOMToo(t *testing.T) { + archive := "service-go_0.0.16_linux_amd64.tar.gz" + sbom := "service-go_0.0.16_linux_amd64.cdx.json" + f := &fakeGitHub{} + client := f.server(t, "v0.0.16", map[string]int64{archive: 7, sbom: 9}) + assets := []loaderAsset{stageAssetWithSBOM(t, archive, sbom)} + + require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) + + require.False(t, f.created, "an existing release must be reused, not recreated") + require.ElementsMatch(t, []int64{7, 9}, f.deleted, + "both the archive and SBOM must be deleted before re-upload") + require.ElementsMatch(t, []string{archive, sbom}, f.uploaded) +} diff --git a/pkg/gh/client.go b/pkg/gh/client.go new file mode 100644 index 00000000..28b35b63 --- /dev/null +++ b/pkg/gh/client.go @@ -0,0 +1,51 @@ +// Package gh provides a shared authenticated go-github client and token +// resolution for the CLI's platform (REST API) flows — agent-release +// publishing and version listing — so they resolve credentials one way. +package gh + +import ( + "os" + "os/exec" + "strings" + + "github.com/google/go-github/v89/github" +) + +// Owner maps a codefly publisher to its GitHub repository owner: dots become +// dashes (codefly.dev -> codefly-dev). This is the single source of the rule +// that core's manager.DownloadURL (the install resolver) and the release +// upload/verify paths must all agree on; keeping it in one place is what stops +// an upload target from silently drifting from the URL installers request. +func Owner(publisher string) string { + return strings.ReplaceAll(publisher, ".", "-") +} + +// NewClient returns a client authenticated with a resolved token when one is +// available, or an anonymous client otherwise. Authenticating lifts the +// 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) { + if token := Token(); token != "" { + return github.NewClient(github.WithAuthToken(token)) + } + return github.NewClient() +} + +// Token resolves a GitHub token from GITHUB_TOKEN/GH_TOKEN, falling back to the +// `gh` CLI's stored credential. The fallback keeps local dev working without +// exporting a token; because it is only a credential source, the `gh` binary is +// optional (present a token via env and it is never invoked) rather than +// required. +func Token() string { + if t := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); t != "" { + return t + } + if t := strings.TrimSpace(os.Getenv("GH_TOKEN")); t != "" { + return t + } + out, err := exec.Command("gh", "auth", "token").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} diff --git a/pkg/gh/client_test.go b/pkg/gh/client_test.go new file mode 100644 index 00000000..417ec995 --- /dev/null +++ b/pkg/gh/client_test.go @@ -0,0 +1,80 @@ +package gh + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestOwnerReplacesDotsWithDashes(t *testing.T) { + for _, tc := range []struct{ publisher, want string }{ + {"codefly.dev", "codefly-dev"}, + {"my.org.dev", "my-org-dev"}, + {"codefly", "codefly"}, + } { + if got := Owner(tc.publisher); got != tc.want { + t.Fatalf("Owner(%q) = %q, want %q", tc.publisher, got, tc.want) + } + } +} + +func TestNewClientAddsAuthorization(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "secret") + var got string + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + got = r.Header.Get("Authorization") + })) + defer server.Close() + + client, err := NewClient() + if err != nil { + t.Fatalf("NewClient: %v", err) + } + resp, err := client.Client().Get(server.URL) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if got != "Bearer secret" { + t.Fatalf("Authorization = %q, want %q", got, "Bearer secret") + } +} + +func TestNewClientUnauthenticated(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("PATH", "") // no `gh` on PATH: force the tokenless path + + client, err := NewClient() + if err != nil { + t.Fatalf("NewClient: %v", err) + } + if client == nil { + t.Fatal("NewClient returned a nil client") + } +} + +func TestTokenPrefersEnv(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "from-github-token") + t.Setenv("GH_TOKEN", "from-gh-token") + if got := Token(); got != "from-github-token" { + t.Fatalf("Token() = %q, want GITHUB_TOKEN to win", got) + } +} + +func TestTokenFallsBackToGHToken(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "from-gh-token") + if got := Token(); got != "from-gh-token" { + t.Fatalf("Token() = %q, want GH_TOKEN fallback", got) + } +} + +func TestTokenEmptyWithoutCredentials(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("PATH", "") // no `gh` on PATH: nothing can supply a token + if got := Token(); got != "" { + t.Fatalf("Token() = %q, want empty when no credential source exists", got) + } +}