Allow workflow to be passed via an environment variable for default setup - #1619
henrymercer merged 2 commits into
Conversation
Sorry, something went wrong.
angelapwen
left a comment
There was a problem hiding this comment.
Looks good and found the appropriate debug log in the internal run (2nd attempt) linked, but assume you want @Daverlo's 👀 too!
A couple of questions:
- will we add integration tests for default setup at a later point?
- do we want to add a changenote since this is user facing?
Sorry, something went wrong.
Yes! Tracking this internally.
Good point, let's add a changenote for this. |
Sorry, something went wrong.
There was a problem hiding this comment.
Not sure why the PR check is failing though. Re-ran with debug mode.
Edit: looks like we might have a test flake, but because the re-run succeeded I'm not sure what it was!
Sorry, something went wrong.
Daverlo
left a comment
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
| * Get the path of the currently executing workflow. | ||
| * Get the path of the currently executing workflow relative to the repository root. | ||
| */ | ||
| export async function getWorkflowPath(): Promise<string> { |
There was a problem hiding this comment.
What is this this returning when using default setup?
I don't think it really matters as I only see it being used for getting the analysis key, and for default setup we are always passing a category, so this value should be overwritten.
But I'm wondering if this should highlight that there is no workflow file somehow?
Sorry, something went wrong.
There was a problem hiding this comment.
For default setup, this will return dynamic/github-code-scanning/codeql, but as you say we get the analysis key directly from CODEQL_ACTION_ANALYSIS_KEY. I think this is a good thing to highlight though, as this is part of some work I'd like us to do around nailing down in the code what context the CodeQL Action expects from default setup, in terms of the github job context, environment variables, etc.
Sorry, something went wrong.
There was a problem hiding this comment.
At the moment, I don't think we want to fail if the workflow file isn't found — we want to preserve the way we've been generating the analysis key to avoid breaking advanced setups where the workflow file doesn't exist, e.g. reusable workflows or workflows executing in other repositories.
Sorry, something went wrong.
The default setup workflow is not checked into the repository, but the Action relies on the workflow file to make features like reporting failed runs work. We'd like to move to a more robust method of making features like reporting failed runs work, but this is hard to do without making a breaking change to the workflow file, and we aren't currently in a good position to do that.
Therefore as a medium term solution, default setup will pass its workflow file to the Action via the CODE_SCANNING_WORKFLOW_FILE environment variable.
GitHub staff: see an example run and the resulting status page.
Merge / deployment checklist