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

build(vrt): sanitize branch names #2948

Merged
merged 2 commits into from Feb 28, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion vrt/ci.js
Expand Up @@ -23,7 +23,7 @@ const {
} = process.env;

// Derive some useful constants
const SNAPSHOT_BRANCH = `${BUILDKITE_BRANCH}--vrt`;
const SNAPSHOT_BRANCH = `${sanitizeBranchName(BUILDKITE_BRANCH)}--vrt`;
const [
ORIGINAL_REPOSITORY_OWNER,
ORIGINAL_REPOSITORY_NAME,
Expand Down Expand Up @@ -346,6 +346,12 @@ function getRepositoryOwnerAndNameFromURL(url) {
return [owner, name];
}

function sanitizeBranchName(branchName) {
// Colons are not permitted in git branch names and for some reason
// Buildkite will report the branch name as "user:branch".
return branchName.replace(/:/g, '-');
Copy link
Member

Choose a reason for hiding this comment

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

Should we sanitize even more characters and use some 3rd party lib?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be fine since we are only protecting against the colon that buildkite might add to the actual branch name. The user's actual branch name can't be created in the first place if it has bad characters. Not sure why buildkite does this, but I think if the branch is called master - buildkite will append the repo owner (ex: sandgraham:master).

}

function log(message) {
console.log(`❖ VRT: ${message}`);
}