hashivault_kv auth_path moved from metadata to inputs #7991
Conversation
The auth_path is used with the approle auth method It's not linked to the secret we are reading but to the auth method, this parameter has to be moved to inputs
|
Build failed.
|
|
Build failed.
|
|
Build succeeded.
|
| 'label': _('Path to Auth'), | ||
| 'type': 'string', | ||
| 'multiline': False, | ||
| 'help_text': _('The path where the Authentication method is mounted e.g, approle') |
There was a problem hiding this comment.
I think more than one auth path can be available to a given AppRole role_id and AppRole secret_id, which may be the reason why the auth path was originally set as a metadata variable instead of an input.
However, I do see the rationale for grouping the auth path with the inputs instead because it is probably more convenient to just make a separate hashi credential for each auth_login path than it would be to provide the same login paths every time you link the hashi credential to an input field.
Based on my understanding of https://learn.hashicorp.com/tutorials/vault/approle#policy-requirements, I think this pr is probably the way to go. An app may have multiple approle auth paths it can use, but it doesn't seem like you'd typically use more than one at a time for most situations.
That said, I may be overlooking a use case that depends on the current configuration.
cc @kawsark @ryanpetrello Thoughts?
There was a problem hiding this comment.
Also worth mentioning that changing this is likely going to be a breaking change and potentially require a messy data migration for people who've already set these up.
There was a problem hiding this comment.
Agreed with Ryan. Also often larger customers have 100s of AppRole Auth methods setup (typically one per application) and it might be cumbersome to create a new hashi credential for each auth_login path. Usually the auth login path can be determined by the CI/CD Pipeline based on variables.
There was a problem hiding this comment.
Also often larger customers have 100s of AppRole Auth methods setup (typically one per application) and it might be cumbersome to create a new hashi credential for each auth_login path
This is great info, thank you @kawsark. This is a compelling reason to leave the current variable as-is.
Usually the auth login path can be determined by the CI/CD Pipeline based on variables.
I think this implies linking input fields to external credentials dynamically in a ci job run? (cool!)
Do you feel that there is also a (separate) use case for keeping an auth path scoped to the whole hashi credential? Perhaps we can accommodate both use cases with a default approle auth path that lives in the inputs. The default will be overridden by any metadata auth paths that are provided.
Pros:
- additive, so non-breaking & no messy data migrations
- leaves currently addressed use cases intact
- solves this problem:
It's not linked to the secret we are reading #7991
Cons:
- Yet another field, complexity, etc.
There was a problem hiding this comment.
Thanks for your reviews
The way approle works is:
- you create an
approleauth method at a specific path in Vault. Using this path you will be able to interact with this approle
# `zuul` is the path at wchich the approle auth method will be created
$> vault auth enable -path=zuul approle
Success! Enabled approle auth method at: zuul/
- for each approle auth method, you can create several roles, each role have specific rights/associated policies
# here we create a role called `repo1` with the policy `default` associated
$> vault write auth/zuul/role/repo1 token_policies=default
Success! Data written to: auth/zuul/role/repo1
- for each role you have one
role_idand you can create severalsecret_id
# we read the role_id of this role
$> vault read auth/zuul/role/repo1/role-id
Key Value
--- -----
role_id 620272b1-00db-b06d-8c25-7b41c6d77da7
# we generate a secret_id for this role
$> vault write -f auth/zuul/role/repo1/secret-id
Key Value
--- -----
secret_id 650c5fbb-8f77-e556-17bf-d37f3001a5f4
secret_id_accessor 75aa1796-c08a-d5bf-824a-4a78d887de0e
- you use the previously generated
role_idandsecret_idto login using the path of the approle auth method
The tuplerole_idandsecret_iddepend on only one approle (you don't need to specify the role name during login as arole_idmatch only one role)
$> vault write auth/zuul/login role_id=620272b1-00db-b06d-8c25-7b41c6d77da7 secret_id=650c5fbb-8f77-e556-17bf-d37f3001a5f4
Key Value
--- -----
token s.hTMy2GLh9WUyTnoZLZxwhrqe
token_accessor pA0pC3o3Qurs8atlH8PMGl8w
token_duration 768h
token_renewable true
token_policies ["default"]
identity_policies []
policies ["default"]
token_meta_role_name repo1
As you can see a tuple role_id and secret_id match only one approle auth method, only one path
I don't think it's possible to use an approle to auth against several auth methods as the role_id is generated by Vault for each role, that's why I made this change
An app may have multiple approle auth paths it can use
That's right but the app will need a role_id and a secret_id for each approle auth path (and also for different roles at the same approle auth path)
Also worth mentioning that changing this is likely going to be a breaking change and potentially require a messy data migration for people who've already set these up.
That's completely right but having the approle auth path elsewhere than in the credential is only data duplication
@kawsark
I can't see the use case you're talking about as, from my point of view, there is already different credentials for each auth_login path
To add more context, we are using Vault since several years now and we're using several approles/roles
But we started the AWX setup recently so maybe I'm missing something on a pure AWX point of view ? (on the way credentials are stored/used ?)
There was a problem hiding this comment.
This is a great conversation and there is some excellent information here. Thanks.
problem: 1:
100s of AppRole Auth methods setup (typically one per application) and it might be cumbersome to create a new hashi credential for each auth_login path. #7991 (comment)
problem: 2:
from my point of view, there is already different credentials for each auth_login path #7991 (comment)
problem 3:
Also worth mentioning that changing this is likely going to be a breaking change and potentially require a messy data migration for people who've already set these up. #7991 (comment)
Adding a default auth_path to the input fields and overriding it w/ the metadata path if both are provided will address all of these problems and concerns. We can explain the override behavior with appropriate variable naming and informative help text on the fields. Thoughts? Other ideas?
There was a problem hiding this comment.
Adding a default auth_path to the input fields and overriding it w/ the metadata path if both are provided will address all of these problems and concerns.
This looks like the right way to do it 👍
I'll work on it then submit a new commit for review
Thanks
There was a problem hiding this comment.
Sounds good - thank you.
This reverts commit 7c8e5ac.
There was a problem hiding this comment.
(See discussion and next steps in the review comments: https://github.com/ansible/awx/pull/7991/files#r478259375)
Current plan is to add a default auth path to the input fields. I'll take another look when that is ready, thank you!
|
Build failed.
|
|
Build failed.
|
|
Build failed.
|
|
recheck |
|
Build failed.
|
|
recheck |
|
Build succeeded.
|
There was a problem hiding this comment.
@bbayszczak The changes LGTM.
Please rebase this PR branch with the latest changes from devel. Once that is done we can merge this. Thank you!
Edit: Nevermind the rebase - this can be merged as-is. 👍
|
Build succeeded (gate pipeline).
|
SUMMARY
The auth_path is used with the approle auth method
As specified here: https://github.com/ansible/awx-custom-credential-plugin-example/blob/master/awx_custom_credential_plugin_example/__init__.py
inputs.fields represents fields the user will specify *when they create* a credential of this typeinputs.metadata represents values the user will specify *every time* they link two credentials together*It's not linked to the secret we are reading but to the auth method so this parameter has to be moved to inputs.
ISSUE TYPE
COMPONENT NAME
AWX VERSION
awx: 14.0.0
ADDITIONAL INFORMATION