Skip to content

Conversation

zahidblackduck
Copy link
Collaborator

@zahidblackduck zahidblackduck commented Sep 19, 2025

JIRA Ticket:
IDETECT-4591

Description:
This merge request updates the logic for enabling the IAC_SCAN tool so that only detect.tools and detect.tools.excluded determine whether IAC_SCAN runs. The property detect.iac.scan.paths no longer affects tool selection, resolving issues where IAC_SCAN could run or be skipped incorrectly.

The helper method isIacScanEnabled now encapsulates this logic, ensuring that:

  • IAC_SCAN is enabled if detect.tools is set to ALL, unset, or explicitly includes IAC_SCAN, and is not excluded by detect.tools.excluded.
  • The value of detect.iac.scan.paths does not affect whether IAC_SCAN runs.

Acceptance Criteria:

  • detect.iac.scan.paths has no effect on whether IAC_SCAN is run.
  • Only detect.tools and detect.tools.excluded determine IAC_SCAN tool selection.
  • If detect.tools.excluded=IAC_SCAN, IAC_SCAN will not run, regardless of other properties.
  • If detect.tools=ALL or is unset, and IAC_SCAN is not excluded, IAC_SCAN will run.
  • If detect.tools=IAC_SCAN and IAC_SCAN is not excluded, IAC_SCAN will run.

N.B: This merge request is aimed for detect 11.0 release.

@zahidblackduck zahidblackduck self-assigned this Sep 19, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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) {
Copy link
Contributor

@devmehtabd devmehtabd Sep 19, 2025

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();
Copy link
Contributor

@andrian-sevastyanov andrian-sevastyanov Sep 19, 2025

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).

@andrian-sevastyanov
Copy link
Contributor

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 ExcludeIncludeEnumFilter in DetectToolFilter already does. Please correct me if I'm wrong; but, if this is true then we should try to reuse existing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants