search

puppetlabs / bolt

Bolt is an open source orchestration tool that automates the manual work it takes to maintain your infrastructure on an as-needed basis or as part of a greater orchestration workflow. It can be installed on your local workstation and connects directly to remote nodes with SSH or WinRM, so you are not required to install any agent software.
https://puppet.com/docs/bolt/latest/bolt.html
Apache License 2.0
501 stars 225 forks source link

Puppetdb Client *ALWAYS* uses token header even when cert/key is configured #3296

Open donoghuc opened 7 months ago

donoghuc commented 7 months ago

Describe the Bug

In the case a token AND cert based auth methods are configured for puppetdb, the client always includes the token in the header for puppetdb requests. This results in the case where puppetdb queries fail even when a valid cert/key pair is configured.

Expected Behavior

Puppetdb client should be able to use either token or cert/key auth.

Options:

  1. Client could have cert/key take precedence over token
  2. Client could try with token, on 401/403 response could retry with cert/key
nmburgan commented 7 months ago

I think what makes sense to me without introducing a new flag or something is:

  1. If either token OR cert is specified, use it.
  2. If both are specified, try the token first. If that fails, record that it failed and don't try using it again, and use the cert/key. If the cert/key is invalid, let that then fail, perhaps with a message saying both methods failed.
donoghuc commented 7 months ago

I think what is a bit confounding is that if cert/key are invalid it will fail regardless of the token. If this were net-new I would just choose to prefer cert based auth when both are specified (IE only include token if cert/key are not specified). But in order to not break anybody relying on token being included in header even if certs are configured I wanted to not disturb that (specifically thinking of the case where you would prefer the identity piece of an rbac token if possible). I bring this up in response to 2, specifically i think that the configuration will get a bit out of control with complexity if we try to use token only even if cert/key are incorrectly configured. We are not currently at risk of breaking anybody with a change today in this regard (if you have bad certs configured, regardless of your token config its not going to connect).

nmburgan commented 7 months ago

I see, didn't realize there was a use case for both. In that case, I think at least just not including the token again if the token was invalid to avoid repeat failures seems reasonable.

donoghuc commented 7 months ago

I'm tempted to simplify this to follow the pattern in the puppetdb CLI. Specifically, if cert is configured, do not use token. I think that really simplifies things and is easier to understand. It also probably helps as a forcing function to ensure there is not ambiguous config files in practice. I can put up a separate PR with that approach and make sure to add a log message to warn that token wont be used when cert based auth is configured.