refactor: Add log_std(out|err) bools to repo_utils that execute a subprocess (#2817)
While making a local patch to work around #2640, I found that I had a
need for running a subprocess (`gcloud auth print-access-token`) via
`repo_utils.execute_checked_stdout`. However, doing so would log that
access token when debug logging was enabled via
`RULES_PYTHON_REPO_DEBUG=1`. This is a security concern for us, so I
hacked in an option to allow a particular `execute_(un)checked(_stdout)`
call to disable logging stdout, stderr, or both.
I figure this might be useful to others so I thought I'd upstream it.
`execute_(un)checked(_stdout)` now support `log_stdout` and `log_stderr`
bools that default to `True` (which is the same behavior as before this
PR.
When the subprocess writes to stdout and `log_stdout = False`, the
logged message will show:
```
===== stdout start =====
<log_stdout = False; skipping>
===== stdout end =====
```
If the subprocess does not write to stdout, the debug log shows the same
as before:
```
<stdout empty>
```
The above also applies for stderr, with text adjusted accordingly.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8d11187..88defb8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -77,7 +77,9 @@
{#v0-0-0-added}
### Added
-* Nothing added.
+* Repo utilities `execute_unchecked`, `execute_checked`, and `execute_checked_stdout` now
+ support `log_stdout` and `log_stderr` keyword arg booleans. When these are `True`
+ (the default), the subprocess's stdout/stderr will be logged.
{#v0-0-0-removed}
### Removed
diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl
index 73883a9..eee56ec 100644
--- a/python/private/repo_utils.bzl
+++ b/python/private/repo_utils.bzl
@@ -98,6 +98,8 @@
arguments,
environment = {},
logger = None,
+ log_stdout = True,
+ log_stderr = True,
**kwargs):
"""Execute a subprocess with debugging instrumentation.
@@ -116,6 +118,10 @@
logger: optional `Logger` to use for logging execution details. Must be
specified when using module_ctx. If not specified, a default will
be created.
+ log_stdout: If True (the default), write stdout to the logged message. Setting
+ to False can be useful for large stdout messages or for secrets.
+ log_stderr: If True (the default), write stderr to the logged message. Setting
+ to False can be useful for large stderr messages or for secrets.
**kwargs: additional kwargs to pass onto rctx.execute
Returns:
@@ -160,7 +166,7 @@
cwd = _cwd_to_str(mrctx, kwargs),
timeout = _timeout_to_str(kwargs),
env_str = _env_to_str(environment),
- output = _outputs_to_str(result),
+ output = _outputs_to_str(result, log_stdout = log_stdout, log_stderr = log_stderr),
))
elif _is_repo_debug_enabled(mrctx):
logger.debug((
@@ -171,7 +177,7 @@
op = op,
status = "success" if result.return_code == 0 else "failure",
return_code = result.return_code,
- output = _outputs_to_str(result),
+ output = _outputs_to_str(result, log_stdout = log_stdout, log_stderr = log_stderr),
))
result_kwargs = {k: getattr(result, k) for k in dir(result)}
@@ -183,6 +189,8 @@
mrctx = mrctx,
kwargs = kwargs,
environment = environment,
+ log_stdout = log_stdout,
+ log_stderr = log_stderr,
),
**result_kwargs
)
@@ -220,7 +228,16 @@
"""Calls execute_checked, but only returns the stdout value."""
return _execute_checked(*args, **kwargs).stdout
-def _execute_describe_failure(*, op, arguments, result, mrctx, kwargs, environment):
+def _execute_describe_failure(
+ *,
+ op,
+ arguments,
+ result,
+ mrctx,
+ kwargs,
+ environment,
+ log_stdout = True,
+ log_stderr = True):
return (
"repo.execute: {op}: failure:\n" +
" command: {cmd}\n" +
@@ -236,7 +253,7 @@
cwd = _cwd_to_str(mrctx, kwargs),
timeout = _timeout_to_str(kwargs),
env_str = _env_to_str(environment),
- output = _outputs_to_str(result),
+ output = _outputs_to_str(result, log_stdout = log_stdout, log_stderr = log_stderr),
)
def _which_checked(mrctx, binary_name):
@@ -331,11 +348,11 @@
def _timeout_to_str(kwargs):
return kwargs.get("timeout", "<default timeout>")
-def _outputs_to_str(result):
+def _outputs_to_str(result, log_stdout = True, log_stderr = True):
lines = []
items = [
- ("stdout", result.stdout),
- ("stderr", result.stderr),
+ ("stdout", result.stdout if log_stdout else "<log_stdout = False; skipping>"),
+ ("stderr", result.stderr if log_stderr else "<log_stderr = False; skipping>"),
]
for name, content in items:
if content: