feature: allow adding named gpg keys #6993
Conversation
cliAutomation
added
the
external
|
Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message. |
There was a problem hiding this comment.
@rpadaki Thanks for opening this PR. This is a feature I have been wanting for awhile. I am surprised we didn't have an issue open for it already.
For now let's not add the TITLE column to the list command. The reason for this is that it is a breaking change due to changing the order of the columns. If a script is depending on the current order they will break.
One approach we could take in a follow up PR to make this change not breaking would be to add the TITLE column to the beginning of TTY output and to the end of non-TTY output. We have done that in the past and it has worked out okay.
pkg/cmd/gpg-key/add/http.go
Outdated
| @@ -25,6 +25,7 @@ func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader) e | |||
|
|
|||
| payload := map[string]string{ | |||
| "armored_public_key": string(keyBytes), | |||
| "name": title, | |||
There was a problem hiding this comment.
If title is blank will the request get rejected by the server? Or will the key still be added? Will there be an automatically generated name?
There was a problem hiding this comment.
As per my local testing, if the title is blank, this does not get rejected by the server, and the key is properly added without a title. Nothing is auto-generated by the backend.
If we're scared about the backend changing its behavior on this, maybe we could only include the name field conditional on title being nonempty, to be extra safe?
Just like SSH keys auto-generate titles, the backend could also auto-generate titles for GPG keys based on the time they were added and the email address associated with the key. Then, there would be no difference between how SSH and GPG would need to be handled. But alas, we live in a world where GPG keys sometimes have titles but SSH keys always have titles.
Got it. I'll revert my second commit for the time being. |
This reverts commit 1df2a97.
rpadaki
changed the title
Set up
giton a new machine after some time and ran into some missing functionality I thought I'd add.Context
GitHub (as I discovered an hour ago) supports naming GPG keys! When adding them mannually in the Settings page, there is an optional box for a
Title, similar to that for SSH keys. However,gh gpg-keysubcommands don't take advantage of this, unlike the analogousgh ssh-keyones.Commits
This PR consists of two commits:
-tor--titleto the commandgh gpg-key add. This parameter gets set as thenamekey in the GPG key addition request.The second commit parses makesgh gpg-key listaware of these titles, by reading them from the JSON response and prepending aTITLEcolumn to the table.Details
The REST API schemata for adding and listing GPG keys uses "name" instead of "title". However, the settings page UI when adding keys says "Title" for both SSH and GPG keys. In order for
gh gpg-keyandgh ssh-keyto match each other, and in order to match the GitHub UI, I decided to go with "title" as the user-facing name for this value.SSH key titles are auto-generated from the SSH key contents if not provided; therefore, they are always present when listing key names. This is different for GPG keys -- old keys and all new keys added without a title will never be assigned one. This makes the table for
gh gpg-key listpotentially confusing. Is there a visual indicator we should be using for an untitled key (e.g. putting-in that table column)? Or should we simply remove my second commit from this PR? (Edit: I have removed this second commit)