plugins/environments.js: Fix creation of new environments #616
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
klutchell
force-pushed
the
kyle/new-environments
branch
from
552cee6
to
6b0d187
Compare
|
@klutchell Would you mind saying why you decided to close? Thanks |
I might reopen, but I noticed that running this build in production was able to create new environments but did not add any environment variables. I quickly closed the PR until I can verify that it's not doing more harm than good. I suspect setting of environment variables is also broken in some way, I just want to check that I didn't introduce the issue. |
|
I suspect this is resolved by #612, marking as draft for now |
The command `eslint --fix ./lib/plugins/environments.js` was used to lint the plugin and some variable names were updated to camel case manually. Signed-off-by: Kyle Harding <[email protected]>
klutchell
force-pushed
the
kyle/new-environments
branch
from
6b0d187
to
50b6c5f
Compare
Without this change, environments that do not exist will not be created by safe-settings. New tests are included. Signed-off-by: Kyle Harding <[email protected]>
klutchell
force-pushed
the
kyle/new-environments
branch
from
50b6c5f
to
b5f0802
Compare
|
This is ready for review & merge now, I've confirmed locally that it is able to create new environments and add variables. |
|
How does this change look to you @decyjphr ? |
Brad-Abrams
added a commit
to RBC/github-safe-settings
that referenced
this pull request
Jun 26, 2024
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
decyjphr
pushed a commit
that referenced
this pull request
Aug 15, 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](#616) and [646](#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 #623](#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.
|
Superseded by #649 |
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.
Currently if environments do not exist in the target repository, they cannot be created
if all properties are not populated.
This PR performs a lint of the environments plugin file (first commit) before adding new
test cases to cover the creation of environments that do not exist.
The command
eslint --fix ./lib/plugins/environments.jswas used to lint the plugin and some variable names were
updated to camel case manually.