Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Route gpg key requests through api.Client - #13997

#13997
Open
williammartin wants to merge 1 commit into
williammartin-route-ssh-key-api-clientcli/cli:williammartin-route-ssh-key-api-clientfrom
williammartin-route-gpg-key-apicli/cli:williammartin-route-gpg-key-apiCopy head branch name to clipboard
Open

Route gpg key requests through api.Client#13997
williammartin wants to merge 1 commit into
williammartin-route-ssh-key-api-clientcli/cli:williammartin-route-ssh-key-api-clientfrom
williammartin-route-gpg-key-apicli/cli:williammartin-route-gpg-key-apiCopy head branch name to clipboard

Conversation

@williammartin

@williammartin williammartin commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Part of #13991

This PR migrates gpg-key add, gpg-key delete and gpg-key list to use the api.Client for API interactions. There should be no user visible change.

We add tests for three ordered sentinel errors:

  1. 404 -> errScopesMissing
  2. 422 mentioning a duplicate key_id -> errDuplicateKey
  3. 422 for an unarmored key -> errWrongFormat.

This might be an overspecification of the tests, but it is behaviour that might be depended upon by someone out there, so at least worth knowing if it changed.

Two test stubs changed from RESTPayload(200, "") to RESTPayload(200, "{}") because the api client expects to unmarshal json (where previously the body was just thrown away); The REST endpoints do return JSON. We also backfill tests for other error paths.

Acceptance Test

➜  williammartin-laughing-couscous git:(williammartin-route-gpg-key-api) ✗ GH_ACCEPTANCE_HOST=github.com \
GH_ACCEPTANCE_ORG=gh-acceptance-testing \
GH_ACCEPTANCE_TOKEN="$(gh auth token --hostname github.com)" \
GH_ACCEPTANCE_SCRIPT=gpg-key.txtar \
go test -tags=acceptance -count=1 -v -run '^TestGPGKeys$' ./acceptance
=== RUN   TestGPGKeys
=== RUN   TestGPGKeys/gpg-key
=== PAUSE TestGPGKeys/gpg-key
=== CONT  TestGPGKeys/gpg-key
    testscript.go:584: WORK=$WORK
        PATH=/var/folders/z7/869nt6ns29d77xln9h9bm8680000gn/T/testscript-main1374413527/bin:/Users/williammartin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.26.5.darwin-arm64/bin:/Users/williammartin/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/pkg/env/global/bin:/Users/williammartin/.local/bin
        GOTRACEBACK=system
        HOME=$WORK
        TMPDIR=$WORK/.tmp
        devnull=/dev/null
        /=/
        :=:
        $=$
        exe=
        SCRIPT_NAME=gpg_key
        GH_CONFIG_DIR=$WORK
        GH_HOST=github.com
        ORG=gh-acceptance-testing
        GH_TOKEN=gho_************************************
        RANDOM_STRING=fVJVPWtXGk
        GH_TELEMETRY=false
        
        #skip 'it modifies the user''s personal GitHub account GPG keys'
        # This test requires the admin:gpg_key scope to add and delete GPG keys to and
        # from the user's personal GitHub account. 
        # This test uses a GPG key that generated for this test only. The private key
        # has been deleted
        # Add the gpg key to GH account (0.816s)
        > exec gh gpg-key add gpg-key.pub
        # Verify the gpg key was added to GH account (0.316s)
        > exec gh gpg-key list
        [stdout]
        cli@github.com  24C30F9C9115E747        xjMEZxpWhhYJKwYBBAHaRw8BAQdAmYiobR2ai/lVWOBtlAPRG1ZEMG5Effavpt5wn+wQ//U=        2026-07-28T17:44:54+02:00       2024-10-25T16:15:34+02:00
        > stdout '24C30F9C9115E747'
        # Delete the gpg key from GH account (0.613s)
        > exec gh gpg-key delete --yes '24C30F9C9115E747'
        # Check the key is deleted (0.310s)
        > exec gh gpg-key list
        > ! stdout '24C30F9C9115E747'
        PASS
        
--- PASS: TestGPGKeys (0.00s)
    --- PASS: TestGPGKeys/gpg-key (2.06s)
PASS
ok      github.com/cli/cli/v2/acceptance        2.776s

@williammartin
Route gpg key requests through api.Client
d9f53ae 
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 605ad5d5-43b6-4332-b94b-6ae9b69bb3ff
@williammartin
williammartin marked this pull request as ready for review July 28, 2026 15:45
@williammartin
williammartin requested a review from a team as a code owner July 28, 2026 15:45
@williammartin
williammartin requested review from Copilot and tidy-dev and removed request for a team July 28, 2026 15:45
Copilot AI reviewed Jul 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the gh gpg-key subcommands’ REST interactions from direct http.Client request handling to the shared api.Client wrapper, aligning this command set with the ongoing effort to consolidate API behavior behind a common client.

Changes:

  • Refactored gpg-key add, gpg-key delete, and gpg-key list HTTP helpers to use api.NewClientFromHTTP(...).REST(...).
  • Added/expanded unit tests to validate error mapping (notably 404 -> insufficient scopes, and ordered handling of 422 cases for add).
  • Updated test stubs to return JSON bodies where the API client expects JSON unmarshalling.
Show a summary per file
File Description
pkg/cmd/gpg-key/add/http.go Switch upload request to api.Client REST call and preserve prior sentinel error mapping behavior.
pkg/cmd/gpg-key/add/add_test.go Add coverage for sentinel error mapping/order and adjust REST payload stubs to return JSON.
pkg/cmd/gpg-key/delete/http.go Switch delete/list helpers to api.Client REST calls.
pkg/cmd/gpg-key/delete/delete_test.go Add coverage for HTTP error surfacing from API client and fix go-gh HTTPError typing in stubs.
pkg/cmd/gpg-key/list/http.go Switch list helper to api.Client REST call and preserve 404 -> scopes error mapping.
pkg/cmd/gpg-key/list/list_test.go Add coverage for 404 mapping, generic HTTP error propagation, and user-scoped listing.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 0
  • Review effort level: Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@tidy-dev tidy-dev Awaiting requested review from tidy-dev tidy-dev is a code owner automatically assigned from cli/code-reviewers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@williammartin
Morty Proxy This is a proxified and sanitized view of the page, visit original site.