Skip to content

Commit

Permalink
Merge pull request #2274 from github/aeisenberg/no-warn-workflow_call
Browse files Browse the repository at this point in the history
Avoid warning on workflow_call triggers
  • Loading branch information
aeisenberg authored May 8, 2024
2 parents 715d348 + ca7f194 commit 7d9b7a1
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 59 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ No user facing changes.
## 3.25.4 - 08 May 2024

- Update default CodeQL bundle version to 2.17.2. [#2270](https://github.com/github/codeql-action/pull/2270)
- Avoid printing out a warning for a missing `on.push` trigger when the CodeQL Action is triggered via a `workflow_call` event. [#2274](https://github.com/github/codeql-action/pull/2274)

## 3.25.3 - 25 Apr 2024

Expand All @@ -34,7 +35,7 @@ No user facing changes.

- The `setup-python-dependencies` input to the `init` Action
- The `CODEQL_ACTION_DISABLE_PYTHON_DEPENDENCY_INSTALLATION` environment variable

We recommend removing any references to these from your workflows. For more information, see the release notes for CodeQL Action v3.23.0 and v2.23.0.
- Automatically overwrite an existing database if found on the filesystem. [#2229](https://github.com/github/codeql-action/pull/2229)
- Bump the minimum CodeQL bundle version to 2.12.6. [#2232](https://github.com/github/codeql-action/pull/2232)
Expand Down
47 changes: 20 additions & 27 deletions lib/workflow.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/workflow.js.map

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions lib/workflow.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/workflow.test.js.map

Large diffs are not rendered by default.

38 changes: 38 additions & 0 deletions src/workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,44 @@ test("getWorkflowErrors() should not report an error if PRs are totally unconfig
);
});

test("getWorkflowErrors() should not report a warning if there is a workflow_call trigger", async (t) => {
const errors = await getWorkflowErrors(
yaml.load(`
name: "CodeQL"
on:
workflow_call:
`) as Workflow,
await getCodeQLForTesting(),
);

t.deepEqual(...errorCodes(errors, []));
});

test("getWorkflowErrors() should not report a warning if there is a workflow_call trigger as a string", async (t) => {
const errors = await getWorkflowErrors(
yaml.load(`
name: "CodeQL"
on: workflow_call
`) as Workflow,
await getCodeQLForTesting(),
);

t.deepEqual(...errorCodes(errors, []));
});

test("getWorkflowErrors() should not report a warning if there is a workflow_call trigger as an array", async (t) => {
const errors = await getWorkflowErrors(
yaml.load(`
name: "CodeQL"
on:
- workflow_call
`) as Workflow,
await getCodeQLForTesting(),
);

t.deepEqual(...errorCodes(errors, []));
});

test("getCategoryInputOrThrow returns category for simple workflow with category", (t) => {
process.env["GITHUB_REPOSITORY"] = "github/codeql-action-fake-repository";
t.is(
Expand Down
52 changes: 23 additions & 29 deletions src/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ export interface Workflow {
on?: string | string[] | WorkflowTriggers;
}

function isObject(o: unknown): o is object {
return o !== null && typeof o === "object";
}

const GLOB_PATTERN = new RegExp("(\\*\\*?)");

function escapeRegExp(string) {
Expand Down Expand Up @@ -193,37 +189,35 @@ export async function getWorkflowErrors(
}
}

let missingPush = false;
// If there is no push trigger, we will not be able to analyze the default branch.
// So add a warning to the user to add a push trigger.
// If there is a workflow_call trigger, we don't need a push trigger since we assume
// that the workflow_call trigger is called from a workflow that has a push trigger.
const hasPushTrigger = hasWorkflowTrigger("push", doc);
const hasPullRequestTrigger = hasWorkflowTrigger("pull_request", doc);
const hasWorkflowCallTrigger = hasWorkflowTrigger("workflow_call", doc);

if (doc.on === undefined) {
// this is not a valid config
} else if (typeof doc.on === "string") {
if (doc.on === "pull_request") {
missingPush = true;
}
} else if (Array.isArray(doc.on)) {
const hasPush = doc.on.includes("push");
const hasPullRequest = doc.on.includes("pull_request");
if (hasPullRequest && !hasPush) {
missingPush = true;
}
} else if (isObject(doc.on)) {
const hasPush = Object.prototype.hasOwnProperty.call(doc.on, "push");
const hasPullRequest = Object.prototype.hasOwnProperty.call(
doc.on,
"pull_request",
);
if (hasPullRequestTrigger && !hasPushTrigger && !hasWorkflowCallTrigger) {
errors.push(WorkflowErrors.MissingPushHook);
}

if (!hasPush && hasPullRequest) {
missingPush = true;
}
return errors;
}

function hasWorkflowTrigger(triggerName: string, doc: Workflow): boolean {
if (!doc.on) {
return false;
}

if (missingPush) {
errors.push(WorkflowErrors.MissingPushHook);
if (typeof doc.on === "string") {
return doc.on === triggerName;
}

return errors;
if (Array.isArray(doc.on)) {
return doc.on.includes(triggerName);
}

return Object.prototype.hasOwnProperty.call(doc.on, triggerName);
}

export async function validateWorkflow(
Expand Down

0 comments on commit 7d9b7a1

Please sign in to comment.