Add organization secret API support #1535

nightlark · 2020-05-22T15:11:11Z

Related to #1525 and #1532.

Adds organization secrets https://developer.github.com/v3/actions/secrets/#get-an-organization-public-key and updates the repository secret actions API (probably would match how they would be named if org support had been in from the beginning, though it would be considered a breaking change).

Some possible issues:

Checking with GitHub support if parameter selected repository ids should be an array of integers instead of array of strings. I think its been ~1 week?
Naming of the functions using the API endpoint header results in very long names.
Handling of extra values in Secret and EncryptedSecret for organization secrets that repository secrets don't have: does omitempty make sense or should they be separate structs?
Should there be an underlying function used for both organization and repository secrets that takes the base url as an argument? Quite a bit of duplication.
Repository id (and id array) argument using int64 (or int64[]), or use the Repository type for the argument?
Handling of selected repository IDs parameter as a type that wraps int64[], so that it gets marshaled to json correctly for SetSelectedRepositoriesForOrganizationSecret or is there a cleaner way to do this and use int64[] directly?

codecov · 2020-05-22T15:12:35Z

Codecov Report

Merging #1535 into master will decrease coverage by 0.15%.
The diff coverage is 58.00%.

@@            Coverage Diff             @@
##           master    #1535      +/-   ##
==========================================
- Coverage   68.03%   67.87%   -0.16%     
==========================================
  Files          94       94              
  Lines        8528     8654     +126     
==========================================
+ Hits         5802     5874      +72     
- Misses       1844     1880      +36     
- Partials      882      900      +18

Impacted Files	Coverage Δ
github/actions_secrets.go	`54.00% <58.00%> (+2.21%)`	⬆️
github/actions_workflow_runs.go	`57.57% <0.00%> (-0.57%)`	⬇️
github/actions_workflows.go	`61.11% <0.00%> (+3.96%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45c5021...48c5e83. Read the comment docs.

gmlewis

This is looking great, @nightlark ! Thank you for all the research and obvious effort that you have put into this... I really like it.

I've made a few suggestions, please let me know what you think.

gmlewis · 2020-05-23T12:00:32Z

github/actions_secrets.go

@@ -92,6 +113,11 @@ func (s *ActionsService) GetSecret(ctx context.Context, owner, repo, name string
 	return secret, resp, nil
 }

+// SelectedRepositoryIDs are the repository IDs that have access to the secret.
+type SelectedRepositoryIDs struct {


Instead of making this a struct, how about if we make it a type so that it can be used more easily both within a response and as a request parameter?

type SelectedRepoIDs []int64

gmlewis · 2020-05-23T12:02:22Z

github/actions_secrets.go

@@ -101,12 +127,14 @@ type EncryptedSecret struct {
 	Name           string `json:"-"`
 	KeyID          string `json:"key_id"`
 	EncryptedValue string `json:"encrypted_value"`
+	Visibility     string `json:"visibility,omitempty"`
+	SelectedRepositoryIDs


Then this would become:

SelectedRepositoryIDs SelectedRepoIDs `json:"selected_repository_ids,omitempty"`

Note the long field name to correspond to the long JSON parameter name but the shorter type name.

gmlewis · 2020-05-23T12:05:18Z

github/actions_secrets.go

@@ -16,10 +16,10 @@ type PublicKey struct {
 	Key   *string `json:"key"`
 }

-// GetPublicKey gets a public key that should be used for secret encryption.
+// GetRepositoryPublicKey gets a public key that should be used for secret encryption.


When names get impractically long (not that this one is, but down below... and you mentioned it in the issue),
in other places in this repo we have chosen to shorten "Repository" to "Repo" and "Organization" to "Org".

I think that would work well here... but let's go through these modified files and be consistent with the renaming since this is a breaking-API-change PR anyway.

So this would be GetRepoPublicKey.

gmlewis · 2020-05-23T12:05:41Z

github/actions_secrets.go

@@ -35,11 +35,32 @@ func (s *ActionsService) GetPublicKey(ctx context.Context, owner, repo string) (
 	return pubKey, resp, nil
 }

+// GetOrganizationPublicKey gets a public key that should be used for secret encryption.


GetOrgPublicKey

gmlewis · 2020-05-23T12:06:02Z

github/actions_secrets.go

@@ -48,12 +69,12 @@ type Secrets struct {
 	Secrets    []*Secret `json:"secrets"`
 }

-// ListSecrets lists all secrets available in a repository
+// ListRepositorySecrets lists all secrets available in a repository


ListRepoSecrets

gmlewis · 2020-05-23T12:23:05Z

github/actions_secrets.go

+	return result, resp, nil
+}
+
+// SetSelectedRepositoriesForOrganizationSecret sets the repositories that have access to a secret.


SetSelectedReposForOrgSecret

gmlewis · 2020-05-23T12:23:34Z

github/actions_secrets.go

+	return s.client.Do(ctx, req, nil)
+}
+
+// AddSelectedRepositoryToOrganizationSecret adds a repository to an organization secret.


AddSelectedRepoToOrgSecret

gmlewis · 2020-05-23T12:24:01Z

github/actions_secrets.go

+	return s.client.Do(ctx, req, nil)
+}
+
+// RemoveSelectedRepositoryFromOrganizationSecret removes a repository from an organization secret.


RemoveSelectedRepoFromOrgSecret

gmlewis · 2020-05-23T12:26:24Z

github/actions_secrets.go

+// GitHub API docs: https://developer.github.com/v3/actions/secrets/#set-selected-repositories-for-an-organization-secret
+func (s *ActionsService) SetSelectedRepositoriesForOrganizationSecret(ctx context.Context, org, name string, ids SelectedRepositoryIDs) (*Response, error) {
+	u := fmt.Sprintf("orgs/%v/actions/secrets/%v/repositories", org, name)
+	req, err := s.client.NewRequest("PUT", u, ids)


Because of my type ... []int64 suggestion above, you will have to wrap this into a private struct with the appropriate json tag to make this work again... but I think it still makes the API cleaner and easier for users of this repo, so I think it is a better way to go.

gmlewis · 2020-05-23T12:26:47Z

github/actions_secrets.go

+	return s.client.Do(ctx, req, nil)
+}
+
+// DeleteOrganizationSecret deletes a secret in an organization using the secret name.


DeleteOrgSecret

gmlewis

Thank you, @nightlark!
LGTM.

Awaiting second LGTM before merging.

wesleimp

LGTM 👌

gmlewis · 2020-05-28T01:50:40Z

Thank you, @wesleimp!
Merging.

z0mbix · 2020-05-28T11:51:35Z

Thanks for all the work on this, any idea when this may get released?

gmlewis · 2020-05-28T12:02:33Z

We could put together a release today.
(We switched to an on-demand model.)

gmlewis · 2020-05-28T12:05:55Z

Could you, @z0mbix or you, @andrew-waters please review #1534 for me to get the one outstanding breaking API change into the release before I bump the major version again? Thank you!

andrew-waters · 2020-05-28T12:09:18Z

I'm unable to review here @gmlewis, I'm not a maintainer - I'm here for https://github.com/terraform-providers/terraform-provider-github/issues/468

gmlewis · 2020-05-28T12:11:10Z

Thank you, @andrew-waters - just to clarify, this repo is completely run by volunteers and all our code reviewers are Go programmers who are also volunteers. So I welcome your reviews, as that would help me unblock this next release. 😄

z0mbix · 2020-05-28T12:13:05Z

LGTM, I’ve approved it.

andrew-waters · 2020-05-28T12:14:03Z

Ah ok, thanks @gmlewis - I hadn't realised and didn't fancy stepping on toes here 😄 . I've added a second approval too

gmlewis · 2020-05-28T12:35:48Z

@z0mbix - we have now released v32: https://github.com/google/go-github/releases/tag/v32.0.0
Thanks for your help, everyone.

z0mbix · 2020-05-28T12:39:06Z

Thank you! 🙇

gmlewis · 2020-05-28T12:55:19Z

You're welcome! Now, if we could just get the Go proxy to recognize it so we can start using it!

https://pkg.go.dev/mod/github.com/google/go-github/v32
https://proxy.golang.org/github.com/google/go-github/v32/@v/v32.0.0.info

gmlewis · 2020-05-28T13:04:58Z

OK, go get just started working. So it looks like the Go proxy takes about 30 minutes to ingest a new release. Good to know. 😄

Fixes: google#1525.

nightlark added 2 commits May 14, 2020 18:39

Update repository secret API functions

6200ffb

Add organization secret API endpoints

5599c9b

googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label May 22, 2020

cj-taylor mentioned this pull request May 22, 2020

Organization Secrets #1532

Closed

9 tasks

gmlewis requested changes May 23, 2020

View reviewed changes

Changes based on code review suggestions - function/type renaming

48c5e83

gmlewis approved these changes May 23, 2020

View reviewed changes

gmlewis requested review from gauntface and wesleimp May 23, 2020 19:25

wesleimp approved these changes May 27, 2020

View reviewed changes

gmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label May 28, 2020

gmlewis merged commit 8d28b93 into google:master May 28, 2020

nightlark deleted the organization-secrets branch August 15, 2020 15:31

n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020

Add organization secret API support (google#1535)

f29827e

Fixes: google#1525.

This was referenced Jun 30, 2021

chore(deps): update all (major) redhat-developer/app-services-cli#765

Closed

fix(deps): update module github.com/google/go-github/v31 to v36 - autoclosed motemen/prchecklist#208

Closed

Add organization secret API support #1535

Add organization secret API support #1535

Uh oh!

Conversation

nightlark commented May 22, 2020

Uh oh!

codecov bot commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

wesleimp left a comment

Choose a reason for hiding this comment

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

z0mbix commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

andrew-waters commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

z0mbix commented May 28, 2020

Uh oh!

andrew-waters commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

z0mbix commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

gmlewis commented May 28, 2020

Uh oh!

Uh oh!

codecov bot commented May 22, 2020 •

edited

Loading