-
Notifications
You must be signed in to change notification settings - Fork 384
Store and check action version in Config
#3100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds version checking to the CodeQL Action configuration system by storing the action version in the Config
object and validating it when loading configurations. This ensures consistency across workflow executions by preventing version mismatches.
Key changes:
- Added a
version
field to theConfig
interface that stores the CodeQL Action version - Implemented version validation in
getConfig()
that throws aConfigurationError
on version mismatches - Updated test utilities and test cases to include the version field
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/config-utils.ts | Added version field to Config interface, version storage in initConfig, and version validation in getConfig |
src/testing-utils.ts | Updated createTestConfig helper to include the action version |
src/config-utils.test.ts | Added test case for version mismatch scenarios and updated existing tests to include version field |
lib/*.js | Generated JavaScript files (auto-generated, not reviewed per guidelines) |
a66b78f
to
a59a92f
Compare
a59a92f
to
4f56152
Compare
Co-authored-by: Copilot <[email protected]>
if (config.version !== getActionVersion()) { | ||
throw new ConfigurationError( | ||
`Loaded a configuration file for version '${config.version}', but running version '${getActionVersion()}'`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a different message than the one produced by /init
in the other PR. But this is acceptable, we don't really expect to hit this case once /init
has been used in the wild for a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense for the messages to be different. In the other PR, it's a warning only and it is based on the ref used for the Action. Here, it's an error and based on the CodeQL Action version. In theory, the same CodeQL Action version can be associated with multiple different commits, so this isn't a perfect safeguard against mixing different commits in a workflow, and we may want to revisit that if we think it's enough of a problem.
In any case, since the conditions aren't the same, we probably want these to be different messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Consider adding a changelog note given that we're adding a new type of error, even though affected setups are unusual.
Stores the CodeQL Action version in
Config
. When loading theConfig
, we check that theversion
value matches the CodeQL Action version. If not, we throw aConfigurationError
.This ensures that the version of the CodeQL Action used in a given workflow is consistent.
Risk assessment
For internal use only. Please select the risk level of this change:
Merge / deployment checklist