Avoid warning on workflow_call triggers - #2274
Conversation
Sorry, something went wrong.
| // Avoid warning when there this action was triggered via workflow_call since | ||
| // the user has a custom workflow that is calling this action and we assume | ||
| // they know what they are doing. | ||
| const isWorkflowCall = github.context.eventName === "workflow_call"; |
There was a problem hiding this comment.
I'm not certain this will work. If you're in a callee workflow, we only see the name of the event that started the caller workflow. I think you'll have to use other heuristics to parse the current workflow (which will be the caller) to recognise that it contains a workflow call, or simply doesn't contain a github/codeql-action step (which indicates that step is in the callee).
Sorry, something went wrong.
There was a problem hiding this comment.
I think Aditya is right, and we want to look for a workflow_call trigger, i.e. workflow_call appearing in the on property (note that this property may be a string or an array).
Sorry, something went wrong.
There was a problem hiding this comment.
We just need to look for the presence of the property, IIUC. The value can also be null. Eg, this is legal:
on: workflow_call:
Sorry, something went wrong.
There was a problem hiding this comment.
How does on: workflow_call get parsed? I assumed it was a string.
Sorry, something went wrong.
There was a problem hiding this comment.
It is parsed as null:
Sorry, something went wrong.
There was a problem hiding this comment.
Ah...I see you were saying something different. We need to handle the following formats as well:
on: workflow_call
on: - workflow_call
Sorry, something went wrong.
|
I implemented the suggested changes, made a refactoring to make things a bit easier to understand, and added a handful of tests to verify. |
Sorry, something went wrong.
Typically, we warn when there is no `push` trigger in the workflow file that triggered this run. However, when this action is triggered by a `workflow_call` event, we assume there is a custom process for triggering the action and we don't want to warn in this case.
henrymercer
left a comment
There was a problem hiding this comment.
Nice!
Sorry, something went wrong.
| return doc.on.includes(triggerName); | ||
| } | ||
|
|
||
| return Object.prototype.hasOwnProperty.call(doc.on, triggerName); |
There was a problem hiding this comment.
I think we can simplify this:
| return Object.prototype.hasOwnProperty.call(doc.on, triggerName); | |
| return Object.hasOwn(doc.on, triggerName); |
Sorry, something went wrong.
There was a problem hiding this comment.
Nice. I didn't know about this function.
It's only available on node 16.9 and later. Are we sure that all supported GHES runners use that version or later?
Also, we need to update our tsconfig, or else we ge tlinter errors. So, I'm going to hold off for now and we can make the tsconfig changes later.
Sorry, something went wrong.
Typically, we warn when there is no push trigger in the workflow file that triggered this run. However, when this action is triggered by a workflow_call event, we assume there is a custom process for triggering the action and we don't want to warn in this case.
Merge / deployment checklist