Skip to content

refactor!: Split Key request bodies into CreateDeployKeyRequest, CreateUserKeyRequest and CreateSSHSigningKeyRequest and pass by value - #4477

Open
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-key-value-params
Open

refactor!: Split Key request bodies into CreateDeployKeyRequest, CreateUserKeyRequest and CreateSSHSigningKeyRequest and pass by value#4477
JamBalaya56562 wants to merge 2 commits into
google:masterfrom
JamBalaya56562:refactor/3644-key-value-params

Conversation

@JamBalaya56562

Copy link
Copy Markdown
Contributor

Continues the request-body-by-value work in #3644, this time for the three key-creation endpoints that shared the Key response type as their request body.

Per the docs, each endpoint accepts only a small request schema — the other six Key fields (id, url, verified, created_at, added_by, last_used) are server-generated — and the schemas differ per resource:

Method Endpoint key title read_only
RepositoriesService.CreateKey Create a deploy key required optional optional
UsersService.CreateKey Create a public SSH key required optional
UsersService.CreateSSHSigningKey Create a SSH signing key required optional

So this splits the body into three per-resource request types — CreateDeployKeyRequest (with ReadOnly), CreateUserKeyRequest and CreateSSHSigningKeyRequest — following the same approach as #4406/#4425/#4432/#4434/#4444. Key is a non-pointer string in all three since it's required everywhere. The methods keep their names (the Create verb already matches the docs, and renaming only the create methods would break consistency with the surrounding Get/List/DeleteKey families).

Also fixed in passing, since the signature change touches the exact lines: the failure case in TestUsersService_CreateSSHSigningKey had a pre-existing copy-paste bug — it called Users.CreateKey (a different method) with methodName = "CreateKey"; it now exercises CreateSSHSigningKey as intended.

Notes:

  • The Key response type is unchanged — it stays the response for ListKeys/GetKey/both create methods, so its fields keep pointer semantics. SSHSigningKey is untouched.
  • test/integration/users_test.go builds under the root module with -tags integration, so its Users.CreateKey call is updated too (verified with go vet -tags integration).

Verified with go build ./..., go vet -tags integration ./test/integration/, gofmt, the full ./github/ test suite (all three methods and the seven generated accessors at 100%), and custom-gcl (no paramcheck findings after removing the allowlist entry).

Updates #3644

BREAKING CHANGE: RepositoriesService.CreateKey, UsersService.CreateKey and UsersService.CreateSSHSigningKey now take new CreateDeployKeyRequest, CreateUserKeyRequest and CreateSSHSigningKeyRequest (with non-pointer Key) by value instead of *Key.

cc @jvm986 — flagging for #3644 coordination; this covers the Key type (repos/users key endpoints), so it doesn't overlap with #4475.

…`CreateUserKeyRequest` and `CreateSSHSigningKeyRequest` and pass by value

Three create methods across three resources reused the 9-field Key
response type as their request body, but each endpoint accepts only key
(required) and title, plus read_only for deploy keys — the other six
fields are server-generated. The new per-resource request types model
each schema exactly, with a non-pointer Key, and are passed by value.

The Key response type stays unchanged, and its entry is removed from the
.golangci.yml allowlist. This also fixes a pre-existing copy-paste bug in
TestUsersService_CreateSSHSigningKey, whose failure case called
Users.CreateKey instead of Users.CreateSSHSigningKey.

BREAKING CHANGE: RepositoriesService.CreateKey, UsersService.CreateKey and UsersService.CreateSSHSigningKey now take new CreateDeployKeyRequest, CreateUserKeyRequest and CreateSSHSigningKeyRequest (with non-pointer Key) by value instead of *Key.
@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Aug 21, 2026

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @Not-Dhananjay-Mishra

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.53%. Comparing base (c4176b1) to head (188c6ab).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4477      +/-   ##
==========================================
+ Coverage   98.51%   98.53%   +0.01%     
==========================================
  Files         195      195              
  Lines       17691    17691              
==========================================
+ Hits        17429    17431       +2     
+ Misses        262      260       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread github/repos_keys.go
The deploy-key response schema includes an enabled boolean that was
missing from the Key struct. It is deploy-key specific (the user SSH key
response doesn't have it), matching how Key already carries other
deploy-only fields such as added_by, so it's an optional pointer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). NeedsReview PR is awaiting a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants