Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented GitHub Actions Secrets API for both Organization and Repository #2221

Closed
wants to merge 58 commits into from

Conversation

mptolly-takeda
Copy link
Contributor

I implemented the GitHub Actions Secrets API calls within octokit.net splitting the clients into repository and organization secrets clients (link).

I also created an intermediate client class for repository and organization actions that sit off of the RepositoryClient and OrganizationClient respectively to enable future implementation of actions APIs.

fixed wrong Ctor unit test
fixed broken unit test for repository secrets client
@IAmHughes
Copy link
Contributor

#2221 (comment)

I'll add a commit after I finish this initial review updating all the developer.github.com and help.github.com links I find in these changes

Okay so this is actually taking a lot longer than I thought it would. The process is:

  1. If file has developer.github.com or help.github.com links, go to that page and follow the redirect message at the top
  2. Copy/paste the new redirect URL (may need to re-do the anchor link) into the file and commit

I've done quite a few, @mptolly-takeda please continue to look for them. If you have your branch local (since it's a fork), it may be easier to find/replace.

@mptolly-takeda
Copy link
Contributor Author

@IAmHughes I updated the documentation links within the 4 clients that I created

mptolly-takeda and others added 2 commits August 12, 2020 15:43
Removing line for consistency.

Co-authored-by: Thomas Hughes <[email protected]>
Removing line for consistency.

Co-authored-by: Thomas Hughes <[email protected]>
}
#endif

private static async Task<Repository> CreateRepoIfNotExists(IGitHubClient github, string name)
Copy link
Member

@shiftkey shiftkey Oct 24, 2020

Choose a reason for hiding this comment

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

As an alternative for this, there are existing test helpers to create a new repository with a timestamp suffix to prevent clashing:

var repoName = Helper.MakeNameWithTimestamp("public-repo");
var context = await github.CreateRepositoryContext(new NewRepository(repoName));

This lends itself to creating and tearing down repositories within the context of the test, rather than potentially clobbering existing data. RepositoryContext implements IDisposable, which handles deleting the repository after the test completes.

The other upside here is that context has handy RepositoryOwner and RepositoryName properties that you can then use in your test, rather than having global ORG and REPO names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be in the new commit

@shiftkey
Copy link
Member

@mptolly-takeda @IAmHughes thanks for being patient with me while I try and make time to review this. This is looking pretty good!

Looking through the changes, I think the only real topic worth unpacking is the integration tests. For the sake of simplicity, let's not stress about granularity of covering everything . Let's instead focus on testing scenarios in a way that we can easily setup and teardown, without depending on the test account being in a certain state.

@mcgear
Copy link

mcgear commented May 8, 2021

Hey, wanted to see if there was any movement here?

@damianh
Copy link

damianh commented Jun 26, 2021

I was looking for this functionality so I could deploy temporary AWS credendials GitHub org/repo secrets and was wondering where it was. It would be great if this awesome PR could get landed.

:shipit:

@isaacrlevin
Copy link

@shiftkey any timeline for merging this in?

@nickfloyd
Copy link
Contributor

@mptolly-takeda I have to apologize. I jumped the gun on trying to resolve a merge conflict on this and it ended up inadvertently adding an unsupported dep to the integration tests project. I did it through the GH interface and I cannot revert it on your branch because I do not have access. I'd be glad to help get this in a place where we could merge it in - this is such a good chance.

If possible, would you mind reverting the commit I did through the interface and resolving the merge conflict so that we can try to get your changeset merged into main? Thank you again for the changes and apologies for the trouble here.

@kfcampbell
Copy link
Member

@mptolly-takeda I'd like to do a friendly bump here. Is this something you have an interest in picking back up? If not, I could branch off of b69eba1 and submit a similar PR.

@kfcampbell
Copy link
Member

Closing in favor of #2598.

@kfcampbell kfcampbell closed this Oct 19, 2022
kfcampbell added a commit that referenced this pull request Oct 20, 2022
…ion and Repository (#2598)

* created the interface and models for the repository secrets client

* created a repository actions client to sit between repository and secrets for future extensibility

* created the repository secret client and supporting objects to enable data transfer

* created object for create or update secret body and made fixes to pass unit tests

* created repository action unit tests

* created unit tests for RepositorySecretsClient

* removed set from secrets interface

* fixed docs and added observable actions client

* added Actions to repository client

* created IObservable repository secrets client

* fixed property in wrong interface
fixed wrong Ctor unit test

* created repository decrets reactive tests and clients

* created organization actions and scerets classes and made them available through the oprganizations client

* fixed intellisense text

* removed uneeded getall call after return type change

* created organization secret client and classes to support it

* created the observable org secrets client and fixed a typo in a method name

* added more ensure checks

* removed unused xml doc setting

* created the unit tests for the organization secrets client
fixed broken unit test for repository secrets client

* created observable organization actions and secrets client unit tests

* added sodium.core to the integration tests to test secret creation

* fixed keyid type

* added actions client integration test classes (empty since the class currently doesn't have any native methods)

* fixed deserialization issue

* changed property name for deserialization issues

* added doc for repoid on orginzation secrets url generator

* created integration tests for repository and organization secrets

* changed how return occurs for setting list of repos for secret

* fixed some names and removed reset org name

* created integration tests for observable org secrets client

* removed  default org value

* created the integration tests for the observable repository secrets client

* removed default owner project value

* fixed unit tests

* Update links to new docs site

* Update doc links to new docs site

* Update docs links to new docs site

* Fix doc link to point to new docs site

* Update links to new docs site

* Update doc links to new docs site

* Update docs links

* Update docs

* Update docs

* Update doc links

* Update docs

* Update doc links

* Update doc links

* Update doc links

* updated documentation links in actions and secrets clients

* Update Octokit/Models/Response/SecretsPublicKey.cs

Removing line for consistency.

Co-authored-by: Thomas Hughes <[email protected]>

* Update Octokit/Models/Response/RepositorySecret.cs

Removing line for consistency.

Co-authored-by: Thomas Hughes <[email protected]>

* set default owner and repo

* switched to using the Helper.Organization from a ORG constant set at the top of the file

* swapped out variable at top of file for the Helper.Organization property

* switched to helper method to create new repositories

* Protected setters --> private setters in response models

* RepositorySecret needs protected setters

Co-authored-by: Mike Tolly <[email protected]>
Co-authored-by: Thomas Hughes <[email protected]>
Co-authored-by: mptolly-takeda <[email protected]>
@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants