-
Notifications
You must be signed in to change notification settings - Fork 424
Overlay: Add feature flag to skip resource checks #3333
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
47b5fac to
3aa1074
Compare
3aa1074 to
1ffb7dd
Compare
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 a new feature flag OverlayAnalysisSkipResourceChecks that allows the CodeQL Action to bypass disk space and memory requirements for overlay analysis. This enables testing and evaluation of overlay analysis on runners that don't meet the standard minimum requirements (20GB disk space and 5GB memory), such as standard GitHub-hosted runners.
Key Changes
- Introduced new feature flag
Feature.OverlayAnalysisSkipResourceCheckswith default valuefalse - Refactored resource checking logic into a dedicated
runnerSupportsOverlayAnalysis()function - Modified
getOverlayDatabaseMode()to conditionally skip resource checks when the flag is enabled
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/feature-flags.ts | Adds the new OverlayAnalysisSkipResourceChecks feature flag definition with environment variable support |
| src/config-utils.ts | Extracts resource checking logic into runnerSupportsOverlayAnalysis() and conditionally applies checks based on the feature flag |
| src/config-utils.test.ts | Adds comprehensive test coverage for the new flag in both PR and default branch scenarios with insufficient disk/memory |
| lib/*.js | Auto-generated JavaScript compiled from TypeScript sources (not reviewed per guidelines) |
mbg
left a comment
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 also looks good. Just a minor comment about the order of the FF definitions.
| OverlayAnalysisSwift = "overlay_analysis_swift", | ||
| OverlayAnalysisSkipResourceChecks = "overlay_analysis_skip_resource_checks", |
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.
Minor: We try to keep the FFs in alphabetical order, so I think this should come earlier on in the list.
| envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_SWIFT", | ||
| minimumVersion: undefined, | ||
| }, | ||
| [Feature.OverlayAnalysisSkipResourceChecks]: { |
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.
Minor: Same point about alphabetical order.
| return true; | ||
| } | ||
|
|
||
| async function runnerSupportsOverlayAnalysis( |
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.
Minor: A docs comment for this function would be nice.
This PR introduces a new feature flag that will cause the action to skip resource checks (disk and memory) for overlay analysis when enabled. The motivation is to ensure that we can continue to evaluate overlay analysis with runners that don't satisfy the minimum resource requirements, such as standard GitHub-hosted runners that do not satisfy the current minimum for available disk space.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, CCR, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.com.How did/will you validate this change?
.test.tsfiles).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist