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

Restore client-side support for working with ansible vaults #177

Merged
merged 54 commits into from
Oct 29, 2021

Conversation

jeinwag
Copy link
Contributor

@jeinwag jeinwag commented Sep 5, 2021

A quick & dirty "backport" of the original ansible vault code which got lost when PR #142 was merged. Fixes #171.

package.json Show resolved Hide resolved
Copy link
Member

@ganeshrn ganeshrn left a comment

Choose a reason for hiding this comment

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

@jeinwag Thank you for re-adding the vault feature and apologies that it was removed during the refactoring.

For achieving feature parity with the older plugin this PR looks good to me.
However, in long term, we can explore the possibility of supporting this functionality in the ansible-language-server https://github.com/ansible/ansible-language-server so that other clients can also use it.

@ssbarnea
Copy link
Member

ssbarnea commented Sep 9, 2021

@jeinwag one mistake that I observed is that you forgot to run npm install and include changes made to the package lock. Due to this the lockfile is missing the newly added deps.

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

I tried to encrypt a vars file and I observed an unnamed input box which I expected to be the password but entering something there did not had any effect. Few seconds later I got an error popup reporting: "Command 'Ansible Vault: Encrypt/Decrypt' resulted in an error (Running the contributed command: 'extension.ansible.vault' failed.)"

By looking at the Log (Extension Host) output I seen some weird messaging:

[2021-09-09 12:33:07.598] [exthost] [error] Error: Command failed: ansible-vault encrypt "/var/folders/3q/pc1jcyjj3qqbj6_x7n4kbtqw0000gn/T/tmp-43559-JYoNwmt0OU03" --encrypt-vault-id="default"
/Users/ssbarnea/.pyenv/versions/3.9.6/lib/python3.9/getpass.py:91: GetPassWarning: Can not control echo on the terminal.
  passwd = fallback_getpass(prompt, stream)
Warning: Password input may be echoed.
New Vault password: �[1;35m[WARNING]: Error in vault password prompt (default): EOFError (ctrl-d) on�[0m
�[1;35mprompt for (default)�[0m
�[0;31mERROR! EOFError (ctrl-d) on prompt for (default)�[0m

	at checkExecSyncError (child_process.js:625:11)
	at Object.execSync (child_process.js:661:15)
	at Object.<anonymous> (electron/js2c/asar_bundle.js:5:12288)
	at exec (/Users/ssbarnea/c/a/vscode-ansible/out/client/features/vault.js:194:15)
	at encryptFile (/Users/ssbarnea/c/a/vscode-ansible/out/client/features/vault.js:176:9)
	at encryptInline (/Users/ssbarnea/c/a/vscode-ansible/out/client/features/vault.js:144:5)
	at toggleEncrypt (/Users/ssbarnea/c/a/vscode-ansible/out/client/features/vault.js:95:48)
	at runMicrotasks (<anonymous>)
	at processTicksAndRejections (internal/process/task_queues.js:93:5)
	at async _executeContributedCommand (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:94:110871) extension.ansible.vault

I think that this encountered some interactive ansible output which is something that can easily go wrong. I think we should do our best to avoid any interactive prompts from ansible (run it in non interactive mode).

@jeinwag
Copy link
Contributor Author

jeinwag commented Sep 9, 2021

@ssbarnea it's not asking for the password, but for the vault identity to use to encrypt the file. I added a fix to display an error message if there are no identities defined.

@webknjaz webknjaz added the new label Sep 9, 2021
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
src/features/vault.ts Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
@webknjaz webknjaz requested a review from a team October 26, 2021 19:43
@jeinwag
Copy link
Contributor Author

jeinwag commented Oct 26, 2021

@webknjaz I'll take a look at your proposed changes tomorrow.

@webknjaz webknjaz changed the title fix: restore original ansible vault code Resurrect the client-side support for working with encrypted data via ansible-vault Oct 26, 2021
@webknjaz webknjaz added the feature New feature or request label Oct 26, 2021
@ssbarnea
Copy link
Member

ssbarnea commented Oct 27, 2021

@jeinwag Please rebase and try to run extension again, open the play1.yml playbook from examples folder and try to decrypt the secret or encrypt a new one.

Sadly that did not work for me and was the first feature I was expecting to get from the vaulting: ability to encrypt/decrypt.

Probably you seen that via https://github.com/ansible/vscode-ansible/pull/270/files I added test files specially for that. If you try to run ansible-playbook playbooks/play1.yml, from inside examples folder, you will see that ansible is able to access the vault secret. I expect for our extension to be able to behave just like ansible.

@ssbarnea ssbarnea removed the bug Something isn't working label Oct 27, 2021
@jeinwag
Copy link
Contributor Author

jeinwag commented Oct 27, 2021

@ssbarnea It does not work because there is no identity list configured. The ansible-vault functionality in the extension has, as far as I know, always required that you make use of vault IDs, see https://docs.ansible.com/ansible/latest/user_guide/vault.html#managing-multiple-passwords-with-vault-ids.

I will look into making it work without the usage of vault IDs.

@ssbarnea
Copy link
Member

@ssbarnea It does not work because there is no identity list configured. The ansible-vault functionality in the extension has, as far as I know, always required that you make use of vault IDs, see docs.ansible.com/ansible/latest/user_guide/vault.html#managing-multiple-passwords-with-vault-ids.

I will look into making it work without the usage of vault IDs.

I hope you can. The most basic way to use vaulting in Ansible is using the setup I added as example as it does not even require any input, being fully unattended.

The second issue was that when it failed for me to encrypt it gave a relatively cryptic error and I did not had any idea on how to make it work, especially as there is no place where user can configure which vault to use for current project. The only configurable settings related to vault is the ansible-vault executable, something that I hope nobody will have to customize.

Maybe you could give me some hints on how to test the way it is supposed to work now?, preferably in context of the newly introduced examples/ folder. We can add more stuff there just for testing purposes.

I mention this because I plan to add ui-tests for all features, so we prevent regressions and that folder would the place where these tests will happen.

src/features/vault.ts Outdated Show resolved Hide resolved
@jeinwag
Copy link
Contributor Author

jeinwag commented Oct 27, 2021

@ssbarnea I changed the code so the en-/decrypting now works when only vault_password_file is defined, no need for vault_identity_list.
I also added an example with multiple vault IDs, you need to open the subdirectory vault-ids-example to give it a try.

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

I tested it and it worked round-trip. The indentation was not perfect but that can be sorted in a follow-up. I think that now it ok to be merged.

@ssbarnea ssbarnea changed the title Resurrect the client-side support for working with encrypted data via ansible-vault Restore client-side support for working with ansible vaults Oct 29, 2021
@ssbarnea ssbarnea merged commit 54a37d0 into ansible:main Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restore support for ansible-vault
5 participants