https://pantsbuild.org/ logo
Join Slack
Powered by
I started looking into <this issue> because it wou...
# development
a
I started looking into this issue because it would be very convenient for me if Pants could handle setting up 
keyring
on its own. However, there's a bit of a thorny bootstrapping problem. My instinct, following the script I have that does this now, is to create a 
keyring
subsystem, which is a 
PythonToolBase
subsystem that can easily export a pex for 
keyring
+ any additional dependencies, like 
keyrings.google-artifactregistry-auth
. However, I believe the keyring pex needs to be added to to the 
PATH
for pex_cli processes, which 
python_tool_base.py
has a dependency on. Furthermore, there's also the issue of needing to call create a pex CLI process to create the keyring pex in the first place. Packaging keyring as an external tool is also problematic because it typically requires other Python dependencies to work with services like Google artifact registry. I also tried only importing the subsystem 
if TYPE_CHECKING
but this doesn't work with the rule decorator. I was curious if anyone had any ideas or opinions on how to work around this.
c
Not sure if we mentioned this in the ticket, but he plan Tom and I were working with was something like: ā€¢ Setup the various keyring injection mechanisms you found ā€¢ Pants does the auth šŸ™€ with some new subsystem. Said subsystem can have whatever deps it needs like a normal pants subsystem ā€¢ Pants injects a binary called 
keyring
into the sandbox that just echos the results
g
Not sure if that is me or Dyas, but that is pretty much how my bitwarden secrets work. It decrypts the secret either as a "package" or a "secret" depending on the ruleset used to get it, and then it can be passed in env, or as a file, or whatever the receiving tool desires. F.ex. for PyPi publishing it's used just like the twine secrets (in env) but not sourced from the parent env, instead coming from another rule which invokes Bitwarden.
a
Pants does the auth šŸ™€ with some new subsystem. Said subsystem can have whatever deps it needs like a normal pants subsystem
So my view is probably somewhat limited given I've only used this in practice with Google artifact registry, but for artifact registry, there's no need to set credentials, it just relies on your global 
gcloud
cli config. Is it different for AWS?
c
AWS uses tokens that last for ~8 to connect to Docker/package registries. So you need to do do something like 
aws some-command
to refresh your credentials. AWS's docs suggest using that to spit out a new 
~/.config/pip/pip.conf
every 8 hours but: ā€¢ That's kinda gross ā€¢ Pants studiously avoids using 
pip.conf
ā€¢ Details are fuzzy, but obviously dont' want the token to end up in the lockfile. An alternative approach is to regen .netrc every 8 hours. That is annoying and fiddly and I thought my less technical users would rebel, but doesn't require any bootstrapping shenanegans.
a
That's frustrating and feels like some AWS specific jankiness!
Could you wrap their keyring provider with a Pants specific one that does the .netrc regen?
c
Maybe! For unrelated to Pants reasons our solution became to not use the AWS Package Registry.
šŸ˜† 1
a
@curved-manchester-66006 I'm taking another stab at this, did you have any thoughts around how the injected keyring binary would echo those results? I'm wondering if we can export the keyring pex to some cached location and then just have the 
keyring
binary be a script that calls said pex.
Another alternative I'm considering is building the 
keyring
pex using a bash script that circumvents the existing rules and circular imports.
c
I'm taking another stab at this, did you have any thoughts around how the injected keyring
Trying to page the context back in, I think the idea was that in the sandbox there would be a file called 
keyring
that was dynamically generated as something like:
Copy code
#!/bin/sh
echo "THE_TOKEN_PANTS_JUST_CALCULATED"
(Or whatever format 
pip
expects to get when calling a 
keyring
binary)
a
I opened a draft PR for this demonstrating the cyclical import issue. It seems like the workaround of doing the imports from within the rules themselves doesn't work with the rule-graph parser. Definitely looking for feedback on this as there's major issues with my draft, starting with the simple fact that it does not work šŸ˜…. But would love to actually have this feature in pants, it would be super helpful for my org - I'm constantly fielding questions on setting up keyring even though there's a script that does exactly that in our repo šŸ™ƒ
f
For some context on the (now closed, unmerged) PRs I had written for the AWS CodeArtifact support operate: ā€¢ https://github.com/pantsbuild/pants/pull/21852 added a 
PexKeyringConfigurationRequest
union which allowed any Pants plugin or backend to supply credentials to Pex invocations. This PR was not AWS-specific. It basically asked plugins for credentials and arranged for a 
keyring
script to essentially echo those credentials. ā—¦ Note: Not quite as simple as an 
echo
though. There was some code involved to try and make sure that varying credentials would not invalidate caching for a 
Process
(assuming all else was the same). This occurs because varying the shell script's content in the input root leads to invalidation given the cache key is computed on basically everything going into a 
Process
. ā—¦ Also, the PR does not deal how to handle 
remote_environment
and 
docker_environent
builds. Some design work would be needed here. ā€¢ https://github.com/pantsbuild/pants/pull/21853 adds AWS CodeArtifact support. It tracks the expiration of the CodeArtifact token and renews it before a run when necessary. It provides an implementation of the 
PexKeyringConfigurationRequest
union. GCP support would just entail building on top of the first PR. I imagine it would be much simpler than the AWS PR since it could obtain the token locally.
Also, no issues from me if anybody wants to reuse the code from closed PRs. They were closed for business reasons, not technical ones.
a
What if we had a called something like 
pants-keyring-helper
that had the ability to store credentials in a cache-accessible location (i.e. named volumes for 
docker_environment
) and we would call that before calling the pex CLI. Then we'd link that on the 
PATH
as 
keyring
and it would be environment aware and know how to find the stored credentials for that environment.
Thinking on this some more, we'd need to do something like this to store the credentials ahead of time
Copy code
from dataclasses import dataclass

