Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branch Ruleset Checks #342

Merged
merged 13 commits into from
Dec 12, 2024
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ As seen above, we have two steps. One for a noop deploy, and one for a regular d
| `skip_successful_noop_labels_if_approved` | `false` | `"false"` | Whether or not the post run logic should skip adding successful noop labels if the pull request is approved. This can be useful if you add a label such as "ready-for-review" after a `.noop` completes but want to skip adding that label in situations where the pull request is already approved. |
| `skip_successful_deploy_labels_if_approved` | `false` | `"false"` | Whether or not the post run logic should skip adding successful deploy labels if the pull request is approved. This can be useful if you add a label such as "ready-for-review" after a `.deploy` completes but want to skip adding that label in situations where the pull request is already approved. |
| `enforced_deployment_order` | `false` | `""` | A comma separated list of environments that must be deployed in a specific order. Example: `"development,staging,production"`. If this is set then you cannot deploy to latter environments unless the former ones have a successful and active deployment on the latest commit first - See the [enforced deployment order docs](./docs/enforced-deployment-order.md) for more details |
| `use_security_warnings` | `false` | `"true"` | Whether or not to leave security related warnings in log messages during deployments. Default is `"true"` |

## Outputs 📤

Expand Down
221 changes: 221 additions & 0 deletions __tests__/functions/branch-protection-checks.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
import {branchRulesetChecks} from '../../src/functions/branch-ruleset-checks'
import * as core from '@actions/core'
import {COLORS} from '../../src/functions/colors'
import {SUGGESTED_RULESETS} from '../../src/functions/suggested-rulesets'

var context
var octokit
var data
var rulesets

// const debugMock = jest.spyOn(core, 'debug').mockImplementation(() => {})
const warningMock = jest.spyOn(core, 'warning').mockImplementation(() => {})
const infoMock = jest.spyOn(core, 'info').mockImplementation(() => {})

beforeEach(() => {
jest.spyOn(core, 'info').mockImplementation(() => {})
jest.spyOn(core, 'debug').mockImplementation(() => {})
jest.spyOn(core, 'warning').mockImplementation(() => {})
jest.clearAllMocks()

data = {
branch: 'main'
}

rulesets = [
{
type: 'deletion'
},
{
type: 'non_fast_forward'
},
{
type: 'pull_request',
parameters: {
required_approving_review_count: 1,
dismiss_stale_reviews_on_push: true,
required_reviewers: [],
require_code_owner_review: true,
require_last_push_approval: false,
required_review_thread_resolution: false,
automatic_copilot_code_review_enabled: false,
allowed_merge_methods: ['merge', 'squash', 'rebase']
}
},
{
type: 'required_status_checks',
parameters: {
strict_required_status_checks_policy: true,
do_not_enforce_on_create: false,
required_status_checks: []
}
},
{
type: 'required_deployments',
parameters: {
required_deployment_environments: []
}
}
]

context = {
repo: {
owner: 'corp',
repo: 'test'
},
issue: {
number: 1
}
}

octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: rulesets})
}
}
}
})

test('finds that no branch protections or rulesets are defined', async () => {
octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: []})
}
}
}
expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: false,
failed_checks: ['missing_branch_rulesets']
})
expect(warningMock).toHaveBeenCalledWith(
`🔐 branch ${COLORS.highlight}rulesets${COLORS.reset} are not defined for branch ${COLORS.highlight}${data.branch}${COLORS.reset}`
)
})

test('exits early if the user has disabled security warnings', async () => {
data.use_security_warnings = false
expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: true
})
expect(warningMock).not.toHaveBeenCalled()
expect(infoMock).not.toHaveBeenCalledWith(
`🔐 branch ruleset checks ${COLORS.success}passed${COLORS.reset}`
)
})

test('finds that the branch ruleset is missing the deletion rule', async () => {
rulesets = rulesets.filter(rule => rule.type !== 'deletion')

octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: rulesets})
}
}
}

expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: false,
failed_checks: ['missing_deletion']
})
expect(warningMock).toHaveBeenCalledWith(
`🔐 branch ${COLORS.highlight}rulesets${COLORS.reset} for branch ${COLORS.highlight}${data.branch}${COLORS.reset} is missing a rule of type ${COLORS.highlight}deletion${COLORS.reset}`
)
})

test('finds that the branch ruleset is missing the dismiss_stale_reviews_on_push parameter on the pull_request rule', async () => {
rulesets = rulesets.map(rule => {
if (rule.type === 'pull_request') {
return {
type: 'pull_request',
parameters: {
...rule.parameters,
dismiss_stale_reviews_on_push: false
}
}
}
return rule
})

octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: rulesets})
}
}
}

expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: false,
failed_checks: ['mismatch_pull_request_dismiss_stale_reviews_on_push']
})
expect(warningMock).toHaveBeenCalledWith(
`🔐 branch ${COLORS.highlight}rulesets${COLORS.reset} for branch ${COLORS.highlight}${data.branch}${COLORS.reset} contains a rule of type ${COLORS.highlight}pull_request${COLORS.reset} with a parameter ${COLORS.highlight}dismiss_stale_reviews_on_push${COLORS.reset} which does not match the suggested parameter`
)
})

test('finds that all suggested branch rulesets are defined', async () => {
rulesets = SUGGESTED_RULESETS.map(suggested_rule => {
return {
type: suggested_rule.type,
parameters: suggested_rule.parameters
}
})

octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: rulesets})
}
}
}

expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: true,
failed_checks: []
})
expect(warningMock).not.toHaveBeenCalled()
expect(infoMock).toHaveBeenCalledWith(
`🔐 branch ruleset checks ${COLORS.success}passed${COLORS.reset}`
)
})

test('finds that all suggested branch rulesets are defined but required reviews is set to 0', async () => {
rulesets = SUGGESTED_RULESETS.map(suggested_rule => {
return {
type: suggested_rule.type,
parameters: suggested_rule.parameters
}
})

rulesets = rulesets.map(rule => {
if (rule.type === 'pull_request') {
return {
type: 'pull_request',
parameters: {
...rule.parameters,
required_approving_review_count: 0
}
}
}
return rule
})

octokit = {
rest: {
repos: {
getBranchRules: jest.fn().mockReturnValueOnce({data: rulesets})
}
}
}

expect(await branchRulesetChecks(context, octokit, data)).toStrictEqual({
success: false,
failed_checks: ['mismatch_pull_request_required_approving_review_count']
})
expect(warningMock).toHaveBeenCalledWith(
`🔐 branch ${COLORS.highlight}rulesets${COLORS.reset} for branch ${COLORS.highlight}${data.branch}${COLORS.reset} contains the required_approving_review_count parameter but it is set to 0`
)
})
15 changes: 11 additions & 4 deletions __tests__/functions/help.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ const defaultInputs = {
checks: 'all',
commit_verification: true,
ignored_checks: [],
enforced_deployment_order: []
enforced_deployment_order: [],
use_security_warnings: true
}

test('successfully calls help with defaults', async () => {
Expand Down Expand Up @@ -87,7 +88,8 @@ test('successfully calls help with non-defaults', async () => {
checks: ['test,build,security'],
ignored_checks: ['lint', 'format'],
commit_verification: false,
enforced_deployment_order: []
enforced_deployment_order: [],
use_security_warnings: false
}

expect(await help(octokit, context, 123, inputs))
Expand Down Expand Up @@ -124,7 +126,8 @@ test('successfully calls help with non-defaults again', async () => {
checks: 'required',
ignored_checks: ['lint'],
commit_verification: false,
enforced_deployment_order: ['development', 'staging', 'production']
enforced_deployment_order: ['development', 'staging', 'production'],
use_security_warnings: false
}

expect(await help(octokit, context, 123, inputs))
Expand Down Expand Up @@ -172,7 +175,8 @@ test('successfully calls help with non-defaults and unknown update_branch settin
allow_sha_deployments: false,
checks: 'required',
ignored_checks: ['lint'],
enforced_deployment_order: []
enforced_deployment_order: [],
use_security_warnings: false
}

expect(await help(octokit, context, 123, inputs))
Expand All @@ -190,4 +194,7 @@ test('successfully calls help with non-defaults and unknown update_branch settin
expect(debugMock).toHaveBeenCalledWith(
expect.stringMatching(/Unknown value for update_branch/)
)
expect(debugMock).toHaveBeenCalledWith(
expect.stringMatching(/not use security warnings/)
)
})
7 changes: 7 additions & 0 deletions __tests__/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {run} from '../src/main'
import * as reactEmote from '../src/functions/react-emote'
import * as contextCheck from '../src/functions/context-check'
import * as prechecks from '../src/functions/prechecks'
import * as branchRulesetChecks from '../src/functions/branch-ruleset-checks'
import * as help from '../src/functions/help'
import * as validPermissions from '../src/functions/valid-permissions'
import * as identicalCommitCheck from '../src/functions/identical-commit-check'
Expand Down Expand Up @@ -87,6 +88,7 @@ beforeEach(() => {
process.env.INPUT_ENFORCED_DEPLOYMENT_ORDER = ''
process.env.INPUT_COMMIT_VERIFICATION = 'false'
process.env.INPUT_IGNORED_CHECKS = ''
process.env.INPUT_USE_SECURITY_WARNINGS = 'true'

github.context.payload = {
issue: {
Expand Down Expand Up @@ -163,6 +165,11 @@ beforeEach(() => {
isFork: false
}
})
jest
.spyOn(branchRulesetChecks, 'branchRulesetChecks')
.mockImplementation(() => {
return undefined
})
jest
.spyOn(commitSafetyChecks, 'commitSafetyChecks')
.mockImplementation(() => {
Expand Down
10 changes: 10 additions & 0 deletions __tests__/schemas/action.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,16 @@ inputs:
default:
type: string
required: false
use_security_warnings:
description:
type: string
Copy link
Preview

Copilot AI Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type for 'description' should be 'boolean' instead of 'string' to match the intended data type.

Suggested change
type: string
description:
type: boolean

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
required: true
required:
type: boolean
required: true
default:
type: string
required: false

# outputs section
outputs:
Expand Down
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ inputs:
description: 'A comma separated list of environments that must be deployed in a specific order. Example: `"development,staging,production"`. If this is set then you cannot deploy to latter environments unless the former ones have a successful and active deployment on the latest commit first.'
required: false
default: ""
use_security_warnings:
description: 'Whether or not to leave security related warnings in log messages during deployments. Default is "true"'
required: false
default: "true"
outputs:
continue:
description: 'The string "true" if the deployment should continue, otherwise empty - Use this to conditionally control if your deployment should proceed or not'
Expand Down
Loading
Loading