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

Support creation of Dependabot Organization and Repository Secrets #2874

Merged
merged 25 commits into from Mar 21, 2024

Conversation

thomascrowley
Copy link
Contributor

@thomascrowley thomascrowley commented Jan 17, 2024

Inline with #2284, this change allows the creation and interactions with both "actions" and "dependabot" secrets both on the organization or a repository.

To achieve this I have added an additional parameter to all related methods to allow you to choose between "actions" or "dependabot" secrets. The default value is set to "actions" to help with back compatibility.

@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2024

Codecov Report

Attention: Patch coverage is 92.00000% with 2 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@9e09245). Click here to learn what that means.

❗ Current head c72f22b differs from pull request most recent head 8ad1bf7. Consider uploading reports for the commit 8ad1bf7 to get more accurate results

Files Patch % Lines
github/Repository.py 85.71% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2874   +/-   ##
=======================================
  Coverage        ?   96.73%           
=======================================
  Files           ?      147           
  Lines           ?    14895           
  Branches        ?        0           
=======================================
  Hits            ?    14408           
  Misses          ?      487           
  Partials        ?        0           

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

@thomascrowley thomascrowley marked this pull request as ready for review January 18, 2024 13:36
@thomascrowley thomascrowley changed the title Enable the creation of Dependabot Organization Secrets Enable the creation of Dependabot Organization and Repository Secrets Jan 18, 2024
@thomascrowley thomascrowley changed the title Enable the creation of Dependabot Organization and Repository Secrets feat: Enable the creation of Dependabot Organization and Repository Secrets Jan 18, 2024
tests/Issue2284.py Outdated Show resolved Hide resolved
github/Organization.py Show resolved Hide resolved
github/Organization.py Outdated Show resolved Hide resolved
@EnricoMi EnricoMi changed the title feat: Enable the creation of Dependabot Organization and Repository Secrets Support creation of Dependabot Organization and Repository Secrets Jan 22, 2024
github/OrganizationSecret.py Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
github/Organization.py Show resolved Hide resolved
github/Organization.py Show resolved Hide resolved
github/Repository.py Show resolved Hide resolved
@thomascrowley
Copy link
Contributor Author

Hi @EnricoMi please can you re-review. I believe we've now addressed all your comments.

github/Organization.py Outdated Show resolved Hide resolved
github/Organization.py Outdated Show resolved Hide resolved
github/Repository.py Show resolved Hide resolved
github/Repository.py Show resolved Hide resolved
github/Repository.py Show resolved Hide resolved
tests/Organization.py Show resolved Hide resolved
tests/Organization.py Outdated Show resolved Hide resolved
Comment on lines 589 to 594
def testOrgGetSecretAssertion(self):
self.org = self.g.get_organization("demoorg")
try:
self.org.get_secret(secret_name="splat", secret_type="supersecret")
except AssertionError:
assert True
Copy link
Collaborator

Choose a reason for hiding this comment

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

What exactly are you testing here?

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 test confirms that if you pass in a secret_type that isn't "dependabot" or "actions" it successfully throws an error.

I added this because codecov flagged this as untested

tests/Repository.py Show resolved Hide resolved
Comment on lines 1973 to 1978
def testRepoGetSecretAssertion(self):
repo = self.g.get_repo("demoorg/demo-repo-1")
try:
repo.get_secret(secret_name="splat", secret_type="supersecret")
except AssertionError:
assert True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, what does this test assert?

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 test confirms that if you pass in a secret_type that isn't "dependabot" or "actions" it successfully throws an error.

I added this because codecov flagged this as untested

@thomascrowley
Copy link
Contributor Author

hi @EnricoMi is this ok to be merged now?

smkillen and others added 4 commits February 27, 2024 11:53
* Moving tests to respective files

* Correcting typo

* Correcting typo

* Creating ReplayData and Fixing tests, updating Organization.py to take secret_type on secrets call
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
@thomascrowley
Copy link
Contributor Author

Hello @EnricoMi I believe we've addressed all comments, can this be merged in?

Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

Can you adopt this for all exception assertions, please?

tests/Organization.py Outdated Show resolved Hide resolved
@jesdi
Copy link

jesdi commented Mar 4, 2024

Hi guys, thanks for working on this, we also would benefit a lot from adding a better support for Dependabot's secrets. 🙏

@jesdi
Copy link

jesdi commented Mar 4, 2024

I think there is missing also the delete_secret to support also dependabot secrets.
You can cherry pick the commit from my fork @thomascrowley jesdi@a1ef679
Thanks again!

smkillen and others added 4 commits March 13, 2024 15:48
* Review Comments: Changing Testing of secret_type assertion

* update delete secret option

---------

Co-authored-by: Thomas Crowley <15927917+thomascrowley@users.noreply.github.com>
tests/Organization.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

LGTM!

@EnricoMi EnricoMi merged commit 0784f83 into PyGithub:main Mar 21, 2024
15 checks passed
@thomascrowley
Copy link
Contributor Author

LGTM!

Thanks for the approval @EnricoMi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants