(GH-3296) Try both token and cert based auth for puppetdb #3297
Conversation
| @@ -16,6 +16,15 @@ def initialize(config:, project: nil, load_defaults: false) | |||
| @logger = Bolt::Logger.logger(self) | |||
| end | |||
|
|
|||
| def post_puppetdb(url, body) | |||
There was a problem hiding this comment.
I think this means if you have an invalid token, it's always going to be making two requests every time and you'll get failures in the PDB log for each one. Also, if you don't have a token but you do have certs specified, this is going to have the same behavior (actually not sure if it's 401 if there is no X-Authentication header at all).
What if this chooses one or the other if only one is specified, and if both are specified, try the token first. If that fails, record that it failed and don't try using it again.
There was a problem hiding this comment.
I'm open to that. I'll look at reducing requests.
There was a problem hiding this comment.
Updated to persist token exclusion when it is determined to be problematic.
There was a problem hiding this comment.
I am unable to reproduce a 401 other than the case where an invalid token is provided.
Previously token was always included in header. With this commit, try POST requests with token in header. If that returns 401 AND cert/key is configured, retry with auth header excluded. !feature * **Try cert based auth, failback to cert based auth for puppetdb** ([puppetlabs#3296](puppetlabs#3296)) When both a token and cert are specified, try using the token in the x-auth header for puppetdb POSTs. If that responds with a 401, try again with the token excluded.
|
Looks like i've got some test cleanup to do. I'll figure that out in the morning. |
|
This approach is needlessly complex. Closing in favor of #3299 We may look at changing the way this is handed in puppetdb rbac. |
Previously token was always included in header. With this commit, try POST requests with token in header. If that returns 401 AND cert/key is configured, retry with auth header excluded.
Alternately we could just prefer cert/key, but that seems like it may be a bigger shift in behavior, especially if people are expecting token to track identity.