-
Notifications
You must be signed in to change notification settings - Fork 80
IAC_SCAN Determined Only by detect.tools and detect.tools.excluded Property #1548
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
base: master
Are you sure you want to change the base?
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 refactors the IAC_SCAN tool selection logic to ensure that only the detect.tools
and detect.tools.excluded
properties determine whether IAC_SCAN runs, removing the previous dependency on detect.iac.scan.paths
.
- Replaces inline IAC enablement logic with a dedicated
isIacScanEnabled
helper method - Removes the condition that checked
detect.iac.scan.paths
for enabling IAC_SCAN - Implements clear logic that respects tool inclusion/exclusion properties exclusively
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
return new DetectToolFilter(filter, impactEnabled.orElse(false), iacEnabled, runDecision, blackDuckDecision); | ||
} | ||
|
||
private boolean isIacScanEnabled(AllNoneEnumCollection<DetectTool> includedTools, AllNoneEnumCollection<DetectTool> excludedTools) { |
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.
Have we considered writing a log message in case someone sets iac.scan.path and detect.tools both but the later does not have IAC_SCAN in its value? It could be the case that some users might be running iac scans with this configuration currently.
AllNoneEnumCollection<DetectTool> excludedTools = detectConfiguration.getValue(DetectProperties.DETECT_TOOLS_EXCLUDED); | ||
ExcludeIncludeEnumFilter<DetectTool> filter = new ExcludeIncludeEnumFilter<>(excludedTools, includedTools, scanTypeEvidenceMap); | ||
|
||
boolean iacEnabled = includedTools.containsValue(DetectTool.IAC_SCAN) || !detectConfiguration.getValue(DetectProperties.DETECT_IAC_SCAN_PATHS).isEmpty(); |
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.
Please take a look at the help message for DETECT_IAC_SCAN_PATHS
in DetectProperties
and update as needed.
Right now it mentions something regarding detect.tools=ALL
(it was added when the bug was discovered).
Could you share how you decided to implement the additional logic instead of the approach in the draft PR https://github.com/blackducksoftware/detect/pull/1335/files ? I'm wondering because here we seem to be re-implementing what |
JIRA Ticket:
IDETECT-4591
Description:
This merge request updates the logic for enabling the
IAC_SCAN
tool so that onlydetect.tools
anddetect.tools.excluded
determine whetherIAC_SCAN
runs. The propertydetect.iac.scan.paths
no longer affects tool selection, resolving issues whereIAC_SCAN
could run or be skipped incorrectly.The helper method
isIacScanEnabled
now encapsulates this logic, ensuring that:IAC_SCAN
is enabled ifdetect.tools
is set toALL
, unset, or explicitly includesIAC_SCAN
, and is not excluded bydetect.tools.excluded
.detect.iac.scan.paths
does not affect whetherIAC_SCAN
runs.Acceptance Criteria:
detect.iac.scan.paths
has no effect on whetherIAC_SCAN
is run.detect.tools
anddetect.tools.excluded
determineIAC_SCAN
tool selection.detect.tools.excluded=IAC_SCAN
,IAC_SCAN
will not run, regardless of other properties.detect.tools=ALL
or is unset, andIAC_SCAN
is not excluded,IAC_SCAN
will run.detect.tools=IAC_SCAN
andIAC_SCAN
is not excluded,IAC_SCAN
will run.N.B: This merge request is aimed for detect 11.0 release.