whl_library.BuildWheelFromSource uses pip to download dependencies and does not inject credential helper information.
#2640
Comments
|
@dougthor42, I think you had some experiments with adding more deps to the pip parse to include credetial support, but that was somewhat hard to complete. Related, I think we should focus on #2410 so that we don't have a need to extend the pip dependencies to support credentials. Potentially related: #2328, where at some point we removed the |
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Apr 24, 2025
…process (#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.
|
FYI: I'm currently working around this with the following patch, with some things redacted. Hopefully I didn't accidentally remove any lines in the copy-paste-redact or else the patch line numbers will be all mucked up 🙃. diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl
index 630dc851..6189946d 100644
--- a/python/private/pypi/whl_library.bzl
+++ b/python/private/pypi/whl_library.bzl
@@ -289,6 +289,38 @@ def _whl_library_impl(rctx):
if filename.endswith(".whl"):
whl_path = rctx.path(rctx.attr.filename)
else:
+ # HACK ALERT
+ # We need to inject a secret into the pypi index url if we are not using any public
+ # package index.
+ # TODO: Remove this hack when rules_python issue is fixed and released.
+ _index_path = "us-python.pkg.dev/REDACTED/REDACTED/simple"
+ _url = "https://oauth2accesstoken@{}".format(_index_path)
+ if _url in extra_pip_args:
+ print("{} is not a wheel and we are using Airlock. Injecting gcloud access token.".format(filename))
+
+ # N.B.: This is a nontrivial time cost of ~1s/call, but we only have ~13 sdist deps.
+ secret = repo_utils.execute_checked_stdout(
+ rctx,
+ op = "GetGCloudAuthAccessToken",
+ arguments = ["gcloud", "auth", "print-access-token"],
+ log_stdout = False,
+ )
+
+ # extra_pip_args is a list. It's safe enough to assume that the element after
+ # the "--index-url" string is the URL because otherwise the value
+ # would not be attached to the correct CLI arg.
+ # We must remove --index-url from `extra_pip_args` because of variable
+ # precedence: CLI args take precedence over env vars (see
+ # https://pip.pypa.io/en/stable/topics/configuration/#precedence-override-order).
+ extra_pip_args.remove(_url)
+ arg_index = extra_pip_args.index("--index-url")
+ extra_pip_args.remove(extra_pip_args[arg_index])
+
+ prefix = "https://oauth2accesstoken:{}@".format(secret.strip())
+ environment["PIP_INDEX_URL"] = prefix + _index_path
+
+ # END HACK
+
# It is an sdist and we need to tell PyPI to use a file in this directory
# and, allow getting build dependencies from PYTHONPATH, which we
# setup in this repository rule, but still download any necessary
diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl
index eee56ec8..3c8fa287 100644
--- a/python/private/repo_utils.bzl
+++ b/python/private/repo_utils.bzl
@@ -338,10 +338,21 @@ def _cwd_to_str(mrctx, kwargs):
return cwd
def _env_to_str(environment):
- if not environment:
+ # HACK ALERT
+ # TODO: Remove this hack when rules_python issue is fixed and released.
+ _env = dict(environment) # N.B.: .copy not supported
+ if "PIP_INDEX_URL" in environment.keys():
+ # This could definitely be better so that we log something like
+ # "PIP_INDEX_URL=https://oauthaccesstoken:[email protected]/..."
+ # but it's not worth it right now.
+ _env["PIP_INDEX_URL"] = "<REDACTED>"
+
+ # END HACK
+
+ if not _env:
env_str = " <default environment>"
else:
- env_str = "\n".join(["{}={}".format(k, repr(v)) for k, v in environment.items()])
+ env_str = "\n".join(["{}={}".format(k, repr(v)) for k, v in _env.items()])
env_str = "\n" + env_str
return env_str |
|
Have you tried a netrc file? https://pip.pypa.io/en/stable/topics/authentication/#netrc-support pip.parse has a netrc arg: https://rules-python.readthedocs.io/en/latest/api/rules_python/python/extensions/pip.html#pip.parse.netrc |
|
FYI, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
🐞 bug report
Affected Rule
pip.parseand the underlying codeIs this a regression?
Not that I can tell.
Description
When using python package index that requires authentication,
whl_library.BuildWheelFromSourcewill pass that index arg to thepip wheelcommand.This is a problem when using the Bazel downloader (
experimental_index_urlet. al.). Bazel can authenticate to the package index via a credential helper header injection and can successfully download the source tarball. However, rules_python then tries to create a wheel from that tarball and usespip wheel --index-url "${INDEX_NEEDING_AUTH}" .... Thus building the wheel fails because pip can't auth to the private index.🔬 Minimal Reproduction
Hard to repro if you don't have a private index readily available, but the gist is:
experimental_index_urlpygraphvizis a good example of such a package.setuptools) in order to build a wheel.🔥 Exception or Error
🌍 Your Environment
Operating System:
gLinux (based on Debian testing)
Output of
bazel version:Rules_python version:
Anything else relevant?
Internal bug: b/399782261
I think I see a couple potential paths forward:
keyringas an auth provider. This may allow the internalpip wheelcommand to auth to the registry.pip wheel's downloading action with a separate Bazel download task might work, but only if people are using the Bazel downloaderhttps://oauth2accesstoken:${SECRET}@private_index.com/simple. It might be possible to add an attribute topip.parsethat injects that secret.The text was updated successfully, but these errors were encountered: