plugins/environments.js: Fix creation of new environments#616
Closed
klutchell wants to merge 2 commits intogithub:main-enterprisefrom
Closed
plugins/environments.js: Fix creation of new environments#616klutchell wants to merge 2 commits intogithub:main-enterprisefrom
klutchell wants to merge 2 commits intogithub:main-enterprisefrom
Conversation
klutchell
force-pushed
the
kyle/new-environments
branch
from
552cee6 to
6b0d187
Compare
Contributor
|
@klutchell Would you mind saying why you decided to close? Thanks |
Contributor
Author
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. |
Contributor
Author
|
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 <kyle@balena.io>
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 <kyle@balena.io>
klutchell
force-pushed
the
kyle/new-environments
branch
from
50b6c5f to
b5f0802
Compare
Contributor
Author
|
This is ready for review & merge now, I've confirmed locally that it is able to create new environments and add variables. |
Contributor
Author
|
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.
Contributor
Author
|
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.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.