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

Add methods in Ansible Vault utilities to find the ansible-vault executable #946

Closed
sleberknight opened this issue Apr 17, 2023 · 2 comments · Fixed by #964
Closed

Add methods in Ansible Vault utilities to find the ansible-vault executable #946

sleberknight opened this issue Apr 17, 2023 · 2 comments · Fixed by #964
Assignees
Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Milestone

Comments

@sleberknight
Copy link
Member

The VaultEncryptionHelperIntegrationTest contains code that finds the location of the ansible-vault executable using which. If it does not find it, then it falls back to "well known" locations for macOS (assuming Homebrew installation) and Linux.

In our new environments, ansible-vault is installed via pip, so the location is ~/.pip/bin/ansible-vault. Our existing service code (and specifically the "core service" library) assumes the standard Linux location of /usr/bin/ansible-vault, so the code obviously fails.

Instead, it would be better to use the strategy used in the test, i.e. first try finding it via which and then maybe falling back based on OS like the test does. This could be a static method in VaultEncryptionHelper or maybe in VaultConfiguration. For example,

// The value returned by this may or may not exist, i.e. if "which" returns nothing, then
// the fallback value might not actually exist on the filesystem. Maybe don't have this
// at all, since it would force callers to check for existence.
public static String whichAnsibleVaultOrFallback() { ... }

// This could use "which", and if that fails, then attempt several fallback locations based on OS
public static Optional<String> whichAnsibleVault() { ... }

These should be implemented using the which method which will be added to the kiwi process-related utilities in #945.

Other considerations include:

  • Should these go in VaultConfiguration, VaultEncryptionHelper, or somewhere else?
  • Should they be static methods only, or also provide instance methods analogous to ProcessHelper?
  • How should the fallback values be determined? VaultEncryptionHelperIntegrationTest assumes Homebrew-based installation on macOS (and the location is pre-Silicon so /usr/local instead of /opt/homebrew), and assumes /usr/bin on Linux.
  • If we have a list of fallbacks, maybe each should be checked to see if they exist, and only return a value that actually exists
  • An alternative fallback could include having the caller supply their own location as a String or as a Supplier<String>, or both

This originated from discussion #944

@sleberknight sleberknight added the new feature A new feature such as a new class, method, package, group of classes, etc. label Apr 17, 2023
@sleberknight sleberknight self-assigned this Apr 25, 2023
@sleberknight sleberknight added this to the 2.6.0 milestone Apr 25, 2023
@sleberknight
Copy link
Member Author

Here are some "well-known" locations that we can try as fallbacks. These are not official locations but are just a few locations I've found using various installation methods and operating systems. And of course, package managers often let you specify a different installation location, etc.

OS Installation method Location
macOS (Intel) Homebrew /usr/local/bin/ansible-vault
macOS (Applie Silicon) Homebrew /opt/homebrew (TODO: get rest of path when on a mac)
Linux Homebrew /home/linuxbrew/.linuxbrew/bin/ansible-vault
Linux yum /bin/ansible-vault
Linux pip ~/.pip/bin/ansible-vault

Since there is so much variation, I am thinking maybe we should just not even try to have "standard" fallback locations at all. Though, I am OK with providing an option to allow callers to specify a fallback location and/or a supplier of fallback location.

@sleberknight
Copy link
Member Author

Actually, if we return Optional there is little value in adding overloads that accept a fallback, since Optional already lets you do that easily using orElse, orElseGet, etc.

sleberknight added a commit that referenced this issue Apr 25, 2023
* Add static method whichAnsibleVault() - returns path as a String
* Add static method whichAnsibleVaultAsPath() - returns path as a Path
* The tests only test whether the return values equal the value returned
  by the Processes#which method, since we cannot (easily) mock static
  methods and can't assume ansible-vault will be installed

Closes #946
dsingley pushed a commit that referenced this issue Apr 25, 2023
* Add static method whichAnsibleVault() - returns path as a String
* Add static method whichAnsibleVaultAsPath() - returns path as a Path
* The tests only test whether the return values equal the value returned
  by the Processes#which method, since we cannot (easily) mock static
  methods and can't assume ansible-vault will be installed

Closes #946
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant