Merge pull request #299 from github/lock-branch-validation

Lock Branch Validation

Author: GrantBirki

Diff for: __tests__/functions/valid-branch-name.test.js

@@ -0,0 +1,53 @@
+import {constructValidBranchName} from '../../src/functions/valid-branch-name'
+import * as core from '@actions/core'
+
+const debugMock = jest.spyOn(core, 'debug')
+
+const branchName = 'production'
+
+beforeEach(() => {
+  jest.clearAllMocks()
+  jest.spyOn(core, 'debug').mockImplementation(() => {})
+})
+
+test('does not make any modifications to a valid branch name', async () => {
+  expect(constructValidBranchName(branchName)).toBe(branchName)
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructing valid branch name: ${branchName}`
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructed valid branch name: ${branchName}`
+  )
+})
+
+test('replaces spaces with hyphens', async () => {
+  expect(constructValidBranchName(`super ${branchName}`)).toBe(
+    `super-${branchName}`
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructing valid branch name: super ${branchName}`
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructed valid branch name: super-${branchName}`
+  )
+})
+
+test('replaces multiple spaces with hyphens', async () => {
+  expect(constructValidBranchName(`super duper ${branchName}`)).toBe(
+    `super-duper-${branchName}`
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructing valid branch name: super duper ${branchName}`
+  )
+  expect(debugMock).toHaveBeenCalledWith(
+    `constructed valid branch name: super-duper-${branchName}`
+  )
+})
+
+test('returns null if the branch is null', async () => {
+  expect(constructValidBranchName(null)).toBe(null)
+})
+
+test('returns undefined if the branch is undefined', async () => {
+  expect(constructValidBranchName(undefined)).toBe(undefined)
+})

Diff for: dist/index.js

@@ -1,5 +1,6 @@
 import {LOCK_METADATA} from './lock-metadata'
 import {COLORS} from './colors'
+import {constructValidBranchName} from './valid-branch-name'
 import * as core from '@actions/core'
 
 const LOCK_FILE = LOCK_METADATA.lockFile
@@ -10,6 +11,8 @@ const LOCK_FILE = LOCK_METADATA.lockFile
 // :param branchName: The name of the branch to check
 // :return: The lock file contents if it exists, false if not
 export async function checkLockFile(octokit, context, branchName) {
+  branchName = constructValidBranchName(branchName)
+
   core.debug(`checking if lock file exists on branch: ${branchName}`)
   // If the lock branch exists, check if a lock file exists
   try {

Diff for: dist/index.js.map

@@ -2,6 +2,7 @@ import * as core from '@actions/core'
 import dedent from 'dedent-js'
 import {checkLockFile} from './check-lock-file'
 import {actionStatus} from './action-status'
+import {constructValidBranchName} from './valid-branch-name'
 import {timeDiff} from './time-diff'
 import {LOCK_METADATA} from './lock-metadata'
 import {COLORS} from './colors'
@@ -23,7 +24,7 @@ async function constructBranchName(environment, global) {
   }
 
   // If the lock is not global, return the environment-specific lock branch name
-  return `${environment}-${LOCK_BRANCH_SUFFIX}`
+  return `${constructValidBranchName(environment)}-${LOCK_BRANCH_SUFFIX}`
 }
 
 // Helper function for creating a lock file for branch-deployment locks

Diff for: src/functions/check-lock-file.js

@@ -2,6 +2,7 @@ import * as core from '@actions/core'
 import {unlock} from './unlock'
 import {LOCK_METADATA} from './lock-metadata'
 import {checkLockFile} from './check-lock-file'
+import {constructValidBranchName} from './valid-branch-name'
 
 // Helper function to automatically find, and release a deployment lock when a pull request is merged
 // :param octokit: the authenticated octokit instance
@@ -28,7 +29,7 @@ export async function unlockOnMerge(octokit, context, environment_targets) {
   var releasedEnvironments = []
   for (const environment of environment_targets.split(',')) {
     // construct the lock branch name for this environment
-    var lockBranch = `${environment}-${LOCK_METADATA.lockBranchSuffix}`
+    var lockBranch = `${constructValidBranchName(environment)}-${LOCK_METADATA.lockBranchSuffix}`
 
     // attempt to fetch the lockFile for this branch
     var lockFile = await checkLockFile(octokit, context, lockBranch)

Diff for: src/functions/lock.js

@@ -2,6 +2,7 @@ import * as core from '@actions/core'
 import {actionStatus} from './action-status'
 import dedent from 'dedent-js'
 import {LOCK_METADATA} from './lock-metadata'
+import {constructValidBranchName} from './valid-branch-name'
 import {COLORS} from './colors'
 
 // Constants for the lock file
@@ -90,7 +91,7 @@ export async function unlock(
       branchName = GLOBAL_LOCK_BRANCH
       successText = '`global`'
     } else {
-      branchName = `${environment}-${LOCK_BRANCH_SUFFIX}`
+      branchName = `${constructValidBranchName(environment)}-${LOCK_BRANCH_SUFFIX}`
       successText = `\`${environment}\``
     }
 

Diff for: src/functions/unlock-on-merge.js

@@ -0,0 +1,20 @@
+import * as core from '@actions/core'
+
+// Helper function to create a valid branch name that will pass GitHub's API ref validation
+// :param branch: The branch name
+// :returns: A string of the branch name with proper formatting
+export function constructValidBranchName(branch) {
+  core.debug(`constructing valid branch name: ${branch}`)
+
+  if (branch === null) {
+    return null
+  } else if (branch === undefined) {
+    return undefined
+  }
+
+  // If environment contains any spaces, replace all of them with a hyphen
+  branch = branch.replace(/\s/g, '-')
+
+  core.debug(`constructed valid branch name: ${branch}`)
+  return branch
+}

Diff for: src/functions/unlock.js

@@ -24,6 +24,7 @@ import {help} from './functions/help'
 import {LOCK_METADATA} from './functions/lock-metadata'
 import {COLORS} from './functions/colors'
 import {getInputs} from './functions/inputs'
+import {constructValidBranchName} from './functions/valid-branch-name'
 
 // :returns: 'success', 'success - noop', 'success - merge deploy mode', 'failure', 'safe-exit', 'success - unlock on merge mode' or raises an error
 export async function run() {
@@ -255,7 +256,7 @@ export async function run() {
             // special comment for global deploy locks
             let globalMsg = ''
             let environmentMsg = `- __Environment__: \`${lockData.environment}\``
-            let lockBranchName = `${lockData.environment}-${LOCK_METADATA.lockBranchSuffix}`
+            let lockBranchName = `${constructValidBranchName(lockData.environment)}-${LOCK_METADATA.lockBranchSuffix}`
             if (lockData.global === true) {
               globalMsg = dedent(`