IAC_SCAN Determined Only by detect.tools and detect.tools.excluded Property #1548
Conversation
There was a problem hiding this comment.
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
isIacScanEnabledhelper method - Removes the condition that checked
detect.iac.scan.pathsfor 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.
zahidblackduck
requested review from
andrian-sevastyanov,
bd-samratmuk,
devmehtabd,
dterrybd and
shantyk
| 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.
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.
There was a problem hiding this comment.
Removed this method altogether and updated as per Andrian's suggestion.
| 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.
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 |
The initial idea was to keep the existing boolean |
If you merge this PR, please close the other one mentioning a comment. Thank you. |
JIRA Ticket:
IDETECT-4591
Description:
This merge request updates the logic for enabling the
IAC_SCANtool so that onlydetect.toolsanddetect.tools.excludeddetermine whetherIAC_SCANruns. The propertydetect.iac.scan.pathsno longer affects tool selection, resolving issues whereIAC_SCANcould run or be skipped incorrectly.The helper method
isIacScanEnablednow encapsulates this logic, ensuring that:IAC_SCANis enabled ifdetect.toolsis set toALL, unset, or explicitly includesIAC_SCAN, and is not excluded bydetect.tools.excluded.detect.iac.scan.pathsdoes not affect whetherIAC_SCANruns.Acceptance Criteria:
detect.iac.scan.pathshas no effect on whetherIAC_SCANis run.detect.toolsanddetect.tools.excludeddetermineIAC_SCANtool selection.detect.tools.excluded=IAC_SCAN,IAC_SCANwill not run, regardless of other properties.detect.tools=ALLor is unset, andIAC_SCANis not excluded,IAC_SCANwill run.detect.tools=IAC_SCANandIAC_SCANis not excluded,IAC_SCANwill run.N.B: This merge request is aimed for detect 11.0 release.