robojerk/codeql-action

Merge pull request #1675 from shaikhul/remove-consts

Browse source 
Remove MismatchedBranches check from code scanning workflow validation
This commit is contained in:
Henry Mercer 2023-05-11 15:45:33 +01:00 • committed by GitHub
commit 130884e4e1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 11 additions and 183 deletions
Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -2,7 +2,7 @@
## [UNRELEASED]
No user facing changes.
- Remove the requirement for `on.push` and `on.pull_request` to trigger on the same branches. [#1675](https://github.com/github/codeql-action/pull/1675)
## 2.3.3 - 04 May 2023

37
lib/workflow.js generated
View file

@ -65,18 +65,6 @@ function patternIsSuperset(patternA, patternB) {
return patternToRegExp(patternA).test(patternB);
}
exports.patternIsSuperset = patternIsSuperset;
function branchesToArray(branches) {
if (typeof branches === "string") {
return [branches];
}
if (Array.isArray(branches)) {
if (branches.length === 0) {
return "**";
}
return branches;
}
return "**";
}
function toCodedErrors(errors) {
return Object.entries(errors).reduce((acc, [code, message]) => {
acc[code] = { message, code };
@ -86,8 +74,7 @@ function toCodedErrors(errors) {
// code to send back via status report
// message to add as a warning annotation to the run
exports.WorkflowErrors = toCodedErrors({
MismatchedBranches: `Please make sure that every branch in on.pull_request is also in on.push so that Code Scanning can compare pull requests against the state of the base branch.`,
MissingPushHook: `Please specify an on.push hook so that Code Scanning can compare pull requests against the state of the base branch.`,
MissingPushHook: `Please specify an on.push hook to analyze and see code scanning alerts from the default branch on the Security tab.`,
CheckoutWrongHead: `git checkout HEAD^2 is no longer necessary. Please remove this step as Code Scanning recommends analyzing the merge commit for best results.`,
});
function getWorkflowErrors(doc) {
@ -132,28 +119,6 @@ function getWorkflowErrors(doc) {
if (!hasPush && hasPullRequest) {
missingPush = true;
}
// if doc.on.pull_request is null that means 'all branches'
// if doc.on.pull_request is undefined that means 'off'
// we only want to check for mismatched branches if pull_request is on.
if (doc.on.pull_request !== undefined) {
const push = branchesToArray(doc.on.push?.branches);
if (push !== "**") {
const pull_request = branchesToArray(doc.on.pull_request?.branches);
if (pull_request !== "**") {
const difference = pull_request.filter((value) => !push.some((o) => patternIsSuperset(o, value)));
if (difference.length > 0) {
// there are branches in pull_request that may not have a baseline
// because we are not building them on push
errors.push(exports.WorkflowErrors.MismatchedBranches);
}
}
else if (push.length > 0) {
// push is set up to run on a subset of branches
// and you could open a PR against a branch with no baseline
errors.push(exports.WorkflowErrors.MismatchedBranches);
}
}
}
}
if (missingPush) {
errors.push(exports.WorkflowErrors.MissingPushHook);

2
lib/workflow.js.map
View file

File diff suppressed because one or more lines are too long

49
lib/workflow.test.js generated
View file

@ -64,12 +64,6 @@ function errorCodes(actual, expected) {
});
t.deepEqual(...errorCodes(errors, []));
});
(0, ava_1.default)("getWorkflowErrors() when on.pull_requests is a string", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: { push: { branches: ["main"] }, pull_request: { branches: "*" } },
});
t.deepEqual(...errorCodes(errors, [workflow_1.WorkflowErrors.MismatchedBranches]));
});
(0, ava_1.default)("getWorkflowErrors() when on.pull_requests is a string and correct", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: { push: { branches: "*" }, pull_request: { branches: "*" } },
@ -84,15 +78,6 @@ function errorCodes(actual, expected) {
`));
t.deepEqual(...errorCodes(errors, []));
});
(0, ava_1.default)("getWorkflowErrors() when on.push is mismatched", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: {
push: { branches: ["main"] },
pull_request: { branches: ["feature"] },
},
});
t.deepEqual(...errorCodes(errors, [workflow_1.WorkflowErrors.MismatchedBranches]));
});
(0, ava_1.default)("getWorkflowErrors() when on.push is not mismatched", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: {
@ -102,15 +87,6 @@ function errorCodes(actual, expected) {
});
t.deepEqual(...errorCodes(errors, []));
});
(0, ava_1.default)("getWorkflowErrors() when on.push is mismatched for pull_request", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: {
push: { branches: ["main"] },
pull_request: { branches: ["main", "feature"] },
},
});
t.deepEqual(...errorCodes(errors, [workflow_1.WorkflowErrors.MismatchedBranches]));
});
(0, ava_1.default)("getWorkflowErrors() for a range of malformed workflows", (t) => {
t.deepEqual(...errorCodes((0, workflow_1.getWorkflowErrors)({
on: {
@ -175,16 +151,6 @@ function errorCodes(actual, expected) {
},
}), []));
});
(0, ava_1.default)("getWorkflowErrors() when on.pull_request for every branch but push specifies branches", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)(yaml.load(`
name: "CodeQL"
on:
push:
branches: ["main"]
pull_request:
`));
t.deepEqual(...errorCodes(errors, [workflow_1.WorkflowErrors.MismatchedBranches]));
});
(0, ava_1.default)("getWorkflowErrors() when on.pull_request for wildcard branches", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: {
@ -194,15 +160,6 @@ function errorCodes(actual, expected) {
});
t.deepEqual(...errorCodes(errors, []));
});
(0, ava_1.default)("getWorkflowErrors() when on.pull_request for mismatched wildcard branches", (t) => {
const errors = (0, workflow_1.getWorkflowErrors)({
on: {
push: { branches: ["feature/moose"] },
pull_request: { branches: "feature/*" },
},
});
t.deepEqual(...errorCodes(errors, [workflow_1.WorkflowErrors.MismatchedBranches]));
});
(0, ava_1.default)("getWorkflowErrors() when HEAD^2 is checked out", (t) => {
process.env.GITHUB_JOB = "test";
const errors = (0, workflow_1.getWorkflowErrors)({
@ -218,7 +175,7 @@ function errorCodes(actual, expected) {
(0, ava_1.default)("formatWorkflowErrors() when there are multiple errors", (t) => {
const message = (0, workflow_1.formatWorkflowErrors)([
workflow_1.WorkflowErrors.CheckoutWrongHead,
workflow_1.WorkflowErrors.MismatchedBranches,
workflow_1.WorkflowErrors.MissingPushHook,
]);
t.true(message.startsWith("2 issues were detected with this workflow:"));
});
@ -229,9 +186,9 @@ function errorCodes(actual, expected) {
(0, ava_1.default)("formatWorkflowCause()", (t) => {
const message = (0, workflow_1.formatWorkflowCause)([
workflow_1.WorkflowErrors.CheckoutWrongHead,
workflow_1.WorkflowErrors.MismatchedBranches,
workflow_1.WorkflowErrors.MissingPushHook,
]);
t.deepEqual(message, "CheckoutWrongHead,MismatchedBranches");
t.deepEqual(message, "CheckoutWrongHead,MissingPushHook");
t.deepEqual((0, workflow_1.formatWorkflowCause)([]), undefined);
});
(0, ava_1.default)("patternIsSuperset()", (t) => {

2
lib/workflow.test.js.map
View file

File diff suppressed because one or more lines are too long

61
src/workflow.test.ts
View file

@ -64,14 +64,6 @@ test("getWorkflowErrors() when on.push is a correct object", (t) => {
t.deepEqual(...errorCodes(errors, []));
});
test("getWorkflowErrors() when on.pull_requests is a string", (t) => {
const errors = getWorkflowErrors({
on: { push: { branches: ["main"] }, pull_request: { branches: "*" } },
});
t.deepEqual(...errorCodes(errors, [WorkflowErrors.MismatchedBranches]));
});
test("getWorkflowErrors() when on.pull_requests is a string and correct", (t) => {
const errors = getWorkflowErrors({
on: { push: { branches: "*" }, pull_request: { branches: "*" } },
@ -92,17 +84,6 @@ test("getWorkflowErrors() when on.push is correct with empty objects", (t) => {
t.deepEqual(...errorCodes(errors, []));
});
test("getWorkflowErrors() when on.push is mismatched", (t) => {
const errors = getWorkflowErrors({
on: {
push: { branches: ["main"] },
pull_request: { branches: ["feature"] },
},
});
t.deepEqual(...errorCodes(errors, [WorkflowErrors.MismatchedBranches]));
});
test("getWorkflowErrors() when on.push is not mismatched", (t) => {
const errors = getWorkflowErrors({
on: {
@ -114,17 +95,6 @@ test("getWorkflowErrors() when on.push is not mismatched", (t) => {
t.deepEqual(...errorCodes(errors, []));
});
test("getWorkflowErrors() when on.push is mismatched for pull_request", (t) => {
const errors = getWorkflowErrors({
on: {
push: { branches: ["main"] },
pull_request: { branches: ["main", "feature"] },
},
});
t.deepEqual(...errorCodes(errors, [WorkflowErrors.MismatchedBranches]));
});
test("getWorkflowErrors() for a range of malformed workflows", (t) => {
t.deepEqual(
...errorCodes(
@ -251,20 +221,6 @@ test("getWorkflowErrors() for a range of malformed workflows", (t) => {
);
});
test("getWorkflowErrors() when on.pull_request for every branch but push specifies branches", (t) => {
const errors = getWorkflowErrors(
yaml.load(`
name: "CodeQL"
on:
push:
branches: ["main"]
pull_request:
`) as Workflow
);
t.deepEqual(...errorCodes(errors, [WorkflowErrors.MismatchedBranches]));
});
test("getWorkflowErrors() when on.pull_request for wildcard branches", (t) => {
const errors = getWorkflowErrors({
on: {
@ -276,17 +232,6 @@ test("getWorkflowErrors() when on.pull_request for wildcard branches", (t) => {
t.deepEqual(...errorCodes(errors, []));
});
test("getWorkflowErrors() when on.pull_request for mismatched wildcard branches", (t) => {
const errors = getWorkflowErrors({
on: {
push: { branches: ["feature/moose"] },
pull_request: { branches: "feature/*" },
},
});
t.deepEqual(...errorCodes(errors, [WorkflowErrors.MismatchedBranches]));
});
test("getWorkflowErrors() when HEAD^2 is checked out", (t) => {
process.env.GITHUB_JOB = "test";
@ -306,7 +251,7 @@ test("formatWorkflowErrors() when there is one error", (t) => {
test("formatWorkflowErrors() when there are multiple errors", (t) => {
const message = formatWorkflowErrors([
WorkflowErrors.CheckoutWrongHead,
WorkflowErrors.MismatchedBranches,
WorkflowErrors.MissingPushHook,
]);
t.true(message.startsWith("2 issues were detected with this workflow:"));
});
@ -320,10 +265,10 @@ test("formatWorkflowCause() with no errors", (t) => {
test("formatWorkflowCause()", (t) => {
const message = formatWorkflowCause([
WorkflowErrors.CheckoutWrongHead,
WorkflowErrors.MismatchedBranches,
WorkflowErrors.MissingPushHook,
]);
t.deepEqual(message, "CheckoutWrongHead,MismatchedBranches");
t.deepEqual(message, "CheckoutWrongHead,MissingPushHook");
t.deepEqual(formatWorkflowCause([]), undefined);
});

41
src/workflow.ts
View file

@ -79,18 +79,6 @@ export function patternIsSuperset(patternA: string, patternB: string): boolean {
return patternToRegExp(patternA).test(patternB);
}
function branchesToArray(branches?: string | null | string[]): string[] | "**" {
if (typeof branches === "string") {
return [branches];
}
if (Array.isArray(branches)) {
if (branches.length === 0) {
return "**";
}
return branches;
}
return "**";
}
export interface CodedError {
message: string;
code: string;
@ -108,8 +96,7 @@ function toCodedErrors(errors: {
// code to send back via status report
// message to add as a warning annotation to the run
export const WorkflowErrors = toCodedErrors({
MismatchedBranches: `Please make sure that every branch in on.pull_request is also in on.push so that Code Scanning can compare pull requests against the state of the base branch.`,
MissingPushHook: `Please specify an on.push hook so that Code Scanning can compare pull requests against the state of the base branch.`,
MissingPushHook: `Please specify an on.push hook to analyze and see code scanning alerts from the default branch on the Security tab.`,
CheckoutWrongHead: `git checkout HEAD^2 is no longer necessary. Please remove this step as Code Scanning recommends analyzing the merge commit for best results.`,
});
@ -162,32 +149,6 @@ export function getWorkflowErrors(doc: Workflow): CodedError[] {
if (!hasPush && hasPullRequest) {
missingPush = true;
}
// if doc.on.pull_request is null that means 'all branches'
// if doc.on.pull_request is undefined that means 'off'
// we only want to check for mismatched branches if pull_request is on.
if (doc.on.pull_request !== undefined) {
const push = branchesToArray(doc.on.push?.branches);
if (push !== "**") {
const pull_request = branchesToArray(doc.on.pull_request?.branches);
if (pull_request !== "**") {
const difference = pull_request.filter(
(value) => !push.some((o) => patternIsSuperset(o, value))
);
if (difference.length > 0) {
// there are branches in pull_request that may not have a baseline
// because we are not building them on push
errors.push(WorkflowErrors.MismatchedBranches);
}
} else if (push.length > 0) {
// push is set up to run on a subset of branches
// and you could open a PR against a branch with no baseline
errors.push(WorkflowErrors.MismatchedBranches);
}
}
}
}
if (missingPush) {