Integration test for daemon environment scrubbing (#5893)

This shows that #5898 works, which itself fixed #5854

src/python/pants/util/collections.py

@@ -9,3 +9,13 @@
 def combined_dict(*dicts):
   """Combine one or more dicts into a new, unified dict (dicts to the right take precedence)."""
   return {k: v for d in dicts for k, v in d.items()}
+
+
+def recursively_update(d, d2):
+  """dict.update but which merges child dicts (dict2 takes precedence where there's conflict)."""
+  for k, v in d2.items():
+    if k in d:
+      if isinstance(v, dict):
+        recursively_update(d[k], v)
+        continue
+    d[k] = v

tests/python/pants_test/pantsd/test_pantsd_integration.py

@@ -17,7 +17,7 @@
 from concurrent.futures import ThreadPoolExecutor
 
 from pants.pantsd.process_manager import ProcessManager
-from pants.util.collections import combined_dict
+from pants.util.collections import recursively_update
 from pants.util.contextutil import environment_as, temporary_dir
 from pants.util.dirutil import rm_rf, safe_file_dump, safe_mkdir, touch
 from pants_test.pants_run_integration_test import PantsResult, PantsRunIntegrationTest
@@ -96,15 +96,17 @@ def pantsd_test_context(self, log_level='info', extra_config=None, expected_runs
         workdir = os.path.join(workdir_base, '.workdir.pants.d')
         print('\npantsd log is {}/pantsd/pantsd.log'.format(workdir))
         pantsd_config = {
-          'GLOBAL': combined_dict({
+          'GLOBAL': {
             'enable_pantsd': True,
             # The absolute paths in CI can exceed the UNIX socket path limitation
             # (>104-108 characters), so we override that here with a shorter path.
             'watchman_socket_path': '/tmp/watchman.{}.sock'.format(os.getpid()),
             'level': log_level,
             'pants_subprocessdir': pid_dir,
-          }, extra_config or {})
+          }
         }
+        if extra_config:
+          recursively_update(pantsd_config, extra_config)
         print('>>> config: \n{}\n'.format(pantsd_config))
         checker = PantsDaemonMonitor(pid_dir)
         self.assert_success_runner(workdir, pantsd_config, ['kill-pantsd'])
@@ -124,13 +126,14 @@ def pantsd_test_context(self, log_level='info', extra_config=None, expected_runs
           checker.assert_stopped()
 
   @contextmanager
-  def pantsd_successful_run_context(self, log_level='info', extra_config=None):
+  def pantsd_successful_run_context(self, log_level='info', extra_config=None, extra_env=None):
     with self.pantsd_test_context(log_level, extra_config) as (workdir, pantsd_config, checker):
       yield (
         functools.partial(
           self.assert_success_runner,
           workdir,
-          pantsd_config
+          pantsd_config,
+          extra_env=extra_env,
         ),
         checker,
         workdir,
@@ -144,16 +147,18 @@ def _run_count(self, workdir):
     else:
       return 0
 
-  def assert_success_runner(self, workdir, config, cmd, extra_config={}, expected_runs=1):
-    combined_config = combined_dict(config, extra_config)
-    print(bold(cyan('\nrunning: ./pants {} (config={})'
-                    .format(' '.join(cmd), combined_config))))
+  def assert_success_runner(self, workdir, config, cmd, extra_config={}, extra_env={}, expected_runs=1):
+    combined_config = config.copy()
+    recursively_update(combined_config, extra_config)
+    print(bold(cyan('\nrunning: ./pants {} (config={}) (extra_env={})'
+                    .format(' '.join(cmd), combined_config, extra_env))))
     run_count = self._run_count(workdir)
     start_time = time.time()
     run = self.run_pants_with_workdir(
       cmd,
       workdir,
       combined_config,
+      extra_env=extra_env,
       # TODO: With this uncommented, `test_pantsd_run` fails.
       # tee_output=True
     )
@@ -180,10 +185,12 @@ def test_pantsd_compile(self):
 
   def test_pantsd_run(self):
     extra_config = {
+      'GLOBAL': {
         # Muddies the logs with warnings: once all of the warnings in the repository
         # are fixed, this can be removed.
         'glob_expansion_failure': 'ignore',
       }
+    }
     with self.pantsd_successful_run_context(
           'debug',
           extra_config=extra_config
@@ -409,7 +416,7 @@ def test_pantsd_launch_env_var_is_not_inherited_by_pantsd_runner_children(self):
 
   def test_pantsd_invalidation_file_tracking(self):
     test_file = 'testprojects/src/python/print_env/main.py'
-    config = {'pantsd_invalidation_globs': '["testprojects/src/python/print_env/*"]'}
+    config = {'GLOBAL': {'pantsd_invalidation_globs': '["testprojects/src/python/print_env/*"]'}}
     with self.pantsd_successful_run_context(extra_config=config) as (
       pantsd_run, checker, workdir, _
     ):
@@ -529,3 +536,16 @@ def test_pantsd_multiple_parallel_runs(self):
       waiter_result = PantsResult(waiter_pants_command, waiter_pants_process.returncode, waiter_stdout_data.decode("utf-8"),
         waiter_stderr_data.decode("utf-8"), workdir)
       self.assert_success(waiter_result)
+
+  def test_pantsd_environment_scrubbing(self):
+    # This pair of JVM options causes the JVM to always crash, so the command will fail if the env
+    # isn't stripped.
+    with self.pantsd_successful_run_context(
+      extra_config={'compile.zinc': {'jvm_options': ['-Xmx1g']}},
+      extra_env={'_JAVA_OPTIONS': '-Xms2g'},
+    ) as (pantsd_run, checker, workdir, _):
+      pantsd_run(['help'])
+      checker.assert_started()
+
+      result = pantsd_run(['compile', 'examples/src/java/org/pantsbuild/example/hello/simple'])
+      self.assert_success(result)

tests/python/pants_test/util/test_collections.py

@@ -7,7 +7,7 @@
 
 import unittest
 
-from pants.util.collections import combined_dict
+from pants.util.collections import combined_dict, recursively_update
 
 
 class TestCollections(unittest.TestCase):
@@ -20,3 +20,10 @@ def test_combined_dict(self):
       ),
       {'a': 1, 'b': 2, 'c': 3}
     )
+
+  def test_recursively_update(self):
+    d = {'a': 1, 'b': {'c': 2, 'o': 'z'}, 'z': {'y': 0}}
+    recursively_update(d, {'e': 3, 'b': {'f': 4, 'o': 9}, 'g': {'h': 5}, 'z': 7})
+    self.assertEqual(
+      d, {'a': 1, 'b': {'c': 2, 'f': 4, 'o': 9}, 'e': 3, 'g': {'h': 5}, 'z': 7}
+    )