From 658e3e3126e50bbceae38826f17e4158d2ec7055 Mon Sep 17 00:00:00 2001 From: Brad Abrams Date: Tue, 14 May 2024 15:36:30 -0400 Subject: [PATCH 1/5] decompose unit tests, patch sync for environments --- lib/plugins/environments.js | 84 +- test/unit/lib/plugins/environments.test.js | 911 +++++++++++++++------ 2 files changed, 753 insertions(+), 242 deletions(-) diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js index 3435388e..299f3407 100644 --- a/lib/plugins/environments.js +++ b/lib/plugins/environments.js @@ -1,4 +1,5 @@ const Diffable = require('./diffable') +const MergeDeep = require('../mergeDeep') module.exports = class Environments extends Diffable { constructor(...args) { @@ -78,7 +79,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(); @@ -243,7 +244,7 @@ module.exports = class Environments extends Diffable { }); } } - + for(let variable of attrs.variables) { await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/variables`, { @@ -272,4 +273,81 @@ module.exports = class Environments extends Diffable { environment_name: existing.name }); } -} \ No newline at end of file + + sync () { + const resArray = [] + if (this.entries) { + let filteredEntries = this.filterEntries() + // this.log.debug(`filtered entries are ${JSON.stringify(filteredEntries)}`) + return this.find().then(existingRecords => { + this.log.debug(` ${JSON.stringify(existingRecords, null, 2)} \n\n ${JSON.stringify(filteredEntries, null, 2)} `) + + // 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)}`) + } + }) + } + } + +} diff --git a/test/unit/lib/plugins/environments.test.js b/test/unit/lib/plugins/environments.test.js index e826e005..e7e214df 100644 --- a/test/unit/lib/plugins/environments.test.js +++ b/test/unit/lib/plugins/environments.test.js @@ -1,274 +1,707 @@ const { when } = require('jest-when') const Environments = require('../../../../lib/plugins/environments') -describe('Environments', () => { - let github - const org = 'bkeepers' - const repo = 'test' - - function fillEnvironment(attrs) { - if (!attrs.wait_timer) attrs.wait_timer = 0; - if (!attrs.prevent_self_review) attrs.prevent_self_review = false; - if (!attrs.reviewers) attrs.reviewers = []; - if (!attrs.deployment_branch_policy) attrs.deployment_branch_policy = null; - if(!attrs.variables) attrs.variables = []; - if(!attrs.deployment_protection_rules) attrs.deployment_protection_rules = []; - if(!attrs.protection_rules) attrs.protection_rules = []; - - return attrs; +describe('Environments Plugin test suite', () => { + let github + let environment_name = '' + const org = 'bkeepers' + const repo = 'test' + const AllEnvironmentNamesBeingTested = ['wait-timer_environment', 'wait-timer_2_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment'] + const log = jest.fn() + log.debug = jest.fn() + log.error = jest.fn() + + function fillEnvironment(attrs) { + if (!attrs.wait_timer) attrs.wait_timer = 0; + if (!attrs.prevent_self_review) attrs.prevent_self_review = false; + if (!attrs.reviewers) attrs.reviewers = []; + if (!attrs.deployment_branch_policy) attrs.deployment_branch_policy = null; + if (!attrs.variables) attrs.variables = []; + if (!attrs.deployment_protection_rules) attrs.deployment_protection_rules = []; + if (!attrs.protection_rules) attrs.protection_rules = []; + + return attrs; + } + + beforeEach(() => { + //arrange for all + github = { + request: jest.fn(() => Promise.resolve(true)) + } + + AllEnvironmentNamesBeingTested.forEach((environment_name) => { + when(github.request) + .calledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }) + .mockResolvedValue({ + data: { + variables: [] + } + }) } + ); - beforeAll(() => { - github = { - request: jest.fn().mockReturnValue(Promise.resolve(true)) + AllEnvironmentNamesBeingTested.forEach((environment_name) => { + when(github.request) + .calledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }) + .mockResolvedValue({ + data: { + custom_deployment_protection_rules: [] + } + }) + } + ); + + when(github.request) + .calledWith('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { org, repo, environment_name: 'deployment-branch-policy-custom_environment' }) + .mockResolvedValue({ + data: { + branch_policies: [] + } } + ); + + when(github.request) + .calledWith('DELETE /repos/:org/:repo/environments/:environment_name/deployment-branch-policies/:branch_policy_id') + .mockResolvedValue({}); + + when(github.request) + .calledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies') + .mockResolvedValue({}); + + when(github.request) + .calledWith('PUT /repos/:org/:repo/environments/:environment_name') + .mockResolvedValue({}); + + when(github.request) + .calledWith('POST /repos/:org/:repo/environments/:environment_name/variables') + .mockResolvedValue({}); + + when(github.request) + .calledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules') + .mockResolvedValue({}); + + when(github.request) + .calledWith('DELETE /repos/:org/:repo/environments/:environment_name/deployment_protection_rules/:rule_id') + .mockResolvedValue({}); + + }) + + afterEach(() => { + // every test should have included the retrieval of the existing GitHub configuration for the environment, variables, and deployment protection rules. + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); + jest.clearAllMocks(); + }); + + // start individual tests + + // wait-timer + describe('When the existing wait-timer is 0 and the config is set to 1', () => { + it('detect divergence and set wait-timer to 1', async () => { + //arrange + environment_name = 'wait-timer_environment' + // represent config with a wait timer of 1 + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + wait_timer: 1 + } + ], { + debug: function () { } + }); + + //model an existing environment with a wait timer of 0 + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + wait_timer: 0 + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update to the wait timer was requested with value 1 + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + wait_timer: 1 + })); + }) }) + }) - it('sync', () => { - const plugin = new Environments(undefined, github, {owner: org, repo}, [ - { - name: 'wait-timer', - wait_timer: 1 - }, + // add reviewers + describe('When there are no existing reviewers and config calls for a user and a team', () => { + it('detect divergence and set reviewers', async () => { + //arrange + environment_name = 'reviewers_environment' + // represent config with a reviewers being a user and a team + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + reviewers: [ { - name: 'reviewers', - reviewers: [ - { + type: 'User', + id: 1 + } + ] + } + ], { + debug: function () { } + }); + + //model an existing environment with no reviewers + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + protection_rules: [ + { + type: 'required_reviewers', + reviewers: [ + { type: 'User', - id: 1 - }, - { - type: 'Team', - id: 2 - } + reviewer: { + id: 56, + type: 'User' + } + } + ] + } ] - }, - { - name: 'prevent-self-review', - prevent_self_review: true - }, + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update the reviewers + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + reviewers: [ { - name: 'deployment-branch-policy', - deployment_branch_policy: { - protected_branches: true, - custom_branch_policies: false - } - }, + type: 'User', + id: 1 + } + ] + })); + }) + }) + }) + + // prevent self review + describe('When prevent self review is false, and the config calls for it to be true', () => { + it('detect divergence and set prevent self review to true', async () => { + //arrange + environment_name = 'prevent-self-review_environment' + // + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + prevent_self_review: true + } + ], { + debug: function () { } + }); + + //model an existing environment with prevent self review false + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + prevent_self_review: false + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update the prevent self review boolean + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + prevent_self_review: true + })); + }) + }) + }) + + // deployment branch policy + describe('When there is no existing deployment branch policy and the config sets a policy', () => { + it('detect divergence and set the deployment branch policy from the config', async () => { + //arrange + environment_name = 'deployment-branch-policy_environment' + // represent config with a reviewers being a user and a team + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + } + ], { + debug: function () { } + }); + + //model an existing environment with prevent self review false + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + deployment_branch_policy: null + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update branch policy + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + })); + }) + }) + }) + + // custom deployment branch policy + describe('When there is no existing deployment branch policy and the config sets a custom policy', () => { + it('detect divergence and set the custom deployment branch policy from the config', async () => { + //arrange + environment_name = 'deployment-branch-policy-custom_environment' + // represent config with a custom branch policy + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: [ + 'master', + 'dev' + ] + } + } + ], { + debug: function () { } + }); + + //model an existing environment with no branch policies + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + deployment_branch_policy: null + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update the custom branch policies + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: true + } + })); + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + name: 'master' + })); + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + name: 'dev' + })); + }) + }) + }) + + // add variable + describe('When there are no existing variables and config calls for one', () => { + it('detect divergence and add the variable', async () => { + //arrange + environment_name = 'variables_environment' + // represent config with a reviewers being a user and a team + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + variables: [ { - name: 'deployment-branch-policy-custom', - deployment_branch_policy: { - protected_branches: false, - custom_branch_policies: [ - 'master', - 'dev' - ] - } - }, + name: 'test', + value: 'test' + } + ] + } + ], { + debug: function () { } + }); + + //model an existing environment with no reviewers + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + variables: [] + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update the variables + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/variables', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + name: 'test', + value: 'test' + })); + }) + }) + }) + + // add deployment protection rules + describe('When there are no existing deployment protection rules, but config calls for one', () => { + it('detect divergence and add the deployment protection rule', async () => { + //arrange + environment_name = 'deployment-protection-rules_environment' + // represent config with a deployment protection rule + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + deployment_protection_rules: [ { - name: 'variables', - variables: [ - { - name: 'test', - value: 'test' - } + app_id: 1 + } + ] + } + ], { + debug: function () { } + }); + + //model an existing environment with no deployment protection rules + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + deployment_protection_rules: [] + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update the deployment protection rules + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + integration_id: 1 // weird that this is integration_id, but above it's app_id + })); + }) + }) + }) + + // wait-timer unchanged + describe('When the existing wait-timer is 2 and the config is set to 2', () => { + it('detect that the value is unchanged, and do nothing', async () => { + //arrange + environment_name = 'wait-timer_2_environment' + // represent config with a wait timer of 2 + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + wait_timer: 2 + } + ], { + debug: function () { } + }); + + //model an existing environment with no reviewers + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: environment_name, + protection_rules: [ + { + type: 'wait_timer', + wait_timer: 2 + } ] + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update to the wait timer was requested with value 2 + expect(github.request).not.toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name, + wait_timer: 2 + })); + }) + }) + }) + + // original 7 changes all combined together test + describe('When there are changes across 7 environments', () => { + it('detect and apply all changes', async () => { + //arrange + environment_name = 'wait-timer_environment' //used in afterEach() + // represent 7 environments and their desired settings + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: 'wait-timer_environment', + wait_timer: 1 + }, + { + name: 'reviewers_environment', + reviewers: [ + { + type: 'User', + id: 1 }, { - name: 'deployment-protection-rules', - deployment_protection_rules: [ - { - app_id: 1 - } - ] + type: 'Team', + id: 2 } - ], { - debug: function() {} - }); + ] + }, + { + name: 'prevent-self-review_environment', + prevent_self_review: true + }, + { + name: 'deployment-branch-policy_environment', + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + }, + { + name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: [ + 'master', + 'dev' + ] + } + }, + { + name: 'variables_environment', + variables: [ + { + name: 'test', + value: 'test' + } + ] + }, + { + name: 'deployment-protection-rules_environment', + deployment_protection_rules: [ + { + app_id: 1 + } + ] + } + ], { + debug: function () { } + }); - when(github.request) - .calledWith('GET /repos/:org/:repo/environments', { org, repo }) - .mockResolvedValue({ - data: { - environments: [ - fillEnvironment({ - name: 'wait-timer', - wait_timer: 0 - }), - fillEnvironment({ - name: 'reviewers', - reviewers: [] - }), - fillEnvironment({ - name: 'prevent-self-review', - prevent_self_review: false - }), - fillEnvironment({ - name: 'deployment-branch-policy', - deployment_branch_policy: null - }), - fillEnvironment({ - name: 'deployment-branch-policy-custom', - deployment_branch_policy: null - }), - fillEnvironment({ - name: 'variables', - variables: [] - }), - fillEnvironment({ - name: 'deployment-protection-rules', - deployment_protection_rules: [] - }) - ] - } - }); - - ['wait-timer', 'reviewers', 'prevent-self-review', 'deployment-branch-policy', 'deployment-branch-policy-custom', 'variables', 'deployment-protection-rules'].forEach((environment_name) => { - when(github.request) - .calledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }) - .mockResolvedValue({ - data: { - variables: [] - } - }) + // model 7 existing environments and their settings + // note: wait-timer, required_reviewers, and branch_policy are modeled incorrectly here as they are not wrapped by protection_rules[] + // the test succeeds anyway because it so happens that the defaults assigned for missing values, coincidentally match the values below + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: 'wait-timer_environment', + wait_timer: 0 + }), + fillEnvironment({ + name: 'reviewers_environment', + reviewers: [] + }), + fillEnvironment({ + name: 'prevent-self-review_environment', + prevent_self_review: false + }), + fillEnvironment({ + name: 'deployment-branch-policy_environment', + deployment_branch_policy: null + }), + fillEnvironment({ + name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: null + }), + fillEnvironment({ + name: 'variables_environment', + variables: [] + }), + fillEnvironment({ + name: 'deployment-protection-rules_environment', + deployment_protection_rules: [] + }) + ] + } }); - ['wait-timer', 'reviewers', 'prevent-self-review', 'deployment-branch-policy', 'deployment-branch-policy-custom', 'variables', 'deployment-protection-rules'].forEach((environment_name) => { - when(github.request) - .calledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }) - .mockResolvedValue({ - data: { - custom_deployment_protection_rules: [] - } - }) - }); + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update to the wait timer was requested with value 2 - when(github.request) - .calledWith('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { org, repo, environment_name: 'deployment-branch-policy-custom' }) - .mockResolvedValue({ - data: { - branch_policies: [] - } - }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); - when(github.request) - .calledWith('DELETE /repos/:org/:repo/environments/:environment_name/deployment-branch-policies/:branch_policy_id') - .mockResolvedValue({}); + ['wait-timer_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment'].forEach((environment_name) => { + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); - when(github.request) - .calledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies') - .mockResolvedValue({}); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); + }); - when(github.request) - .calledWith('PUT /repos/:org/:repo/environments/:environment_name') - .mockResolvedValue({}); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'wait-timer_environment', + wait_timer: 1 + })); - when(github.request) - .calledWith('POST /repos/:org/:repo/environments/:environment_name/variables') - .mockResolvedValue({}); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'reviewers_environment', + reviewers: [ + { + type: 'User', + id: 1 + }, + { + type: 'Team', + id: 2 + } + ] + })); - when(github.request) - .calledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules') - .mockResolvedValue({}); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'prevent-self-review_environment', + prevent_self_review: true + })); - when(github.request) - .calledWith('DELETE /repos/:org/:repo/environments/:environment_name/deployment_protection_rules/:rule_id') - .mockResolvedValue({}); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'prevent-self-review_environment', + prevent_self_review: true + })); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy_environment', + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + })); - return plugin.sync().then(() => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: true + } + })); - ['wait-timer', 'reviewers', 'prevent-self-review', 'deployment-branch-policy', 'deployment-branch-policy-custom', 'variables', 'deployment-protection-rules'].forEach((environment_name) => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + name: 'master' + })); - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); - }); + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + name: 'dev' + })); - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'wait-timer', - wait_timer: 1 - })); + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/variables', expect.objectContaining({ + org, + repo, + environment_name: 'variables_environment', + name: 'test', + value: 'test' + })); - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'reviewers', - reviewers: [ - { - type: 'User', - id: 1 - }, - { - type: 'Team', - id: 2 - } - ] - })); - - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'prevent-self-review', - prevent_self_review: true - })); - - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'prevent-self-review', - prevent_self_review: true - })); - - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'deployment-branch-policy', - deployment_branch_policy: { - protected_branches: true, - custom_branch_policies: false - } - })); - - expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ - org, - repo, - environment_name: 'deployment-branch-policy-custom', - deployment_branch_policy: { - protected_branches: false, - custom_branch_policies: true - } - })); - - expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ - org, - repo, - environment_name: 'deployment-branch-policy-custom', - name: 'master' - })); - - expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ - org, - repo, - environment_name: 'deployment-branch-policy-custom', - name: 'dev' - })); - - expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/variables', expect.objectContaining({ - org, - repo, - environment_name: 'variables', - name: 'test', - value: 'test' - })); - - expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', expect.objectContaining({ - org, - repo, - environment_name: 'deployment-protection-rules', - integration_id: 1 - })); - }) + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-protection-rules_environment', + integration_id: 1 + })); + }) }) -}) \ No newline at end of file + }) + +}) From e8e7e53531e2549bc4247d5076b376a5e27952bf Mon Sep 17 00:00:00 2001 From: Brad Abrams Date: Wed, 15 May 2024 13:49:52 -0400 Subject: [PATCH 2/5] remove logging, combine loops as per review comments --- lib/plugins/environments.js | 2 -- test/unit/lib/plugins/environments.test.js | 8 ++------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js index 299f3407..bfdd0f10 100644 --- a/lib/plugins/environments.js +++ b/lib/plugins/environments.js @@ -278,9 +278,7 @@ module.exports = class Environments extends Diffable { const resArray = [] if (this.entries) { let filteredEntries = this.filterEntries() - // this.log.debug(`filtered entries are ${JSON.stringify(filteredEntries)}`) return this.find().then(existingRecords => { - this.log.debug(` ${JSON.stringify(existingRecords, null, 2)} \n\n ${JSON.stringify(filteredEntries, null, 2)} `) // Filter out all empty entries (usually from repo override) for (const entry of filteredEntries) { diff --git a/test/unit/lib/plugins/environments.test.js b/test/unit/lib/plugins/environments.test.js index e7e214df..18ca7f7e 100644 --- a/test/unit/lib/plugins/environments.test.js +++ b/test/unit/lib/plugins/environments.test.js @@ -37,18 +37,14 @@ describe('Environments Plugin test suite', () => { variables: [] } }) - } - ); - - AllEnvironmentNamesBeingTested.forEach((environment_name) => { - when(github.request) + when(github.request) .calledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }) .mockResolvedValue({ data: { custom_deployment_protection_rules: [] } }) - } + } ); when(github.request) From f1d5036a3dfdc57709932f6d5370f82f58395c80 Mon Sep 17 00:00:00 2001 From: Brad Abrams Date: Thu, 20 Jun 2024 16:57:09 -0400 Subject: [PATCH 3/5] Add NopCommand, log.error, and errors --- lib/plugins/environments.js | 1 + test/unit/lib/plugins/environments.test.js | 41 ++++++---------------- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js index bfdd0f10..fc602210 100644 --- a/lib/plugins/environments.js +++ b/lib/plugins/environments.js @@ -1,5 +1,6 @@ const Diffable = require('./diffable') const MergeDeep = require('../mergeDeep') +const NopCommand = require('../nopcommand') module.exports = class Environments extends Diffable { constructor(...args) { diff --git a/test/unit/lib/plugins/environments.test.js b/test/unit/lib/plugins/environments.test.js index 18ca7f7e..ba8938db 100644 --- a/test/unit/lib/plugins/environments.test.js +++ b/test/unit/lib/plugins/environments.test.js @@ -7,9 +7,8 @@ describe('Environments Plugin test suite', () => { const org = 'bkeepers' const repo = 'test' const AllEnvironmentNamesBeingTested = ['wait-timer_environment', 'wait-timer_2_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment'] - const log = jest.fn() - log.debug = jest.fn() - log.error = jest.fn() + const log = { debug: jest.fn(), error: console.error } + const errors = [] function fillEnvironment(attrs) { if (!attrs.wait_timer) attrs.wait_timer = 0; @@ -103,9 +102,7 @@ describe('Environments Plugin test suite', () => { name: environment_name, wait_timer: 1 } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with a wait timer of 0 when(github.request) @@ -150,9 +147,7 @@ describe('Environments Plugin test suite', () => { } ] } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with no reviewers when(github.request) @@ -210,9 +205,7 @@ describe('Environments Plugin test suite', () => { name: environment_name, prevent_self_review: true } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with prevent self review false when(github.request) @@ -255,9 +248,7 @@ describe('Environments Plugin test suite', () => { custom_branch_policies: false } } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with prevent self review false when(github.request) @@ -306,9 +297,7 @@ describe('Environments Plugin test suite', () => { ] } } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with no branch policies when(github.request) @@ -368,9 +357,7 @@ describe('Environments Plugin test suite', () => { } ] } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with no reviewers when(github.request) @@ -415,9 +402,7 @@ describe('Environments Plugin test suite', () => { } ] } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with no deployment protection rules when(github.request) @@ -457,9 +442,7 @@ describe('Environments Plugin test suite', () => { name: environment_name, wait_timer: 2 } - ], { - debug: function () { } - }); + ], log, errors); //model an existing environment with no reviewers when(github.request) @@ -555,9 +538,7 @@ describe('Environments Plugin test suite', () => { } ] } - ], { - debug: function () { } - }); + ], log, errors); // model 7 existing environments and their settings // note: wait-timer, required_reviewers, and branch_policy are modeled incorrectly here as they are not wrapped by protection_rules[] From fc179df514e8c430b69e84aa670eb8f7efbafe65 Mon Sep 17 00:00:00 2001 From: Brad Abrams Date: Mon, 24 Jun 2024 14:16:23 -0400 Subject: [PATCH 4/5] Allow concise config for Environments This commit combines PR [616](https://github.com/github/safe-settings/pull/616) and [646](https://github.com/github/safe-settings/pull/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 --- lib/plugins/environments.js | 53 ++- test/unit/lib/plugins/environments.test.js | 396 ++++++++++++++++++++- 2 files changed, 423 insertions(+), 26 deletions(-) diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js index fc602210..6d52b409 100644 --- a/lib/plugins/environments.js +++ b/lib/plugins/environments.js @@ -16,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() { @@ -160,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) { @@ -197,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); @@ -229,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, @@ -244,26 +251,34 @@ module.exports = class Environments extends Diffable { name: policy.name }); } + } + 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 - }); - } + 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 + }); + } - 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 - }); } } diff --git a/test/unit/lib/plugins/environments.test.js b/test/unit/lib/plugins/environments.test.js index ba8938db..276a82cd 100644 --- a/test/unit/lib/plugins/environments.test.js +++ b/test/unit/lib/plugins/environments.test.js @@ -6,7 +6,9 @@ describe('Environments Plugin test suite', () => { let environment_name = '' const org = 'bkeepers' const repo = 'test' - const AllEnvironmentNamesBeingTested = ['wait-timer_environment', 'wait-timer_2_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment'] + const PrimaryEnvironmentNamesBeingTested = ['wait-timer_environment', 'wait-timer_2_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment', 'new_environment', 'old_environment'] + const EnvironmentNamesForTheNewEnvironmentsTest = ['new-wait-timer', 'new-reviewers', 'new-prevent-self-review', 'new-deployment-branch-policy', 'new-deployment-branch-policy-custom', 'new-variables', 'new-deployment-protection-rules'] + const AllEnvironmentNamesBeingTested = PrimaryEnvironmentNamesBeingTested.concat(EnvironmentNamesForTheNewEnvironmentsTest) const log = { debug: jest.fn(), error: console.error } const errors = [] @@ -82,10 +84,6 @@ describe('Environments Plugin test suite', () => { }) afterEach(() => { - // every test should have included the retrieval of the existing GitHub configuration for the environment, variables, and deployment protection rules. - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); jest.clearAllMocks(); }); @@ -121,6 +119,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update to the wait timer was requested with value 1 + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -179,6 +180,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update the reviewers + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -224,6 +228,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update the prevent self review boolean + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -267,6 +274,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update branch policy + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -316,6 +326,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update the custom branch policies + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -376,6 +389,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update the variables + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/variables', expect.objectContaining({ org, repo, @@ -421,6 +437,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update the deployment protection rules + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', expect.objectContaining({ org, repo, @@ -466,6 +485,9 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { //assert - update to the wait timer was requested with value 2 + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); expect(github.request).not.toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ org, repo, @@ -476,11 +498,100 @@ describe('Environments Plugin test suite', () => { }) }) + // Zero existing environments + describe('When there are no existing environments, and the config has one environment', () => { + it('detect that and environment needs to be added, and add it', async () => { + //arrange + environment_name = 'new_environment' + // represent a new environment + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + } + ], log, errors); + + //model an existing state which has zero environments + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - the new environment was added + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name + })); + }) + }) + }) + + // Single environment name change + describe('When there is one existing environment with an old name, and the config has one environment with a new name', () => { + it('detect that an environment name has changed, add the new one, and delete the old one', async () => { + //arrange + environment_name = 'new_environment' + const old_environment_name = 'old_environment' + // represent a new environment + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: environment_name, + } + ], log, errors); + + //model an existing environment with an old name + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: old_environment_name + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - the new environment was added + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: environment_name + })); + + //assert - the old environment was deleted + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, old_environment_name }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, old_environment_name }); + expect(github.request).toHaveBeenCalledWith('DELETE /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: old_environment_name + })); + + }) + }) + }) + // original 7 changes all combined together test describe('When there are changes across 7 environments', () => { it('detect and apply all changes', async () => { //arrange - environment_name = 'wait-timer_environment' //used in afterEach() // represent 7 environments and their desired settings const plugin = new Environments(undefined, github, { owner: org, repo }, [ { @@ -582,7 +693,266 @@ describe('Environments Plugin test suite', () => { //act - run sync() in environments.js await plugin.sync().then(() => { - //assert - update to the wait timer was requested with value 2 + //assert - update to the wait timer was requested with value 1, etc. + + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); + + ['wait-timer_environment', 'reviewers_environment', 'prevent-self-review_environment', 'deployment-branch-policy_environment', 'deployment-branch-policy-custom_environment', 'variables_environment', 'deployment-protection-rules_environment'].forEach((environment_name) => { + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name }); + + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name }); + }); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'wait-timer_environment', + wait_timer: 1 + })); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'reviewers_environment', + reviewers: [ + { + type: 'User', + id: 1 + }, + { + type: 'Team', + id: 2 + } + ] + })); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'prevent-self-review_environment', + prevent_self_review: true + })); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'prevent-self-review_environment', + prevent_self_review: true + })); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy_environment', + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + })); + + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: true + } + })); + + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + name: 'master' + })); + + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-branch-policy-custom_environment', + name: 'dev' + })); + + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/variables', expect.objectContaining({ + org, + repo, + environment_name: 'variables_environment', + name: 'test', + value: 'test' + })); + + expect(github.request).toHaveBeenCalledWith('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', expect.objectContaining({ + org, + repo, + environment_name: 'deployment-protection-rules_environment', + integration_id: 1 + })); + }) + }) + }) + + // Add 7 new environments, each with one environment attribute set + describe('When there are 7 existing environments and 7 new environments each with one environment attribute in the config', () => { + it('make changes in the existing environments and also add the 7 new environments', async () => { + //arrange + // represent 14 environments (7 new) and their desired settings + const plugin = new Environments(undefined, github, { owner: org, repo }, [ + { + name: 'wait-timer_environment', + wait_timer: 1 + }, + { + name: 'reviewers_environment', + reviewers: [ + { + type: 'User', + id: 1 + }, + { + type: 'Team', + id: 2 + } + ] + }, + { + name: 'prevent-self-review_environment', + prevent_self_review: true + }, + { + name: 'deployment-branch-policy_environment', + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + }, + { + name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: [ + 'master', + 'dev' + ] + } + }, + { + name: 'variables_environment', + variables: [ + { + name: 'test', + value: 'test' + } + ] + }, + { + name: 'deployment-protection-rules_environment', + deployment_protection_rules: [ + { + app_id: 1 + } + ] + }, + { + name: 'new-wait-timer', + wait_timer: 1 + }, + { + name: 'new-reviewers', + reviewers: [ + { + type: 'User', + id: 1 + }, + { + type: 'Team', + id: 2 + } + ] + }, + { + name: 'new-prevent-self-review', + prevent_self_review: true + }, + { + name: 'new-deployment-branch-policy', + deployment_branch_policy: { + protected_branches: true, + custom_branch_policies: false + } + }, + { + name: 'new-deployment-branch-policy-custom', + deployment_branch_policy: { + protected_branches: false, + custom_branch_policies: [ + 'master', + 'dev' + ] + } + }, + { + name: 'new-variables', + variables: [ + { + name: 'test', + value: 'test' + } + ] + }, + { + name: 'new-deployment-protection-rules', + deployment_protection_rules: [ + { + app_id: 1 + } + ] + } + ], log, errors); + + // model 7 existing environments and their settings + // note: wait-timer, required_reviewers, and branch_policy are modeled incorrectly here as they are not wrapped by protection_rules[] + // the test succeeds anyway because it so happens that the defaults assigned for missing values, coincidentally match the values below + when(github.request) + .calledWith('GET /repos/:org/:repo/environments', { org, repo }) + .mockResolvedValue({ + data: { + environments: [ + fillEnvironment({ + name: 'wait-timer_environment', + wait_timer: 0 + }), + fillEnvironment({ + name: 'reviewers_environment', + reviewers: [] + }), + fillEnvironment({ + name: 'prevent-self-review_environment', + prevent_self_review: false + }), + fillEnvironment({ + name: 'deployment-branch-policy_environment', + deployment_branch_policy: null + }), + fillEnvironment({ + name: 'deployment-branch-policy-custom_environment', + deployment_branch_policy: null + }), + fillEnvironment({ + name: 'variables_environment', + variables: [] + }), + fillEnvironment({ + name: 'deployment-protection-rules_environment', + deployment_protection_rules: [] + }) + ] + } + }); + + //act - run sync() in environments.js + await plugin.sync().then(() => { + //assert - update to the wait timer was requested with value 1, etc. expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo }); @@ -677,6 +1047,18 @@ describe('Environments Plugin test suite', () => { environment_name: 'deployment-protection-rules_environment', integration_id: 1 })); + + //assert - seven new environments were also added + EnvironmentNamesForTheNewEnvironmentsTest.forEach(new_environment_name => { + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, new_environment_name }); + expect(github.request).not.toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, new_environment_name }); + expect(github.request).toHaveBeenCalledWith('PUT /repos/:org/:repo/environments/:environment_name', expect.objectContaining({ + org, + repo, + environment_name: new_environment_name + })); + }); + }) }) }) From 8d369c0e6ae5927349e06d25f94886dedf342a14 Mon Sep 17 00:00:00 2001 From: Brad Abrams Date: Mon, 24 Jun 2024 16:25:41 -0400 Subject: [PATCH 5/5] Update documentation: Environments permissions. Addresses issue: [Environments do not get provisioned for repositories set to internal or private #623](https://github.com/github/safe-settings/issues/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 | 5 +++-- app.yml | 12 ++++++++++++ docs/deploy.md | 3 +++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0c5f00bd..20a29bfe 100644 --- a/README.md +++ b/README.md @@ -266,9 +266,9 @@ And the `checkrun` page will look like this: image

-### 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. diff --git a/app.yml b/app.yml index 24c28282..44dd0bdc 100644 --- a/app.yml +++ b/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 diff --git a/docs/deploy.md b/docs/deploy.md index 7e016777..ba3dff7f 100644 --- a/docs/deploy.md +++ b/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