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

feat: Add support for HCP Vault Secrets #3067

Merged
merged 1 commit into from Jul 14, 2023
Merged

feat: Add support for HCP Vault Secrets #3067

merged 1 commit into from Jul 14, 2023

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented Jun 26, 2023

Fixes #3061.

WIP. Not ready for review or merge yet.

@twpayne twpayne added work in progress Work in progress do not merge Do not merge labels Jun 26, 2023
@twpayne twpayne changed the title feat: Add support for HashiCorp Vault Secrets feat: Add support for HCP Vault Secrets Jun 28, 2023
@twpayne twpayne force-pushed the fix-3061 branch 3 times, most recently from 3366124 to 2fd6696 Compare June 28, 2023 00:26
@halostatue
Copy link
Collaborator

I’m playing with vlt myself and I’m not sure that --format json provides anything useful at this point; the metadata is limited and there doesn’t appear to be a way to get the secret value in the returned JSON:

$ vlt secrets get --format json test_secret | jq . | pbcopy
{
  "created_at": "2023-06-28T01:55:18.686Z",
  "created_by": {
    "email": "[REDACTED]",
    "name": "Austin Ziegler",
    "type": "TYPE_USER"
  },
  "latest_version": "2",
  "name": "test_secret",
  "version": {
    "created_at": "2023-06-28T01:56:11.053Z",
    "created_by": {
      "email": "[REDACTED]",
      "name": "Austin Ziegler",
      "type": "TYPE_USER"
    },
    "type": "kv",
    "version": "2"
  }
}

$ vlt secrets get --plaintext --format json test_secret | jq .
{
  "created_at": "2023-06-28T01:55:18.686Z",
  "created_by": {
    "email": "austin@zieglers.ca",
    "name": "Austin Ziegler",
    "type": "TYPE_USER"
  },
  "latest_version": "2",
  "name": "test_secret"
}

$ vlt secrets get --plaintext test_secret
real-test-value

vlt feels like a "cloud-enhanced" chezmoi secrets keyring. Still worth doing, but unless there’s a good use case or unless they add the ability to put the secret and application information into the metadata, I’m not sure that implementing the …Json template command is worthwhile.

@halostatue
Copy link
Collaborator

halostatue commented Jun 28, 2023

Another thought: vlt config init --local is not the default behaviour, but it feels generally safe enough that we could check that into the working tree or in the working tree directory specified by .chezmoiroot.

Unfortunately, it looks like chezmoi never changes $PWD, and that would require explicit specification or a single global configuration.

$ go run . execute-template '{{ hcpVaultSecrets "test_secret" }}'
chezmoi: template: arg1:1:3: executing "arg1" at <hcpVaultSecrets "test_secret">: error calling hcpVaultSecrets: /opt/homebrew/bin/vlt secrets get --plaintext test_secret: exit status 1
Error: organization ID must be set via `vlt config init` or passed in

exit status 1
$ go run . execute-template '{{ output "pwd" }}'
/Users/austin/dev/oss/forks/chezmoi

(I currently have .vlt.json in .chezmoiroot, but I think that the better location is just the chezmoi worktree root. Others might feel differently if they have two effective roots like in felipecrs/dotfiles.

@halostatue
Copy link
Collaborator

I’ve provided feedback with feature requests linking back to this draft PR:

I’m a contributor on twpayne/chezmoi and we’ve had someone ask to implement support for vlt / Vault Secrets, which the primary maintainer has started in #3067.

  1. vlt secrets get --plaintext key does exactly what is required to support the minimum use case (putting some sort of secret value in place). This would be done in chezmoi with a template like {{ hcpVaultSecrets "key" "app-name" "project" "organization" }}`.
  1. vlt secrets get --format json key doesn’t provide much useful information and there appears to be no way to get the JSON shape of the key and the secret value included in the JSON (I can understand where --format json might not be desired, but perhaps --format json+value). In the event that vlt config init or vlt config init --local has been run, it would be useful to include information like the app name, project name/id, and organization name/id. This would be a template command like hcpVaultSecretsJson, but would be most useful if the password were available in the JSON.
  1. There appears to be no way to pass a path to a configuration file.

(This last would make it so that chezmoi could check for $CHEZMOIROOT/.vlt.json and pass --config-path $CHEZMOIROOT/.vlt.json instead of needing to change $PWD.)

@kartiklunkad26
Copy link

@halostatue thank you for providing your feedback! Would you be up for a conversation to make sure we understand the improvements request & correspondingly do the work to fix it?

My calendar in case you'd be up for it.

@twpayne twpayne force-pushed the fix-3061 branch 4 times, most recently from 2f4ea2b to 67785e3 Compare June 28, 2023 23:37
@twpayne
Copy link
Owner Author

twpayne commented Jun 28, 2023

Thanks very much for testing @halostatue!

I've updated the PR with a few changes based on your work and feedback:

  • Added hcpVaultSecrets.organization, hcpVaultSecrets.project, and hcpVaultSecrets.appName config variables that are used as defaults if you don't specify an organization, project, or application name. Effectively, this allows you to provide a default --organization argument.
  • Ensured that the working directory when running vlt is run is the destination directory (typically the user's home directory) so vlt can find .vlt.json. Not sure if the working directory should always be the user's home directory instead.
  • Template functions renamed to hcpVaultSecret and hcpVaultSecretJson (singular, no s) to better reflect that fact that they return data from a single secret.
  • Added hcpVaultSecrets.args to pass extra optional arguments to vlt in all cases.

@kartiklunkad26 I've booked 30 minutes with you next Friday.

@twpayne twpayne force-pushed the fix-3061 branch 2 times, most recently from d0a99bc to 74229ce Compare June 29, 2023 00:03
@twpayne
Copy link
Owner Author

twpayne commented Jul 5, 2023

@arrrgi do you have any input on this?

@arrrgi
Copy link

arrrgi commented Jul 5, 2023

Thanks for the invite to review progress @twpayne

If I'm to understand the WIP so far:

  • the new feature requires the vlt CLI to be available on the target
  • a Chezmoi user can provide a default configuration to be used if none are provided as overrides
  • a template function allows a specific application/project/organisation to act as the root to fetch a name secret from

What I wasn't able to follow was the link to the .vlt.json file as I'm unsure where that fits into the usage pattern. Is that a file that needs to be checked into the local repo with Chezmoi to ensure portability?

I think there is also an undocumented assumption that before this works, you need to have installed the vlt CLI and exec vlt login which builds the oAuth session for subsequent requests with the client and this typically happens interactively with a browser. This might need a callout in the docs.

In a headless environment, will Chezmoi support the convention of respecting variables defined before the chezmoi command, eg. HCP_CLIENT_ID=xyz HCP_CLIENT_SECRET=foo chezmoi apply ?

Other than my couple of knowledge gap questions, it's a great implementation @twpayne and @halostatue !! Love it!

@arrrgi
Copy link

arrrgi commented Jul 5, 2023

@kartiklunkad26 thanks also for supporting the Chezmoi community's feedback and requests. Sorry I didn't reach out to you directly as I consider myself a hobbyist consumer, not your usual mid-market/Ent. customer :)

@twpayne
Copy link
Owner Author

twpayne commented Jul 5, 2023

Thanks for taking a look!

A couple of answers:

What I wasn't able to follow was the link to the .vlt.json file as I'm unsure where that fits into the usage pattern. Is that a file that needs to be checked into the local repo with Chezmoi to ensure portability?

I don't think you need a .vlt.json file. I think (but have not verified) that you can instead set the hcpVaultSecrets.organization, hcpVaultSecrets.project, and hcpVaultSecrets.appName variables in your chezmoi configuration file.

I think there is also an undocumented assumption that before this works, you need to have installed the vlt CLI and exec vlt login which builds the oAuth session for subsequent requests with the client and this typically happens interactively with a browser. This might need a callout in the docs.

This is correct. You have to install the vlt CLI and run vlt login before running chezmoi. I'll update the docs.

In a headless environment, will Chezmoi support the convention of respecting variables defined before the chezmoi command, eg. HCP_CLIENT_ID=xyz HCP_CLIENT_SECRET=foo chezmoi apply ?

Yes, chezmoi passes its environment on to subcommands (like vlt) when it executes the subscommands.

@kartiklunkad26
Copy link

@arrrgi

Sorry I didn't reach out to you directly as I consider myself a hobbyist consumer, not your usual mid-market/Ent. customer :)

Our intent with Vault Secrets is to make it valuable for hobbyist consumers and SMB organizations, so your use-case is very much our focus! Nevertheless, let me know if you have other feedback points that you'd like to see improve in the product.

@twpayne twpayne force-pushed the fix-3061 branch 3 times, most recently from 98d0bb1 to 8bc8472 Compare July 8, 2023 00:02
@twpayne
Copy link
Owner Author

twpayne commented Jul 8, 2023

This is now ready for review.

Thank you @arrrgi for the initial input and thank you @halostatue for the thorough investigation.

@kartiklunkad26 if you would be able to review the documentation, that would be fantastic.

@twpayne twpayne marked this pull request as ready for review July 8, 2023 01:06
@twpayne
Copy link
Owner Author

twpayne commented Jul 12, 2023

@kartiklunkad26 I'd like to do a new release of chezmoi by the end of this week. If you could review the docs by Thursday evening your time, that would be awesome.

@twpayne twpayne merged commit c887641 into master Jul 14, 2023
20 checks passed
@twpayne twpayne deleted the fix-3061 branch July 14, 2023 15:02
@kartiklunkad26
Copy link

@twpayne Hey Tom. Sorry I could only get to this now. I'll review it today and leave any comments.

bjw-s pushed a commit to bjw-s/dotfiles that referenced this pull request Jul 17, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [chezmoi](https://togithub.com/twpayne/chezmoi) | minor | `2.34.3` ->
`2.35.0` |

---

### Release Notes

<details>
<summary>twpayne/chezmoi (chezmoi)</summary>

###
[`v2.35.0`](https://togithub.com/twpayne/chezmoi/releases/tag/v2.35.0)

[Compare
Source](https://togithub.com/twpayne/chezmoi/compare/v2.34.3...v2.35.0)

##### What's Changed

- feat: Add archive-file externals by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3080
- fix: Never consider localhost.localdomain in /etc/hosts as the FQDN by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3082
- chore: Miscellaneous fixes by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3091
- chore: Use golang/govulncheck-action by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3094
- feat: Add support for HCP Vault Secrets by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3067
- chore: Use Goreleaser's WinGet support by
[@&#8203;twpayne](https://togithub.com/twpayne) in
[twpayne/chezmoi#3059

**Full Changelog**:
twpayne/chezmoi@v2.34.3...v2.35.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44LjExIiwidXBkYXRlZEluVmVyIjoiMzYuOC4xMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: bjw-s-bot <87358111+bjw-s-bot[bot]@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Add support for Hashicorp Vault Secrets
4 participants