Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'New Vault password' on vault 'edit' #35923

Merged
merged 2 commits into from
Mar 27, 2018
Merged

Fix 'New Vault password' on vault 'edit' #35923

merged 2 commits into from
Mar 27, 2018

Conversation

alikins
Copy link
Contributor

@alikins alikins commented Feb 8, 2018

SUMMARY

ffe0dde introduce a
change on 'ansible-vault edit' that tried to check
for --encrypt-vault-id in that mode. But '--encrypt-vault-id'
is not intended for 'edit' since the 'edit' should always
reuse the vault secret that was used to decrypt the text.

Change cli to not check for --encrypt-vault-id on 'edit'.

VaultLib.decrypt_and_get_vault_id() was change to return
the vault secret used to decrypt (in addition to vault_id
and the plaintext).

VaultEditor.edit_file() will now use 'vault_secret_used'
as returned from decrypt_and_get_vault_id() so that
an edited file always gets reencrypted with the same
secret, regardless of any vault id configuration or
cli options.

Fixes #35834

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/cli/vault.py
lib/ansible/parsing/vault/init.py

ANSIBLE VERSION
ansible 2.5.0 (vault_edit_prompt_35834 ed8de141d8) last updated 2018/02/08 15:43:46 (GMT -400)
  config file = /home/adrian/src/ansible/ansible.cfg
  configured module search path = [u'/home/adrian/ansible/my-modules']
  ansible python module location = /home/adrian/src/ansible/lib/ansible
  executable location = /home/adrian/src/ansible/bin/ansible
  python version = 2.7.14 (default, Jan 17 2018, 14:28:32) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]


ADDITIONAL INFORMATION

@ansibot ansibot added bugfix_pull_request needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Feb 8, 2018
@ansibot
Copy link
Contributor

ansibot commented Feb 8, 2018

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/cli/vault.py:185:9: E303 too many blank lines (2)
lib/ansible/parsing/vault/__init__.py:780:5: E303 too many blank lines (2)

click here for bot help

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Feb 8, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Feb 8, 2018
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Feb 13, 2018
@@ -181,7 +181,7 @@ def run(self):
if not vault_secrets:
raise AnsibleOptionsError("A vault password is required to use Ansible's Vault")

if self.action in ['encrypt', 'encrypt_string', 'create', 'edit']:
if self.action in ['encrypt', 'encrypt_string', 'create']:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So following on from my comment in #35834 I thought you added the edit action in PR #31067 to solve #30491

I'm not saying either way is right or wrong, simply asking the question seeing as an issue of the same type introduced a fix, now that code is being removed. Just trying to provide context here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #31067 I thought I was going to try to let the encrypt-vault-id be chosen/overridden for 'edit' but have since decided not to allow that and to always use the decrypt vault-id as the encrypt vault-id for 'edit'.

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Feb 26, 2018
ffe0dde introduce a
change on 'ansible-vault edit' that tried to check
for --encrypt-vault-id in that mode. But '--encrypt-vault-id'
is not intended for 'edit' since the 'edit' should always
reuse the vault secret that was used to decrypt the text.

Change cli to not check for --encrypt-vault-id on 'edit'.

VaultLib.decrypt_and_get_vault_id() was change to return
the vault secret used to decrypt (in addition to vault_id
and the plaintext).

VaultEditor.edit_file() will now use 'vault_secret_used'
as returned from decrypt_and_get_vault_id() so that
an edited file always gets reencrypted with the same
secret, regardless of any vault id configuration or
cli options.

Fixes #35834
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 20, 2018
@ilyakamens
Copy link

Would love for this to be merged!

@alikins alikins merged commit 6e737c8 into ansible:devel Mar 27, 2018
ryancurrah pushed a commit to ryancurrah/ansible that referenced this pull request Apr 4, 2018
* Fix 'New Vault password' on vault 'edit'

ffe0dde introduce a
change on 'ansible-vault edit' that tried to check
for --encrypt-vault-id in that mode. But '--encrypt-vault-id'
is not intended for 'edit' since the 'edit' should always
reuse the vault secret that was used to decrypt the text.

Change cli to not check for --encrypt-vault-id on 'edit'.

VaultLib.decrypt_and_get_vault_id() was change to return
the vault secret used to decrypt (in addition to vault_id
and the plaintext).

VaultEditor.edit_file() will now use 'vault_secret_used'
as returned from decrypt_and_get_vault_id() so that
an edited file always gets reencrypted with the same
secret, regardless of any vault id configuration or
cli options.

Fixes ansible#35834
nitzmahone pushed a commit that referenced this pull request Apr 9, 2018
* Fix 'New Vault password' on vault 'edit'

ffe0dde introduce a
change on 'ansible-vault edit' that tried to check
for --encrypt-vault-id in that mode. But '--encrypt-vault-id'
is not intended for 'edit' since the 'edit' should always
reuse the vault secret that was used to decrypt the text.

Change cli to not check for --encrypt-vault-id on 'edit'.

VaultLib.decrypt_and_get_vault_id() was change to return
the vault secret used to decrypt (in addition to vault_id
and the plaintext).

VaultEditor.edit_file() will now use 'vault_secret_used'
as returned from decrypt_and_get_vault_id() so that
an edited file always gets reencrypted with the same
secret, regardless of any vault id configuration or
cli options.

Fixes #35834

(cherry picked from commit 6e737c8)
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ansible-vault edit asks to change the vault password (devel)
5 participants