Add Action input to set CodeQL DB location #505
Conversation
|
|
📚 Thanks for the docs ping! 🛎️ This was added to our docs first-responder project board. A team member will be along shortly to respond. To request changes to the docs you can also open a 
|
@edoardopirovano - thanks for the docs team ping. 🙇🏻♀️ I think this will be easier to discuss and track in the related issue, so I've added that to our first responder board instead. |
src/util.ts
Outdated
| export function getCodeQLDatabasePath(dbDir: string, language: Language) { | ||
| return path.resolve(dbDir, language); |
There was a problem hiding this comment.
Nit: This is a small function. Would it make sense to inline?
There was a problem hiding this comment.
I think it's nice to have everywhere we resolve the location of a database flow through this function since it makes future changes to where databases are located easier.
There was a problem hiding this comment.
Also nit: Should this instead be taking a config, and then internally using config.dbLocation?
src/config-utils.ts
Outdated
| @@ -933,6 +948,19 @@ async function loadConfig( | |||
| } | |||
| } | |||
|
|
|||
| let dbLocation; | |||
| if (DB_LOCATION in parsedYAML) { | |||
There was a problem hiding this comment.
The new field is in the CodeQL YAML config file. I was originally thinking this would be an Actions input to the init or analyze steps (see their action.yml files for the existing inputs), so that the option is directly settable from the Actions workflow YAML, and can reference other Actions context and environment variables.
I'm open to revising that though. What are the advantages of having this in the config file?
There was a problem hiding this comment.
Aha, adding it as an Actions input sounds totally reasonable given the above. I have no strong argument for putting it in the config file. I will change the option to an Actions input.
edoardopirovano
force-pushed
the
db-location
branch
2 times, most recently
from
3cbc563
to
e6c7f81
Compare
edoardopirovano
changed the title
edoardopirovano
force-pushed
the
db-location
branch
3 times, most recently
from
e8e3f58
to
49185d3
Compare
src/util.ts
Outdated
| export function getCodeQLDatabasePath(dbDir: string, language: Language) { | ||
| return path.resolve(dbDir, language); |
There was a problem hiding this comment.
Also nit: Should this instead be taking a config, and then internally using config.dbLocation?
| with: | ||
| db-location: "${{ runner.temp }}/customDbLocation" |
There was a problem hiding this comment.
Have we got a similar test where we use the default?
There was a problem hiding this comment.
All the other tests were using the default location anyway, but I've changed one of them to explicitly check that DBs are created there.
edoardopirovano
force-pushed
the
db-location
branch
2 times, most recently
from
cb2777b
to
8a9e1bb
Compare
edoardopirovano
force-pushed
the
db-location
branch
from
8a9e1bb
to
9b26313
Compare
This adds a new option that lets the user control where the CodeQL databases that the Action creates are placed.
Documentation information
@github/docs-content-codeql Note that this documentation page will need updating: https://docs.github.com/en/code-security/secure-coding/configuring-code-scanning
The new option has the name
db-locationand should contain a path that the CodeQL Action can write to. If provided, this will result in the Action storing the CodeQL databases it creates at that location instead of the default temporary directory. A typical use case for this option would be that the user wishes for the CodeQL database to be in a consistent location so that a later step in their workflow can upload the DB somewhere.We should highlight that it is the responsibility of the user that if they set a non-temporary directory in this configuration option, they should ensure that they clear it in a step later in their workflow when they are done using the database.
Merge / deployment checklist