Support for keyring auth with bzlmod pip parse

[brett-patterson-ent]

🚀 feature request

Relevant Rules

In MODULE.bazel:

pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
pip.parse(
  ...
)

Description

All of our PyPI access is through a private Artifact Registry repo that requires authentication. We typically do this authentication through the use of keyring, which requires the keyring packages to be installed for the Python interpreter that is doing the pip installation.

By default, a hermetic installation of Python won't have these packages installed. While it's a bit of a chicken/egg problem, I've made some progress by taking inspiration from rules_python's internal dependencies and fetching the necessary wheels using http_archive. I was able to get this working with compile_pip_requirements by using the extra_deps argument so that the dependency_resolver.py script runs with the keyring libraries available.

However, I've been unable to get the same approach working with pip.parse so that wheel_installer.py runs with these libraries available. Is there a standard way to do this? Or a way to take a similar approach of bundling additional dependencies into the tool's PYTHONPATH? I found that there was a private attribute specifying this.

Describe the solution you'd like

A way to pipe down additional dependencies to include in the _python_path_entries attribute would work, I think. However, given that the actual pip installation happens in a subprocess I'm not 100% sure of that solution. I'm open to any different approaches that would be better.

Describe alternatives you've considered

It looks like pip.parse does let you pass in an environment argument that (I think) ultimately gets set for the pip subprocess inside of wheel_installer.py. I experimented with setting PYTHONPATH there but was unsuccessful.

I also did get a setup working with WORKSPACE rather than bzlmod where the pip_parse call uses the system interpreter, but I'd like to use bzlmod and not involve the system Python if at all possible.

[dougthor42]

I'm running into the same issue. Can you elaborate on:

I was able to get this working with compile_pip_requirements by using the extra_deps argument so that the dependency_resolver.py script runs with the keyring libraries available.

Perhaps you have a demo/example repo that I can look at?

---

I found that there was a private attribute specifying this.

Is that link still valid? I noticed it's not a permalink so as of right now it points to this alias(name = "flake8", ...) line:

rules_python/python/pip_install/pip_repository.bzl

Line 727 in da10ac4

alias(

---

During my investigation I found that I have access to keyring via subprocess but not via import (which agrees with your assessment of "hermetic python won't have keyring installed).

$ PIP_KEYRING_PROVIDER=subprocess RULES_PYTHON_REPO_DEBUG=1 bazel run --subcommands //:requirements.update
...
  2 location(s) to search for versions of absl-py:
  * https://pypi.org/simple/absl-py/
  * https://us-west2-python.pkg.dev/[REDACTED]/simple/absl-py/
  Fetching project page and analyzing links: https://pypi.org/simple/absl-py/
  Getting page https://pypi.org/simple/absl-py/
  Found index url https://pypi.org/simple/
  Looking up "https://pypi.org/simple/absl-py/" in the cache
  Request header has "max_age" as 0, cache bypassed
  Starting new HTTPS connection (1): pypi.org:443
  https://pypi.org:443 "GET /simple/absl-py/ HTTP/1.1" 304 0
  Fetched page https://pypi.org/simple/absl-py/ as application/vnd.pypi.simple.v1+json
    ...
  Fetching project page and analyzing links: https://us-west2-python.pkg.dev/[REDACTED]/simple/absl-py/
  Getting page https://us-west2-python.pkg.dev/[REDACTED]/simple/absl-py/
  Found index url https://us-west2-python.pkg.dev/[REDACTED]/simple/
  Looking up "https://us-west2-python.pkg.dev/[REDACTED]/simple/absl-py/" in the cache
  Request header has "max_age" as 0, cache bypassed
  Starting new HTTPS connection (1): us-west2-python.pkg.dev:443
  https://us-west2-python.pkg.dev:443 "GET /[REDACTED]/simple/absl-py/ HTTP/1.1" 401 60
  Found index url https://us-west2-python.pkg.dev/[REDACTED]/simple/
  Keyring provider requested: subprocess
  Keyring provider set: subprocess with executable /usr/local/google/home/dthor/.virtualenvs/[REDACTED]/bin/keyring

Without PIP_KEYRING_PROVIDER=subprocess, the 2nd to last log line would say "Keyring provider requested: auto".

Dropping into a debugger shows that keyring is not importable but is usable via subprocess (because I happen to have it installed in the active venv):

...
  Keyring provider requested: subprocess
  Keyring provider set: subprocess with executable /usr/local/google/home/dthor/.virtualenvs/[REDACTED]/bin/keyring
User for us-west2-python.pkg.dev:^C
(Pdb) import keyring
*** ModuleNotFoundError: No module named 'keyring'
(Pdb) import subprocess
(Pdb) subprocess.run(["keyring", "--list-backends"])
keyring.backends.SecretService.Keyring (priority: 5)
keyring.backends.chainer.ChainerBackend (priority: 10)
keyring.backends.fail.Keyring (priority: 0)
keyrings.gauth.GooglePythonAuth (priority: 9)
CompletedProcess(args=['keyring', '--list-backends'], returncode=0

[brett-patterson-ent]

Perhaps you have a demo/example repo that I can look at?

I don't have a public example to reproduce unfortunately, but the general gist for compile_pip_requirements was to fetch keyrings_google_artifactregistry_auth and all transitive dependencies via http_archive rules like rules_python does for its own internal dependencies and pass the labels for the produced py_library targets as extra_deps to compile_pip_requirements. However, I ultimately hit a wall with this approach because the pip.parse interface didn't expose the same way to inject additional dependencies for the wheel installer tool.

I finally worked around this by applying a patch to the rules_python dependency in my MODULE.bazel that adds keyrings_google_artifactregistry_auth and all its transitive dependencies to this list of internal rules_python dependencies and includes the additional requirements here and here. It's not a great long-term solution, but works for now.

Is that link still valid? I noticed it's not a permalink so as of right now it points to this alias(name = "flake8", ...) line:

Sorry about that! Fixed to a permalink to the correct attribute

[dougthor42]

Awesome, thanks for the elaboration and the link fix!

I'm hoping that I can get this addressed.

[dougthor42]

@brett-patterson-ent Now that #1827 was merged (well, commit 4a615be was made but it looks like the PR is still open for some reason), you might not need the keyring dep anymore by using (a) the bazel downloader and (b) a credential helper.

To use the bazel downloader:

 pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
 pip.parse(
+    experimental_extra_index_urls = ["https://example.com/foo/bar/simple"],
+    experimental_index_url = "https://pypi.org/simple",
     hub_name = "pypi",
     python_version = "3.11",
     requirements_lock = "//:requirements_lock.txt",

For the credential helper, check out the docs in #1834

[aignas]

Is there anything left to do in this issue given the above two PRs are merged?

My thinking is that an extra hub repo for keyring can be created and used as extra_deps in the in compile_pip_requirements rule, thus eliminating the need for the patch to rules_python and then bazel downloader can be used as documented here.

I'll close the issue for now, but feel free to reopen if there is work to be done.

[brett-patterson-ent]

Is there anything left to do in this issue given the above two PRs are merged?

My thinking is that an extra hub repo for keyring can be created and used as extra_deps in the in compile_pip_requirements rule, thus eliminating the need for the patch to rules_python and then bazel downloader can be used as documented here.

I'll close the issue for now, but feel free to reopen if there is work to be done.

I haven't had the time to test it out yet, but I think this should work. Using the bazel downloader sounds like a great solution, we already have a credential helper set up for other use cases. I'll report back once I try it out. Thank you @dougthor42 and @aignas!