SCANPY-178: The scanner should respect the SONAR_SCANNER_OPTS environment variable (#219)

Author: joke1196

src/pysonar_scanner/configuration/environment_variables.py

@@ -63,4 +63,6 @@ def load_properties_env_variables():
         env_var_name = prop.env_variable_name()
         if env_var_name in os.environ:
             properties[prop.name] = os.environ[env_var_name]
+            if prop.deprecation_message:
+                logging.warning(prop.deprecation_message)
     return properties

src/pysonar_scanner/configuration/properties.py

@@ -17,8 +17,8 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
-import time
 import argparse
+import time
 from dataclasses import dataclass
 from typing import Any, Callable, Optional
 
@@ -93,14 +93,17 @@
 SONAR_WORKING_DIRECTORY: Key = "sonar.working.directory"
 SONAR_SCM_FORCE_RELOAD_ALL: Key = "sonar.scm.forceReloadAll"
 SONAR_MODULES: Key = "sonar.modules"
-SONAR_PYTHON_XUNIT_REPORT_PATH = "sonar.python.xunit.reportPath"
-SONAR_PYTHON_XUNIT_SKIP_DETAILS = "sonar.python.xunit.skipDetails"
-SONAR_PYTHON_MYPY_REPORT_PATHS = "sonar.python.mypy.reportPaths"
-SONAR_PYTHON_BANDIT_REPORT_PATHS = "sonar.python.bandit.reportPaths"
-SONAR_PYTHON_FLAKE8_REPORT_PATHS = "sonar.python.flake8.reportPaths"
-SONAR_PYTHON_RUFF_REPORT_PATHS = "sonar.python.ruff.reportPaths"
+SONAR_PYTHON_XUNIT_REPORT_PATH: Key = "sonar.python.xunit.reportPath"
+SONAR_PYTHON_XUNIT_SKIP_DETAILS: Key = "sonar.python.xunit.skipDetails"
+SONAR_PYTHON_MYPY_REPORT_PATHS: Key = "sonar.python.mypy.reportPaths"
+SONAR_PYTHON_BANDIT_REPORT_PATHS: Key = "sonar.python.bandit.reportPaths"
+SONAR_PYTHON_FLAKE8_REPORT_PATHS: Key = "sonar.python.flake8.reportPaths"
+SONAR_PYTHON_RUFF_REPORT_PATHS: Key = "sonar.python.ruff.reportPaths"
 TOML_PATH: Key = "toml-path"
 
+# ============ DEPRECATED ==============
+SONAR_SCANNER_OPTS: Key = "sonar.scanner.opts"
+
 
 @dataclass
 class Property:
@@ -113,6 +116,8 @@ class Property:
     cli_getter: Optional[Callable[[argparse.Namespace], Any]] = None
     """function to get the value from the CLI arguments namespace. If None, the property is not settable via CLI"""
 
+    deprecation_message: Optional[str] = None
+
     def python_name(self) -> str:
         """Convert Java-style camel case name to Python-style kebab-case name."""
         result = []
@@ -524,6 +529,12 @@ def env_variable_name(self) -> str:
         name=SONAR_MODULES,
         default_value=None,
         cli_getter=lambda args: args.sonar_modules
+    ),
+    Property(
+        name=SONAR_SCANNER_OPTS,
+        default_value=None,
+        cli_getter=None,
+        deprecation_message="SONAR_SCANNER_OPTS is deprecated, please use SONAR_SCANNER_JAVA_OPTS instead."
     )
 ]
 # fmt: on

src/pysonar_scanner/scannerengine.py

@@ -20,15 +20,18 @@
 import json
 import logging
 import pathlib
-from dataclasses import dataclass
 import shlex
-from subprocess import Popen, PIPE
+from dataclasses import dataclass
+from subprocess import PIPE, Popen
 from threading import Thread
 from typing import IO, Any, Callable, Optional
 
 from pysonar_scanner.api import EngineInfo, SonarQubeApi
 from pysonar_scanner.cache import Cache, CacheFile
-from pysonar_scanner.configuration.properties import SONAR_SCANNER_JAVA_OPTS
+from pysonar_scanner.configuration.properties import (
+    SONAR_SCANNER_JAVA_OPTS,
+    SONAR_SCANNER_OPTS,
+)
 from pysonar_scanner.exceptions import ChecksumException
 from pysonar_scanner.jre import JREResolvedPath
 
@@ -148,6 +151,7 @@ def __init__(self, jre_path: JREResolvedPath, scanner_engine_path: pathlib.Path)
     def run(self, config: dict[str, Any]):
         # Extract Java options if present; they must influence the JVM invocation, not the scanner engine itself
         java_opts = config.get(SONAR_SCANNER_JAVA_OPTS)
+        java_opts = config.get(SONAR_SCANNER_OPTS) if not java_opts else java_opts
 
         cmd = self.__build_command(self.jre_path, self.scanner_engine_path, java_opts)
         logging.debug(f"Command: {cmd}")
@@ -173,7 +177,11 @@ def __build_command(
 
     def __config_to_json(self, config: dict[str, Any]) -> str:
         # SONAR_SCANNER_JAVA_OPTS are properties that shouldn't be passed to the engine, only to the JVM
-        scanner_properties = [{"key": k, "value": v} for k, v in config.items() if k != SONAR_SCANNER_JAVA_OPTS]
+        scanner_properties = [
+            {"key": k, "value": v}
+            for k, v in config.items()
+            if k != SONAR_SCANNER_JAVA_OPTS and k != SONAR_SCANNER_OPTS
+        ]
         return json.dumps({"scannerProperties": scanner_properties})
 
     def __decompose_java_opts(self, java_opts: str) -> list[str]:

tests/test_environment_variables.py

@@ -31,6 +31,7 @@
     SONAR_TOKEN,
     SONAR_USER_HOME,
     SONAR_PROJECT_KEY,
+    SONAR_SCANNER_OPTS,
 )
 
 
@@ -134,3 +135,27 @@ def test_environment_variables_priority_over_json_params(self):
             }
             self.assertEqual(len(properties), 3)
             self.assertDictEqual(properties, expected_properties)
+
+    @patch("pysonar_scanner.configuration.environment_variables.logging")
+    def test_SONAR_SCANNER_OPTS(self, mock_logging):
+        env = {
+            "SONAR_TOKEN": "my-token",
+            "SONAR_HOST_URL": "https://sonarqube.example.com",
+            "SONAR_USER_HOME": "/custom/sonar/home",
+            "SONAR_SCANNER_OPTS": "-Xmx1024m -XX:MaxPermSize=256m",
+            "SONAR_REGION": "us",
+        }
+        with patch.dict("os.environ", env, clear=True):
+            properties = environment_variables.load()
+            expected_properties = {
+                SONAR_TOKEN: "my-token",
+                SONAR_HOST_URL: "https://sonarqube.example.com",
+                SONAR_USER_HOME: "/custom/sonar/home",
+                SONAR_SCANNER_OPTS: "-Xmx1024m -XX:MaxPermSize=256m",
+                SONAR_REGION: "us",
+            }
+            self.assertEqual(len(properties), 5)
+            self.assertDictEqual(properties, expected_properties)
+            mock_logging.warning.assert_called_once_with(
+                "SONAR_SCANNER_OPTS is deprecated, please use SONAR_SCANNER_JAVA_OPTS instead.",
+            )

tests/unit/test_scannerengine.py

@@ -22,14 +22,15 @@
 import pathlib
 import unittest
 from subprocess import PIPE
-from unittest.mock import Mock
-from unittest.mock import patch, MagicMock
+from unittest.mock import MagicMock, Mock, patch
 
 import pyfakefs.fake_filesystem_unittest as pyfakefs
 
-from pysonar_scanner import cache
-from pysonar_scanner import scannerengine
-from pysonar_scanner.configuration.properties import SONAR_SCANNER_JAVA_OPTS
+from pysonar_scanner import cache, scannerengine
+from pysonar_scanner.configuration.properties import (
+    SONAR_SCANNER_JAVA_OPTS,
+    SONAR_SCANNER_OPTS,
+)
 from pysonar_scanner.exceptions import ChecksumException
 from pysonar_scanner.scannerengine import (
     LogLine,
@@ -48,7 +49,10 @@ def test_without_stacktrace(self):
     def test_with_stacktrace(self):
         line = '{"level":"INFO","message":"a message", "stacktrace":"a stacktrace"}'
         log_line = scannerengine.parse_log_line(line)
-        self.assertEqual(log_line, LogLine(level="INFO", message="a message", stacktrace="a stacktrace"))
+        self.assertEqual(
+            log_line,
+            LogLine(level="INFO", message="a message", stacktrace="a stacktrace"),
+        )
 
     def test_invalid_json(self):
         line = '"level":"INFO","message":"a message", "stacktrace":"a stacktrace"}'
@@ -62,7 +66,6 @@ def test_no_level(self):
 
 
 class TestCmdExecutor(unittest.TestCase):
-
     @patch("pysonar_scanner.scannerengine.Popen")
     def test_execute_successful(self, mock_popen):
         mock_process = MagicMock()
@@ -119,11 +122,17 @@ def test_to_logging_level(self):
         self.assertEqual(LogLine(level="UNKNOWN", message="").get_logging_level(), logging.INFO)
 
     def test_default_log_line_listener(self):
-        with self.subTest("log line without stacktrace"), self.assertLogs(level="INFO") as logs:
+        with (
+            self.subTest("log line without stacktrace"),
+            self.assertLogs(level="INFO") as logs,
+        ):
             scannerengine.default_log_line_listener(LogLine(level="INFO", message="info1", stacktrace=None))
             self.assertEqual(logs.output, ["INFO:root:info1"])
 
-        with self.subTest("log line with stacktrace"), self.assertLogs(level="INFO") as logs:
+        with (
+            self.subTest("log line with stacktrace"),
+            self.assertLogs(level="INFO") as logs,
+        ):
             default_log_line_listener(LogLine(level="WARN", message="info2", stacktrace="a stacktrace"))
             self.assertEqual(logs.output, ["WARNING:root:info2", "WARNING:root:a stacktrace"])
 
@@ -161,7 +170,8 @@ def test_command_building(self, execute_mock):
         scannerengine.ScannerEngine(jre_resolve_path_mock, scanner_engine_mock).run(config)
 
         execute_mock.assert_called_once_with(
-            [str(java_path), "-jar", str(pathlib.Path("/test/scanner-engine.jar"))], expected_std_in
+            [str(java_path), "-jar", str(pathlib.Path("/test/scanner-engine.jar"))],
+            expected_std_in,
         )
 
     @patch("pysonar_scanner.scannerengine.CmdExecutor")
@@ -280,6 +290,61 @@ def test_java_opts_edge_cases(self, execute_mock):
                 self.assertEqual(actual_command[-2], "-jar")
                 self.assertEqual(actual_command[-1], str(scanner_engine_mock))
 
+    @patch("pysonar_scanner.scannerengine.CmdExecutor")
+    def test_command_building_with_scanner_opts(self, execute_mock):
+        config = {
+            "sonar.token": "myToken",
+            "sonar.projectKey": "myProjectKey",
+            SONAR_SCANNER_OPTS: "-Xmx1024m -XX:MaxPermSize=256m",
+        }
+
+        java_path = pathlib.Path("jre/bin/java")
+        jre_resolve_path_mock = Mock()
+        jre_resolve_path_mock.path = java_path
+        scanner_engine_mock = pathlib.Path("/test/scanner-engine.jar")
+
+        scannerengine.ScannerEngine(jre_resolve_path_mock, scanner_engine_mock).run(config)
+
+        called_args = execute_mock.call_args[0]
+        actual_command = called_args[0]
+
+        expected_command = [
+            str(java_path),
+            "-Xmx1024m",
+            "-XX:MaxPermSize=256m",
+            "-jar",
+            str(scanner_engine_mock),
+        ]
+        self.assertEqual(actual_command, expected_command)
+
+    @patch("pysonar_scanner.scannerengine.CmdExecutor")
+    def test_command_ignore_scanner_opts_for_java_opts(self, execute_mock):
+        config = {
+            "sonar.token": "myToken",
+            "sonar.projectKey": "myProjectKey",
+            SONAR_SCANNER_OPTS: "-Xmx512m -XX:MaxPermSize=128m",
+            SONAR_SCANNER_JAVA_OPTS: "-Xmx1024m -XX:MaxPermSize=256m",
+        }
+
+        java_path = pathlib.Path("jre/bin/java")
+        jre_resolve_path_mock = Mock()
+        jre_resolve_path_mock.path = java_path
+        scanner_engine_mock = pathlib.Path("/test/scanner-engine.jar")
+
+        scannerengine.ScannerEngine(jre_resolve_path_mock, scanner_engine_mock).run(config)
+
+        called_args = execute_mock.call_args[0]
+        actual_command = called_args[0]
+
+        expected_command = [
+            str(java_path),
+            "-Xmx1024m",
+            "-XX:MaxPermSize=256m",
+            "-jar",
+            str(scanner_engine_mock),
+        ]
+        self.assertEqual(actual_command, expected_command)
+
 
 class TestScannerEngineProvisioner(pyfakefs.TestCase):
     def setUp(self):
@@ -304,7 +369,9 @@ def test_happy_path(self):
     def test_happy_path_with_download_url(self):
         with sq_api_utils.sq_api_mocker() as mocker:
             mocker.mock_analysis_engine(
-                filename="scanner-engine.jar", sha256=self.test_file_checksum, download_url="http://example.com"
+                filename="scanner-engine.jar",
+                sha256=self.test_file_checksum,
+                download_url="http://example.com",
             )
             mocker.mock_download_url(url="http://example.com", body=self.test_file_content)
 
@@ -331,7 +398,10 @@ def test_scanner_engine_is_cached(self):
             self.assertEqual(engine_download_rsps.call_count, 0)
 
     def test_checksum_is_invalid(self):
-        with self.assertRaises(ChecksumException), sq_api_utils.sq_api_mocker() as mocker:
+        with (
+            self.assertRaises(ChecksumException),
+            sq_api_utils.sq_api_mocker() as mocker,
+        ):
             mocker.mock_analysis_engine(filename="scanner-engine.jar", sha256="invalid-checksum")
             mocker.mock_analysis_engine_download(body=self.test_file_content)