Environments tolerate concise configuration #649
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit combines PR [616](github#616) and [646](github#646) environments.js Add defensive code to prevent the GitHub API from being called with undefined data. In the UI, and API an environment can be added with just an name. Now, safe-settings permits this as well. In the UI, and API an environment can be added without variables. Now, safe-settings permits this as well. In the UI, and API an environment can be added without deployment_protection_rules. Now, safe-settings permits this as well. environments.test.js Add a test case for the scenario when there are zero existing environments Add a test case for an environment name change Add a test case inspired by PR 616 which adds 7 new environments with various attributes Move expect statements out of aftereach() as there is now variability in what is expected across test cases. Specifically, when there is no existing environment, that environment should NOT be queried for variables nor deployment_protection_rules
Addresses issue: [Environments do not get provisioned for repositories set to internal or private github#623](github#623) Adds documentation for permissions required for safe-settings when Environments are used [List Environments](https://docs.github.com/en/rest/deployments/environments?apiVersion=2022-11-28#list-environments) API requires: ``` The fine-grained token must have the following permission set: "Actions" repository permissions (read) ``` [Create an environment variable](https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-an-environment-variable) API requires: ``` The fine-grained token must have the following permission set: "Variables" repository permissions (write) and "Environments" repository permissions (write) ``` With permissions added, issue 623 was resolved.
|
I can confirm that the documentation changes for additional permissions address the issue of managing environments under private or internal repositories. I can also confirm that these changes address the API errors received when creating and managing existing environments under safe-settings. We should get these changes reviewed and merged ASAP. |
|
I don't think #616 is required if we can get this merged |
|
@Brad-Abrams Is your PR addressed this issue as well ? This is another problem with environment in safe settings |
I did not do anything to try to address #551. That felt like it was deserving of a separate PR. |
|
@decyjphr I've been a grateful user of safe-settings for many months now, and would like to also be a contributor. Please consider this PR to tighten up the handling of environments. |
|
Thank you kindly @decyjphr, @klutchell, @luvsaxena1 and those behind the scenes. I appreciate the support in seeing this through. Expect to see me again for further safe-settings contributions. |
admtorgst
pushed a commit
to helse-sorost/safe-settings
that referenced
this pull request
Sep 14, 2024
* decompose unit tests, patch sync for environments * remove logging, combine loops as per review comments * Add NopCommand, log.error, and errors * Allow concise config for Environments This commit combines PR [616](github#616) and [646](github#646) environments.js Add defensive code to prevent the GitHub API from being called with undefined data. In the UI, and API an environment can be added with just an name. Now, safe-settings permits this as well. In the UI, and API an environment can be added without variables. Now, safe-settings permits this as well. In the UI, and API an environment can be added without deployment_protection_rules. Now, safe-settings permits this as well. environments.test.js Add a test case for the scenario when there are zero existing environments Add a test case for an environment name change Add a test case inspired by PR 616 which adds 7 new environments with various attributes Move expect statements out of aftereach() as there is now variability in what is expected across test cases. Specifically, when there is no existing environment, that environment should NOT be queried for variables nor deployment_protection_rules * Update documentation: Environments permissions. Addresses issue: [Environments do not get provisioned for repositories set to internal or private github#623](github#623) Adds documentation for permissions required for safe-settings when Environments are used [List Environments](https://docs.github.com/en/rest/deployments/environments?apiVersion=2022-11-28#list-environments) API requires: ``` The fine-grained token must have the following permission set: "Actions" repository permissions (read) ``` [Create an environment variable](https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-an-environment-variable) API requires: ``` The fine-grained token must have the following permission set: "Variables" repository permissions (write) and "Environments" repository permissions (write) ``` With permissions added, issue 623 was resolved.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A basket of improvements to Environments in safe-settings.
The GitHub UI and APIs permit the creation of an environment with only a name. Before this change the environments plugin for safe-settings required a verbose definition of environments. This boiled down to failing api calls that were being made even if the config had empty values. Environments can now be modelled without the need to set empty arrays for variables and deployment protection rules.
The largest change in this PR is to the unit tests for Environments. One all encompassing test has been broken down into smaller units as detailed with screenshots in PR #646. Additionally, tests have been added for a previously missing scenario where there were zero environments existing in the repo.
This PR with commit fc179df combines together PR #646 and PR #616. I suggest that if this PR is taken, then other two can be closed without merging. However @klutchell would be the authority to speak to 616.
This PR with commit 8d369c0 addresses issue #623 via documentation updates which add in previously missing required permissions when using safe-settings with Environments.
I would be most appreciative of a review and subsequent merge.