Implement support for action secrets #1402

martinssipenko · 2020-01-28T14:08:26Z

This adds support for managing github action secrets, an API feature that was recently announced.

Relates to: #1399

gmlewis · 2020-01-28T14:16:28Z

Thank you, @martinssipenko ! That was fast.
In the future, please comment on the issue first that you would like to own it so that we don't have multiple people duplicating work on the same issue.
I hope to review this tonight after work.

gmlewis

Thank you, @martinssipenko.
This is a good start, but needs some work before it will be ready to merge.
Also, please note that this PR will not close #1399... it only addresses one portion of the announcement... specifically, Secrets. I'll update #1399 with the expectations to close it.

github/actions_secrets.go

gmlewis · 2020-01-29T01:21:09Z

github/actions_secrets.go

+	UpdatedAt *Timestamp `json:"updated_at,omitempty"`
+}
+
+type secrets struct {


According to the docs, the full response looks like this:

[ { "total_count": 2, "secrets": [ { "name": "GH_TOKEN", "created_at": "2019-08-10T14:59:22Z", "updated_at": "2020-01-10T14:59:22Z" }, { "name": "GIST_ID", "created_at": "2020-01-10T10:59:22Z", "updated_at": "2020-01-11T11:59:22Z" } ] } ]

So let's please change this struct to be:

// Secrets represents one item from the ListSecrets response. type Secrets struct { TotalCount int `json:"total_count"` Secrets []*Secret `json:"secrets"` }

and then we will return a slice of Secrets ([]*Secrets) below in ListSecrets to match the GitHub API docs.

I think the API docs are wrong, when I make a request to list secrets I get an object, not an array, so I decided to return Secrets object, not a slice. I've reached out to GitHub support also regarding this API/doc discrepancy, more details in comment below.

gmlewis · 2020-01-29T01:22:04Z

github/actions_secrets.go

+		return nil, nil, err
+	}
+
+	secrets := new(secrets)


This should be:

var secrets []*Secrets

#1402 (comment)

github/actions_secrets.go

gmlewis · 2020-01-29T01:28:58Z

github/actions_secrets.go

+}
+
+type EncryptedSecret struct {
+	Name           *string `json:"-"`


Let's please make all three of these fields string instead of *string and remove the omitemptys, since all three fields are required.

github/actions_secrets.go

github/actions_secrets_test.go

martinssipenko · 2020-01-29T09:21:36Z

I've reached out to GitHub support with the following email:

Hi,

I'm currently working on implementing GitHub action secrets API as part for Golang GitHub client (#1402).

During this I've come across that the documentation for List Secrets endpoint states that the response will be a JSON array, however when I make a request to API I get JSON object as shown below. Is this a bug in documentation or in the API? Can you please fix it so they align?

{
  "total_count": 3,
  "secrets": [
    {
      "name": "A1",
      "created_at": "2020-01-29T00:24:44Z",
      "updated_at": "2020-01-29T00:24:44Z"
    },
    {
      "name": "A2",
      "created_at": "2020-01-29T00:24:50Z",
      "updated_at": "2020-01-29T00:24:50Z"
    },
    {
      "name": "A3",
      "created_at": "2020-01-29T00:24:56Z",
      "updated_at": "2020-01-29T00:24:56Z"
    }
  ]
}

Another thing I've noticed is that Get Secret endpoint documentation states that response might have pagination, however by looking at response its unlikely it will have it given the response in a single object. Furthermore the endpoint returns a single secret, so there is nothing to paginate on. Could this also be a bug in documentation?

Hoping to hear back from you soon,
Martins

martinssipenko · 2020-01-29T09:27:38Z

Another thing I was thinking about - should this library provide a helper to encrypt a secret (obtain EcryptedSecret) that could later be used in CreateOrUpdated method? Or you think it would be better to leave it out the the end user?

gmlewis · 2020-01-29T13:04:55Z

Excellent... thank you very much, @martinssipenko for sending the email to GitHub Tech Support.

Another thing I was thinking about - should this library provide a helper to encrypt a secret (obtain EcryptedSecret) that could later be used in CreateOrUpdated method? Or you think it would be better to leave it out the the end user?

We have tried to keep external dependencies in this repo to a bare minimum and when there was an option to support GPG encryption, we chose to allow the user of this client library to implement it any way they wished without binding them to a particular solution.

Since there appear to be two possible Go libraries listed on the webpage that GitHub linked to for LibSodium, I'm thinking
that we should follow the same route here and allow the user to choose their own method of
encryption then passing in the value as you have done in your PR.

Thanks again, @martinssipenko ! I will try to take another look at your updates after work tonight.

gmlewis · 2020-01-29T13:12:51Z

Having said that, I think we could add some comments and possibly some links suggesting how Go users of this client library could encrypt the fields such that they would work with GitHub (if you haven't already).

gmlewis

Thank you, @martinssipenko!
Just a few minor tweaks, please, then I think we will be ready for a second review.

github/actions_secrets.go

gmlewis · 2020-01-30T02:50:38Z

github/actions_secrets.go

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

+// EncryptedSecret represents represents a secret that is encrypted using public key.
+//
+// The value of EncryptedValue must value for your secret, encrypted with


I'm having troubles parsing "The value of EncryptedValue must value for your secret". Can you please fix this comment?

I change it slightly, is it better now?

martinssipenko · 2020-01-30T05:35:10Z

Here is a response from GitHub support:

Hi Martins -

Thanks for the report!

During this I've come across that the documentation for List Secrets endpoint states that the response will be a JSON array, however when I make a request to API I get JSON object as shown below. Is this a bug in documentation or in the API? Can you please fix it so they align?

We confirmed the discrepancy with some manual testing so we'll mention this to our documentation team and our Actions team. I can't promise when we might get confirmation about whether this an issue in the API vs. the documentation but we'll followup as soon as there's more information.

Another thing I've noticed is that Get Secret endpoint documentation states that response might have pagination, however by looking at response its unlikely it will have it given the response in a single object. Furthermore the endpoint returns a single secret, so there is nothing to paginate on. Could this also be a bug in documentation?

Ahhh yes - this does seem like a documentation issue since that endpoint returns single secret so I'll mention this to the team as well.

Thanks again,
Robert

gmlewis · 2020-01-30T05:39:45Z

github/actions_secrets.go

 //
-// The value of EncryptedValue must value for your secret, encrypted with
+// The value of EncryptedValue must value of your secret, encrypted with


How about: The value of EncryptedValue must be your secret, ...?

I'm sorry, I need to learn to read :) should be fixed now.

Hey, I only know one language... you are doing awesome... thanks so much!!!

gmlewis

Thank you, @martinssipenko!
LGTM.

Awaiting second LGTM before merging.

gmlewis · 2020-01-30T05:44:01Z

Thanks again for contacting GitHub Tech Support and reporting back!
Improving their documentation will help all users of their API!!!

martinssipenko · 2020-01-30T05:45:41Z

This is my pleasure, very quick and constructive feedback really helps!

martinssipenko · 2020-02-05T06:06:38Z

@gauntface could take a look at this PR?

gmlewis · 2020-02-09T01:50:15Z

@joshuabezaleel - if you are comfortable, please feel free to review this PR and then we can hopefully get it merged after you approve it.

gmlewis · 2020-02-10T01:03:36Z

github/actions_secrets.go

+// without revealing their encrypted values.
+//
+// GitHub API docs: https://developer.github.com/v3/actions/secrets/#list-secrets-for-a-repository
+func (s *ActionsService) ListSecrets(ctx context.Context, owner, repo string, opt *ListOptions) (*Secrets, *Response, error) {


When you get a chance, could you please change opt to opts to be consistent with #1417?
(Here and anywhere else you find it in this PR, please.)

joshuabezaleel · 2020-02-10T03:08:43Z

missed @gmlewis 's mention, so sorry.
LGTM. Thank you for a really great PR and starting this, @martinssipenko ! I also truly appreciate your effort on contacting the GitHub's support team. Learnt so much and this is also definitely a great starting point of mine.

gmlewis · 2020-02-10T03:20:56Z

Thank you, @joshuabezaleel!
I'm going to go ahead and merge this... and then once this is merged, can you please fix up all the opt to opts in #1411? I would greatly appreciate it!

martinssipenko · 2020-02-28T06:11:31Z

@gmlewis I got another response from GitHub support:

Hi Martins -

One last followup for you about your second report:

Another thing I've noticed is that Get Secret endpoint documentation states that response might have pagination, however by looking at response its unlikely it will have it given the response in a single object. Furthermore the endpoint returns a single secret, so there is nothing to paginate on. Could this also be a bug in documentation?

Our documentation folks fixed this in the last few days (sorry I missed when the fix went live) and the pagination has been removed since like you mentioned, there's nothing to paginate on when getting a single secret.

Thanks again for the reports - cheers,
Robert

martinssipenko · 2020-02-28T06:15:26Z

I've reached out to GitHub support about the response issue mentioned in #1399 (comment):

Hi,

One more thing came up - documentation suggests that "Create or update a secret for a repository" request will return 201 response when creating a secret and 204 when updating, however, despite what the docs say I do get 204 on both create and update.

Could this be another documentation bug?

Martins

gmlewis · 2020-02-28T14:27:39Z

Thank you so much, @martinssipenko!
This improves the GitHub developer experience for everyone by making the documentation better.

martinssipenko · 2020-05-19T07:59:13Z

Got another message from github support regarding the last issue:

Just wanted to followup about the last issue you mentioned:

One more thing came up - documentation suggests that "Create or update a secret for a repository" request will return 201 response when creating a secret and 204 when updating, however, despite what the docs say I do get 204 on both create and update.

Could this be another documentation bug?

This ended up not being a documentation issue in particular and the Actions team shipped an update late last week so that creating a secret now returns a 201 as documented (and updating a secret continues to return a 204).

gmlewis · 2020-05-19T11:36:54Z

Thank you for the update, @martinssipenko !
So it sounds like they fixed it and no more changes are needed on our side.
Awesome! 🎉

Relates to: google#1399.

Implement support for action secrets

5ed8db9

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

martinssipenko requested a review from gmlewis January 28, 2020 14:10

martinssipenko mentioned this pull request Jan 28, 2020

Feature request: Support secret creation for github actions integrations/terraform-provider-github#271

Closed

Added missing delete test

1908e4b

gmlewis requested changes Jan 29, 2020

View reviewed changes

gmlewis mentioned this pull request Jan 29, 2020

Support GitHub Actions API #1399

Closed

Implement PR feedback

00f2f03

Added info on how to encrypt secrets

bbff48b

gmlewis requested changes Jan 30, 2020

View reviewed changes

gmlewis requested a review from gauntface January 30, 2020 02:52

Implement 2nd round of feedback

6367ea6

martinssipenko requested a review from gmlewis January 30, 2020 05:36

gmlewis reviewed Jan 30, 2020

View reviewed changes

fix my broken english

bbe3d53

gmlewis approved these changes Jan 30, 2020

View reviewed changes

joshuabezaleel mentioned this pull request Feb 5, 2020

Implement support for action workflows #1411

Merged

gmlewis reviewed Feb 10, 2020

View reviewed changes

gmlewis merged commit 7d28d53 into google:master Feb 10, 2020

martinssipenko deleted the actions branch February 10, 2020 04:08

martinssipenko mentioned this pull request Feb 11, 2020

Update go-github to v29. integrations/terraform-provider-github#342

Merged

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

Implement support for action secrets (google#1402)

a2a0e2d

Relates to: google#1399.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement support for action secrets #1402

Implement support for action secrets #1402

martinssipenko commented Jan 28, 2020 •

edited by gmlewis

Loading

gmlewis commented Jan 28, 2020

gmlewis left a comment

gmlewis Jan 29, 2020

martinssipenko Jan 29, 2020 •

edited

Loading

gmlewis Jan 29, 2020

martinssipenko Jan 29, 2020

gmlewis Jan 29, 2020

martinssipenko commented Jan 29, 2020

martinssipenko commented Jan 29, 2020

gmlewis commented Jan 29, 2020

gmlewis commented Jan 29, 2020

gmlewis left a comment

gmlewis Jan 30, 2020

martinssipenko Jan 30, 2020

martinssipenko commented Jan 30, 2020

gmlewis Jan 30, 2020 •

edited

Loading

martinssipenko Jan 30, 2020 •

edited

Loading

gmlewis Jan 30, 2020

gmlewis left a comment

gmlewis commented Jan 30, 2020

martinssipenko commented Jan 30, 2020

martinssipenko commented Feb 5, 2020

gmlewis commented Feb 9, 2020

gmlewis Feb 10, 2020

joshuabezaleel commented Feb 10, 2020 •

edited

Loading

gmlewis commented Feb 10, 2020

martinssipenko commented Feb 28, 2020

martinssipenko commented Feb 28, 2020

gmlewis commented Feb 28, 2020

martinssipenko commented May 19, 2020

gmlewis commented May 19, 2020

Implement support for action secrets #1402

Implement support for action secrets #1402

Conversation

martinssipenko commented Jan 28, 2020 • edited by gmlewis Loading

gmlewis commented Jan 28, 2020

gmlewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinssipenko Jan 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinssipenko commented Jan 29, 2020

martinssipenko commented Jan 29, 2020

gmlewis commented Jan 29, 2020

gmlewis commented Jan 29, 2020

gmlewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinssipenko commented Jan 30, 2020

gmlewis Jan 30, 2020 • edited Loading

Choose a reason for hiding this comment

martinssipenko Jan 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmlewis left a comment

Choose a reason for hiding this comment

gmlewis commented Jan 30, 2020

martinssipenko commented Jan 30, 2020

martinssipenko commented Feb 5, 2020

gmlewis commented Feb 9, 2020

Choose a reason for hiding this comment

joshuabezaleel commented Feb 10, 2020 • edited Loading

gmlewis commented Feb 10, 2020

martinssipenko commented Feb 28, 2020

martinssipenko commented Feb 28, 2020

gmlewis commented Feb 28, 2020

martinssipenko commented May 19, 2020

gmlewis commented May 19, 2020

martinssipenko commented Jan 28, 2020 •

edited by gmlewis

Loading

martinssipenko Jan 29, 2020 •

edited

Loading

gmlewis Jan 30, 2020 •

edited

Loading

martinssipenko Jan 30, 2020 •

edited

Loading

joshuabezaleel commented Feb 10, 2020 •

edited

Loading