Environments tolerate concise configuration (#649)

* 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.

README.md

@@ -266,9 +266,9 @@ And the `checkrun` page will look like this:
 <img width="860" alt="image" src="https://github.com/github/safe-settings/assets/57544838/893ff4e6-904c-4a07-924a-7c23dc068983">
 </p>
 
-### The Settings File
+### The Settings Files
 
-The settings file can be used to set the policies at the `org`, `suborg` or `repo` level.
+The settings files can be used to set the policies at the `org`, `suborg` or `repo` level.
 
 The following can be configured:
 
@@ -284,6 +284,7 @@ The following can be configured:
 - `Autolinks`
 - `Repository name validation` using regex pattern
 - `Rulesets`
+- `Environments` - wait timer, required reviewers, prevent self review, protected branches deployment branch policy, custom deployment branch policy, variables, deployment protection rules
 
 It is possible to provide an `include` or `exclude` settings to restrict the `collaborators`, `teams`, `labels` to a list of repos or exclude a set of repos for a collaborator.
 

app.yml

@@ -34,6 +34,10 @@ default_permissions:
     repository_custom_properties: write
     organization_custom_properties: admin
 
+    # Workflows, workflow runs and artifacts. (needed to read environments when repo is private or internal)
+    # https://developer.github.com/v3/apps/permissions/#repository-permissions-for-actions
+    actions: read
+
     # Repository creation, deletion, settings, teams, and collaborators.
     # https://developer.github.com/v3/apps/permissions/#permission-on-administration
     administration: write
@@ -50,6 +54,10 @@ default_permissions:
     # https://developer.github.com/v3/apps/permissions/#permission-on-deployments
     # deployments: read
 
+    # Manage repository environments.
+    # https://developer.github.com/v3/apps/permissions/#repository-permissions-for-environments
+    environments: write
+
     # Issues and related comments, assignees, labels, and milestones.
     # https://developer.github.com/v3/apps/permissions/#permission-on-issues
     issues: write
@@ -106,6 +114,10 @@ default_permissions:
     # https://developer.github.com/v3/apps/permissions/
     organization_administration: write
 
+    # Manage Actions repository variables.
+    # https://developer.github.com/v3/apps/permissions/#repository-permissions-for-variables
+    variables: write
+
 
 # The name of the GitHub App. Defaults to the name specified in package.json
 name: Safe Settings

docs/deploy.md

@@ -255,14 +255,17 @@ Every deployment will need an [App](https://developer.github.com/apps/).
 
 #### Repository Permissions
 
+- Actions: **Read-only**
 - Administration: **Read & Write**
 - Checks: **Read & Write**
 - Commit statuses: **Read & Write**
 - Contents: **Read & Write**
 - Custom properties: **Read & Write**
+- Environments: **Read & Write**
 - Issues: **Read & Write**
 - Metadata: **Read-only**
 - Pull requests: **Read & Write**
+- Variables: **Read & Write**
 
 #### Organization Permissions
 

lib/plugins/environments.js

@@ -1,4 +1,6 @@
 const Diffable = require('./diffable')
+const MergeDeep = require('../mergeDeep')
+const NopCommand = require('../nopcommand')
 
 module.exports = class Environments extends Diffable {
     constructor(...args) {
@@ -14,7 +16,11 @@ module.exports = class Environments extends Diffable {
                 });
               }
             })
