-
Notifications
You must be signed in to change notification settings - Fork 451
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Config
- Loading branch information
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { performance } from "perf_hooks"; | |
| import * as yaml from "js-yaml"; | ||
| import * as semver from "semver"; | ||
|
|
||
| import { isAnalyzingPullRequest } from "./actions-util"; | ||
| import { getActionVersion, isAnalyzingPullRequest } from "./actions-util"; | ||
| import { | ||
| AnalysisConfig, | ||
| AnalysisKind, | ||
|
|
@@ -102,6 +102,10 @@ interface IncludeQueryFilter { | |
| * Format of the parsed config file. | ||
| */ | ||
| export interface Config { | ||
| /** | ||
| * The version of the CodeQL Action that the configuration is for. | ||
| */ | ||
| version: string; | ||
| /** | ||
| * Set of analysis kinds that are enabled. | ||
| */ | ||
|
|
@@ -591,6 +595,7 @@ export async function initActionState( | |
| ); | ||
|
|
||
| return { | ||
| version: getActionVersion(), | ||
| analysisKinds, | ||
| languages, | ||
| buildMode, | ||
|
|
@@ -1308,7 +1313,21 @@ export async function getConfig( | |
| const configString = fs.readFileSync(configFile, "utf8"); | ||
| logger.debug("Loaded config:"); | ||
| logger.debug(configString); | ||
| return JSON.parse(configString) as Config; | ||
|
|
||
| const config = JSON.parse(configString) as Partial<Config>; | ||
|
|
||
| if (config.version === undefined) { | ||
| throw new ConfigurationError( | ||
| `Loaded configuration file, but it does not contain the expected 'version' field.`, | ||
| ); | ||
| } | ||
| if (config.version !== getActionVersion()) { | ||
| throw new ConfigurationError( | ||
| `Loaded a configuration file for version '${config.version}', but running version '${getActionVersion()}'`, | ||
| ); | ||
|
Comment on lines
+1324
to
+1327
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a different message than the one produced by
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| } | ||
|
|
||
| return config as Config; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.