from pants.engine.unions import union
from pants.engine.rules import Get


@union
class GenerateCredentialsRequest:
    """Union base class to generate credentials."""


@dataclass(frozen=True)
class CredentialsResult:
    key: str
    contents: bytes


async def generate_credentials(request: GenerateCredentialsRequest) -> None:
    credentials = await Get(CredentialsResult, GenerateCredentialsRequest, request)
    # Call into some (probably Rust?) function to store the credentials outside of a sandbox
    # store_credentials(credentials)
f
The first PR already does that (from Python). It picks a subdirectory of the repository's 
.pants.d
directory and stashes the credentials there.
(That of course doesn't solve the 
docker_environment
issue, but it's an example.)
a
Yeah I was thinking somewhere like 
~/.cache/pants/credstore
or something like that for 
docker_environment
, which we could have the containers mount. However, that probably wouldn't work for remote execution, which I'm assuming just uses local on the remote machine.
f
yeah remote execution is a problematic edge case. It's not really a Pants problem either since the REAPI protocol makes any injected configuration value change the cache key.
for GCP, the server operator could potentially authorize the worker machines to access a repository (and put the credentials in a known location). but the code artifact rotating access token is a bit of a problem with AWS.
a
Given that as you said, if you're using remote execution, you can probably configure the remote machines to authenticate themselves, is it fair to design a solution that solves for local and docker and punts on remote?
f
I believe so in this case, so long as the documentation calls out the issue with remote execution and that it likely requires server operator cooperation to solve.
Still an open question to me: Is the trade off of putting an auth token into an input root or environment variable worth the resulting increased security risk versus having a fallback to still allow remote execution to work with a credentialed artifact repository?
I can imagine some people will argue for the fallback, but my own opinion is that it hides the security issue from an "ordinary" Pants user using remote execution. Then again, I assume people using remote execution are not the "ordinary" user and hopefully perceive the trade-off.
So not supporting remote execution and failing early highlights the inherent security issue.
But that's my take on a fundamental design choice for this feature.
a
Environment variable would certainly be easy to implement. You could make it an opt-in feature for remote execution.
f
So my choice had been to punt, and so of course I agree with your choice to potentially punt. :)
yeah opt-in would at least force the user to tacitly confront the security issue even if it is them just shrugging it off
too bad there isn't a spec for a REAPI secrets service
and also too bad, REAPI doesn't really spec out named caches or persistent workers
a
My employer does now pay for premium support from Google šŸ¤”šŸ˜…
f
so there is some friction between how Pants perceives executing processes remotely and how the REAPI models it.
some sort of opt-in to a fallback seems reasonable.
g
Does marking something as uncacheable propagate over reapi? It's what I do in my plugin when I decrypt a secret. Haven't thought much about reapi in that regard
(I'd be equally worried about cache busting with SLTs as the security, too.)
f
> Does marking something as uncacheable propagate over reapi? As it relates to storing a secret beyond the immediate need? Not really for a few reasons: ā€¢ The 
skip_cache_lookup
member of 
ExecuteRequest
applies to whether the remote execution system short circuits the execution by returning an already cached 
ActionResult
. See the spec. ā€¢ The choice to use the cache is on the client (i.e., Pants) calling or not calling into the REAPI 
GetActionResult
API. If Pants asks for remote execution, then the remote execution system will store the 
ActionResult
in the Action Cache (subject to application of the 
skip_cache_lookup
bool). If Pants decides something is uncached, it just means Pants decided to not look in the remote cache, asking for remote execution will still result in the CAS having the inputs and the Action Cache will have the resulting 
ActionResult
. ā€¢ The secret will be stored in the 
Command
proto (for an environment variable) and uploaded to the CAS or in a file in the input root which itself put in the CAS. So it really depends on the server's GC algorithm as to when those would be purged.
There is a 
ResultsCachePolicy
with a "priority" value for how long to cache an 
ActionResult
but that is interpreted in a server-specific way.
off the top of my head, I wonder if the Remote Asset API could be coopted to support secrets resolution, but beats me. (The Remote Asset API introduces some indirection which might not invalidate the cache key if the indirect name is stable, but beats me without doing actual research into it.)
(or my idea could entirely miss the point of the Remote Asset API)
any way, just ideas and way beyond the smaller scope of a solution applicable to local and docker environments
a
I made some updates to my draft PR but I'm pretty sure this won't work as is for 
docker_environment
+ it feels very hacky doing filesystem ops this way https://github.com/pantsbuild/pants/pull/22370/files#diff-6486557a2dc478e17102593a7d055c9713ee262a3ab8ed6552399ec878206532R49-R64
8 Views
Open in Slack
PreviousNext
Powered by