-          }
+        }
+
+        // Remove 'name' from filtering list so Environments with only a name defined are processed.
+        MergeDeep.NAME_FIELDS.splice(MergeDeep.NAME_FIELDS.indexOf('name'), 1)
+
     }
 
     async find() {
@@ -78,7 +84,7 @@ module.exports = class Environments extends Diffable {
         const wait_timer = existing.wait_timer !== attrs.wait_timer;
         const prevent_self_review = existing.prevent_self_review !== attrs.prevent_self_review;
         const reviewers = JSON.stringify(existing.reviewers.sort((x1, x2) => x1.id - x2.id)) !== JSON.stringify(attrs.reviewers.sort((x1, x2) => x1.id - x2.id));
-        
+
         let existing_custom_branch_policies = existing.deployment_branch_policy === null ? null : existing.deployment_branch_policy.custom_branch_policies;
         if(typeof(existing_custom_branch_policies) === 'object' && existing_custom_branch_policies !== null) {
             existing_custom_branch_policies = existing_custom_branch_policies.sort();
@@ -158,6 +164,7 @@ module.exports = class Environments extends Diffable {
 
         if(variables) {
             let existingVariables = [...existing.variables];
+
             for(let variable of attrs.variables) {
                 const existingVariable = existingVariables.find((_var) => _var.name === variable.name);
                 if(existingVariable) {
@@ -195,6 +202,7 @@ module.exports = class Environments extends Diffable {
 
         if(deployment_protection_rules) {
             let existingRules = [...existing.deployment_protection_rules];
+
             for(let rule of attrs.deployment_protection_rules) {
                 const existingRule = existingRules.find((_rule) => _rule.id === rule.id);
 
@@ -227,13 +235,14 @@ module.exports = class Environments extends Diffable {
             wait_timer: attrs.wait_timer,
             prevent_self_review: attrs.prevent_self_review,
             reviewers: attrs.reviewers,
-            deployment_branch_policy: attrs.deployment_branch_policy === null ? null : {
-                protected_branches: attrs.deployment_branch_policy.protected_branches,
+            deployment_branch_policy: attrs.deployment_branch_policy == null ? null : {
+                protected_branches: !!attrs.deployment_branch_policy.protected_branches,
                 custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies
             }
         });
 
         if(attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) {
+
             for(let policy of attrs.deployment_branch_policy.custom_branch_policies) {
                 await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', {
                     org: this.repo.owner,
@@ -242,26 +251,34 @@ module.exports = class Environments extends Diffable {
                     name: policy.name
                 });
             }
-        }
-
 
-        for(let variable of attrs.variables) {
-            await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/variables`, {
-                org: this.repo.owner,
-                repo: this.repo.repo,
-                environment_name: attrs.name,
-                name: variable.name,
-                value: variable.value
-            });
         }
 
-        for(let rule of attrs.deployment_protection_rules) {
-            await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules`, {
-                org: this.repo.owner,
-                repo: this.repo.repo,
-                environment_name: attrs.name,
-                integration_id: rule.app_id
-            });
+        if(attrs.variables) {
+
+            for(let variable of attrs.variables) {
+                await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/variables`, {
+                    org: this.repo.owner,
+                    repo: this.repo.repo,
+                    environment_name: attrs.name,
+                    name: variable.name,
+                    value: variable.value
+                });
+            }
+
+          }
+
+        if(attrs.deployment_protection_rules) {
+
+            for(let rule of attrs.deployment_protection_rules) {
+                await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules`, {
+                    org: this.repo.owner,
+                    repo: this.repo.repo,
+                    environment_name: attrs.name,
+                    integration_id: rule.app_id
+                });
+            }
+
         }
     }
 
@@ -272,4 +289,79 @@ module.exports = class Environments extends Diffable {
             environment_name: existing.name
         });
     }
-}
+
+    sync () {
+      const resArray = []
+      if (this.entries) {
+        let filteredEntries = this.filterEntries()
+        return this.find().then(existingRecords => {
+
+          // Filter out all empty entries (usually from repo override)
+          for (const entry of filteredEntries) {
+            for (const key of Object.keys(entry)) {
+              if (entry[key] === null || entry[key] === undefined) {
+                delete entry[key]
+              }
+            }
+          }
+          filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !MergeDeep.NAME_FIELDS.includes(key)).length !== 0)
+
+          const changes = []
+
+          existingRecords.forEach(x => {
+            if (!filteredEntries.find(y => this.comparator(x, y))) {
+              const change = this.remove(x).then(res => {
+                if (this.nop) {
+                  return resArray.push(res)
+                }
+                return res
+              })
+              changes.push(change)
+            }
+          })
+
+          filteredEntries.forEach(attrs => {
+            const existing = existingRecords.find(record => {
+              return this.comparator(record, attrs)
+            })
+
+            if (!existing) {
+              const change = this.add(attrs).then(res => {
+                if (this.nop) {
+                  return resArray.push(res)
+                }
+                return res
+              })
+              changes.push(change)
+            } else if (this.changed(existing, attrs)) {
+              const change = this.update(existing, attrs).then(res => {
+                if (this.nop) {
+                  return resArray.push(res)
+                }
+                return res
+              })
+              changes.push(change)
+            }
+          })
+
+          if (this.nop) {
+            return Promise.resolve(resArray)
+          }
+          return Promise.all(changes)
+        }).catch(e => {
+          if (this.nop) {
+            if (e.status === 404) {
+              // Ignore 404s which can happen in dry-run as the repo may not exist.
+              return Promise.resolve(resArray)
+            } else {
+              resArray.push(new NopCommand(this.constructor.name, this.repo, null, `error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`, 'ERROR'))
+              return Promise.resolve(resArray)
+            }
+          } else {
+            this.logError(`Error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`)
+          }
+        })
+      }
+    }
+
+}