Vault: Remove global usage of Vault and introduce a Vault Provider type

andygrunwald commented 5 months ago

Why this change?

The vault package was a single, globally used package. Several other packages had dependencies on the global variables of the vault package. Access (read/write) was hard to track.

We introduce a type of "Provider" object. This way, we can instantiate multiple Vaults of different Vault providers if needed.

What is this PR doing?

Introducing a scoped Provider (1Password) in the Vault package
Removed the dependencies of the Vault package (e.g. Viper)
Introduced a DetermineBinary function to "find" the binary magically -> Removed the default value "one_password_cli_command" = "/usr/local/bin/op"
Moved the vault types into vault/types.go
Added error checks wherever possible
Removing the dependency to bash, when querying 1password

What is still TODO?

Introducing proper and human-friendly error messages

brgmn commented 5 months ago

Nice work! Thanks! The changes seem to work and make sense! Only the error message when not connected to a vault is a bit cryptic now. Can we make this more user-friendly again? (e.g. just "Could not connect to 1Password vault. Open 1Password vault with "eval $(op signin)", first.").

andygrunwald commented 5 months ago

@brgmn Done in https://github.com/buchhalter-ai/buchhalter-ai-cli/pull/11/commits/5ce216b1dfe634f154ea8f50be21dc35baa13035

The idea with these "error codes" was that if we get error messages, we know where it fails and can also offer a FAQ if necessary. Just a thought, but not needed at the moment.

buchhalter-ai / buchhalter-ai-cli