Don't ask for password confirm on 'ansible-vault edit' #30514
Conversation
This is to match the 2.3 behavior on:
ansible-vault edit encrypted_file.yml
Previously, the above command would consider that a 'new password'
scenario and prompt accordingly, ie:
$ ansible-vault edit encrypted_file.yml
New Password:
Confirm New Password:
The bug was cause by 'create_new_password' being used for
'edit' action. This also causes the previous implicit 'auto prompt'
to get triggered and prompt the user.
Fix is to make auto prompt explicit in the calling code to handle
the 'edit' case where we want to auto prompt but we do not want
to request a password confirm.
Fixes #30491
ansibot
added
affects_2.5
If they would not trigger it before anyway (no vault ids, no ask_vault_pass, no created_new_password, defaults otherwise)
|
For non-existent files, this PR introduces this behaviour: which matches with the 2.3 behaviour. I think because it has precedent, that behaviour is okay. In the future, someone may request the 2.4.0 behaviour is restored, though, wherein ansible-vault edit new-file.yml creates the new file and then opens the editor on it. We probably should have a comment in the code that warns someone that goes to implement that, that they have to be careful not to unfix this case. Test case would probably also be a good idea. Could be some unittests that check whether the correct prompt function is called the correct number of times. |
2.4/devel didnt change that behavior, 'ansible-vault edit file-that-doesnt-exist.yml' has always[1] caused an error. The password confirm for 'ansible-vault edit existing-file.yml' was just kind of bogus since it confirms the password by successful decrypt as you mentioned. The extra confirm was not an intentional change and wasnt intended to make 'ansible-vault newfile.yml' work. [1] Or at least for a long time: |
|
We might want to just 'open a new file' w/o errors, just a warning, as a feature enhancement in the future. |
|
Just commented on the issue, pasting here since I hadn't seen this thread yet:
YES, but the same behavior occurs with A lot of this hinges upon whether we want or expect
That is what |
|
I like the new If the On 2.3, it returned a |
Doh! Somewhere in the middle of my testing I switched from testing ansible-vault edit to testing ansible-vault create. |
|
@nrwahl2 yeah, that is the default being used as the first secret. The code that checks for multiple secrets on encrypt actions only checks for vault ids before creating the secrets and the config based ones are added later in setup_vault_secrets. So vault cli .run() could check the number of secrets after setup_vault_secrets() instead of before and raise an error there. To choose which of the vault secrets to use may need something like the first parts of https://github.com/ansible/ansible/pull/27668/files vault.py changes. |
|
@alikins: I don't quite understand. In my test case encryption, I'm using |
|
@nrwahl2 --ask-vault-pass does not override what is in the config file, it adds to it. So the first secret is from vault_password_file and is used to encrypt the file. The view works because it also uses vault_password_file per config and for that file, that is the password it was encrypted with. |
|
Nice.... |
|
the default decrypt behavoior is to try all the known secrets in order. In your case, the first one is the vault_password_file from config. It succeeds because that is what the file was encrypted with. The number of secrets/vault ids comes into play on the encrypt, where there should be an error if there are more than one vault ids provided. But that fails because of a bug where the check is too early (before setup_vault_secrets when the configured secret is addded) so it uses the configured ansible_password_file secret to encrypt with because the config is a higher precendence (setup_vault_secrets prepends it to the list) |
|
ie: That last line should raise an error but currently does not. |
ansibot
added
the
needs_revision
|
@nrwahl2 devel...alikins:error_on_config_plug_ask_vault_pass is an example patch changing where the number of vault ids is checked would prevent the case you mentioned in #30514 (review) |
|
Thanks for submitting the patch :) IMO it would make more sense to make Sincere question: After this change, would it make sense to use I haven't dived too deeply into this code so forgive any misunderstandings. |
|
@abadger cherry-pick candidate commit 307be59 |
This is to match the 2.3 behavior on:
ansible-vault edit encrypted_file.yml
Previously, the above command would consider that a 'new password'
scenario and prompt accordingly, ie:
$ ansible-vault edit encrypted_file.yml
New Password:
Confirm New Password:
The bug was cause by 'create_new_password' being used for
'edit' action. This also causes the previous implicit 'auto prompt'
to get triggered and prompt the user.
Fix is to make auto prompt explicit in the calling code to handle
the 'edit' case where we want to auto prompt but we do not want
to request a password confirm.
Fixes #30491
(cherry picked from commit 307be59)
bcoca
removed
the
needs_triage
* Don't ask for password confirm on 'ansible-vault edit'
This is to match the 2.3 behavior on:
ansible-vault edit encrypted_file.yml
Previously, the above command would consider that a 'new password'
scenario and prompt accordingly, ie:
$ ansible-vault edit encrypted_file.yml
New Password:
Confirm New Password:
The bug was cause by 'create_new_password' being used for
'edit' action. This also causes the previous implicit 'auto prompt'
to get triggered and prompt the user.
Fix is to make auto prompt explicit in the calling code to handle
the 'edit' case where we want to auto prompt but we do not want
to request a password confirm.
Fixes ansible#30491
* Don't ask for password confirm on 'ansible-vault edit'
This is to match the 2.3 behavior on:
ansible-vault edit encrypted_file.yml
Previously, the above command would consider that a 'new password'
scenario and prompt accordingly, ie:
$ ansible-vault edit encrypted_file.yml
New Password:
Confirm New Password:
The bug was cause by 'create_new_password' being used for
'edit' action. This also causes the previous implicit 'auto prompt'
to get triggered and prompt the user.
Fix is to make auto prompt explicit in the calling code to handle
the 'edit' case where we want to auto prompt but we do not want
to request a password confirm.
Fixes ansible#30491
ansibot
added
bug
|
Hi, Not sure whether this is the right forum, but the issue stated in this forum is similar to what I am currently facing, hence request your help on the same. Tried to create the Vault password, below are the step performed vi target.yml ansible_user: ansible ansible_ssh_pass: ansible_become_pass: touch target.pass ansible-vault encrypt target.pass ansible-vault encrypt target.yml --vault-password-file=target.pass At this point : Error : [WARNING]: Error in vault password file loading (default): A vault password must be specified to decrypt data ERROR! A vault password must be specified to decrypt data Even tried the below command, but still no luck. ansible-vault encrypt /home/ansible/playbooks/target.yml --vault-password-file=/home/ansible/playbooks/target.pass From, |
This is to match the 2.3 behavior on:
Previously, the above command would consider that a 'new password'
scenario and prompt accordingly, ie:
The bug was cause by 'create_new_password' being used for
'edit' action. This also causes the previous implicit 'auto prompt'
to get triggered and prompt the user.
Fix is to make auto prompt explicit in the calling code to handle
the 'edit' case where we want to auto prompt but we do not want
to request a password confirm.
Fixes #30491
SUMMARY
ISSUE TYPE
COMPONENT NAME
lib/ansible/cli/
ANSIBLE VERSION
ADDITIONAL INFORMATION
output of test case in #30491 after fix