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

Supporting k/v v2 #23

Closed
petems opened this issue Dec 19, 2018 · 21 comments
Closed

Supporting k/v v2 #23

petems opened this issue Dec 19, 2018 · 21 comments

Comments

@petems
Copy link
Owner

petems commented Dec 19, 2018

It would be nice to be able to support the new k/v v2 backend, especially as it's now default for the main kv mount.

Blockers

Right now, the Vault gem does not have an easy way of using the new KV v2 backend

The following issues will need to be solved/merged before hiera_vault can support v2:

hashicorp/vault-ruby#195
hashicorp/vault-ruby#194
hashicorp/vault-ruby#196

@maxadamo
Copy link
Contributor

maxadamo commented Jan 14, 2019

@petems is it possible to upgrade the keystore on Vault from v1 to v2? Or do we need to copy the keys one by one 🤕

@isbear
Copy link

isbear commented Jan 15, 2019

Upgrade is possible, we did that. Read the documentation for v2 store, there's instructions for upgrading: https://www.vaultproject.io/docs/secrets/kv/kv-v2.html

Note: you have to modify your policies and clients to work with new paths.

@traviscosgrave
Copy link
Contributor

see PR #36 regarding one approach to supporting kv2 (among a bunch of other things), still needs a modification to hashicorp/vault-ruby (see PR 201)

@arcenik
Copy link
Contributor

arcenik commented Aug 29, 2019

See also PR #41 which propose another approach to be v1 and v2 compatible.

@jhejl
Copy link

jhejl commented Nov 12, 2019

See also PR #41 which propose another approach to be v1 and v2 compatible.

@arcenik can you please check the #41 PR, please? It feels like the data part should be constructed differently, resp. it should be put right behind the mount part and then the rest of the path. Current version of the code doesn't work for me (even though it was merged into master already). Simple hack (sorry, not a rubyist) made it work for me:

--- /opt/puppetlabs/puppet/cache/lib/puppet/functions/hiera_vault.rb    2019-11-12 12:48:18.320664160 +0000
+++ /tmp/puppet-file20191112-396-1jzwdd7        2019-11-12 13:56:24.046216794 +0000
@@ -125,15 +125,15 @@
     kv_mounts.each_pair do |mount, paths|
       paths.each do |path|

-        secretpath = context.interpolate(File.join(mount, path))
-
-        context.explain { "[hiera-vault] Looking in path #{secretpath} for #{key}" }
-
         begin

+          secretpath = context.interpolate(File.join(mount, path))
+          context.explain { "[hiera-vault] Looking in path #{secretpath} for #{key}" }
           secret = get_kv_v1(secretpath, key)
           if secret.nil?
-            secret = get_kv_v2(secretpath, key)
+            secretpath_v2 = context.interpolate(File.join(mount, 'data', path))
+            context.explain { "[hiera-vault] Looking in path #{secretpath_v2} for #{key} via KV version 2" }
+            secret = get_kv_v2(secretpath_v2, key)
           end

         rescue Vault::HTTPConnectionError
@@ -189,7 +189,7 @@
   end

   def get_kv_v2(secretpath, key)
-    res = $vault.logical.read(File.join(secretpath,'data',key))
+    res = $vault.logical.read(File.join(secretpath,key))
     if ! res.nil?
       res=res.data[:data]
     end

@arcenik
Copy link
Contributor

arcenik commented Nov 13, 2019

Hello @jhejl

I think that you should make you own PR or create a new issue instead of putting file diff in an issue concerning an already merge PR.

Although, I don't see the point of moving the 'data' part of the path outside of get_kv_v2 function.
As it does not appear in the web UI of Vault, there no point to display it on the context.explain.

Best regards.

@maxadamo
Copy link
Contributor

wonder if #41 is usable and if we can create a tag.

@petems
Copy link
Owner Author

petems commented Nov 20, 2019

I just tested the change in #41 and it looks like it didnt work (I tried in my test environment at the time and it had but I think it was misconfifured)

I might be able to give this a go over the weekend and re-work it more 😄

@slauger
Copy link

slauger commented Nov 20, 2019

I can confirm that the changes from #41 do not work with KV2.

I created to mounts - hiera_backend (as kv1) and hiera_backend (as kv2) with the same parameters inside.

---
version: 5

defaults:
  datadir: data
  data_hash: yaml_data

hierarchy:
  - name: 'Vault kv2 backend'
    lookup_key: hiera_vault
    options:
      confine_to_keys:
        - '^vault_.*'
        - '^.*_password$'
        - '^password.*'
        - 'webserver::php_fpm_enable'
      address: 'https://dev01.example.tld:8200/'
      token: /etc/puppetlabs/puppet/vault-token
      default_field: value
      mounts:
        hiera_backend2:
          - '%{trusted.certname}'
          - 'common'
  - name: 'Vault kv backend'
    lookup_key: hiera_vault
    options:
      confine_to_keys:
        - '^vault_.*'
        - '^.*_password$'
        - '^password.*'
        - 'webserver::php_fpm_enable'
      address: 'https://dev01.example.tld:8200/'
      token: /etc/puppetlabs/puppet/vault-token
      default_field: value
      mounts:
        hiera_backend:
          - '%{trusted.certname}'
          - 'common'

Current result:

-bash# puppet lookup webserver::php_fpm_enable --explain --compile --node=web02.example.tld
Searching for "webserver::php_fpm_enable"
  Global Data Provider (hiera configuration version 5)
    No such key: "webserver::php_fpm_enable"
  Environment Data Provider (hiera configuration version 5)
    Using configuration "/etc/puppetlabs/code/environments/production/hiera.yaml"
    Hierarchy entry "Vault kv2 backend"
      No such key: "webserver::php_fpm_enable"
      [hiera-vault] Client configured to connect to https://dev01.example.tld:8200/
      [hiera-vault] Looking in path hiera_backend2/web02.example.tld for webserver::php_fpm_enable
      [hiera-vault] Looking in path hiera_backend2/common for webserver::php_fpm_enable
    Hierarchy entry "Vault kv backend"
      Found key: "webserver::php_fpm_enable" value: false

Would be great if someone could provide a working solution for KV2.

@arcenik
Copy link
Contributor

arcenik commented Nov 20, 2019

Can you also provides corresponding audit log from vault ?

@slauger
Copy link

slauger commented Nov 20, 2019

Can you also provides corresponding audit log from vault ?

Yep, vault responses with an error 404. Seems like something is wrong in the api call.

{
  "time": "2019-11-20T23:47:54.805571348Z",
  "type": "response",
  "auth": {
    "client_token": "hmac-sha256:fab71a05a86763d6c3988d32a3ed91cf9adc114b831eecc64902e435f586cefa",
    "accessor": "hmac-sha256:99664f96d2c3f373ae5e8e541f599b0d88ca61108d1a223072e8bfe317c6a120",
    "display_name": "root",
    "policies": [
      "root"
    ],
    "token_policies": [
      "root"
    ],
    "metadata": null,
    "entity_id": "",
    "token_type": "service"
  },
  "request": {
    "id": "ee9f4b23-c593-410f-a2fd-c59aa818e6ee",
    "operation": "read",
    "client_token": "hmac-sha256:fab71a05a86763d6c3988d32a3ed91cf9adc114b831eecc64902e435f586cefa",
    "client_token_accessor": "hmac-sha256:99664f96d2c3f373ae5e8e541f599b0d88ca61108d1a223072e8bfe317c6a120",
    "namespace": {
      "id": "root",
      "path": ""
    },
    "path": "hiera_backend2/common/data/webserver::php_fpm_enable",
    "data": null,
    "policy_override": false,
    "remote_address": "127.0.0.1",
    "wrap_ttl": 0,
    "headers": {}
  },
  "response": {
    "data": {
      "http_content_type": "hmac-sha256:ed1d318d9cbb5093fabe96bc06c826d045201a8f32913aee1c722023f3b23f47",
      "http_raw_body": "hmac-sha256:c7ef757aeaf9e7d3dc1ee2e644ff470d3e7d0afd46efe8bc380d9b77ba4aa098",
      "http_status_code": 404
    },
    "headers": null
  },
  "error": ""
}

@maxadamo
Copy link
Contributor

maxadamo commented Nov 21, 2019

This is wrong:

$vault.logical.read(File.join(secretpath,'data',key))

but it's fairly easy to fix. I write down a possible solution, as a reminder (and I can try a PR myself, tomorrow):

key_path = File.dirname(key)
key_name = File.basename(key)
res = $vault.logical.read(File.join(secretpath,'data,key_path)).data[:data][:"#{key_name}"]

@maxadamo
Copy link
Contributor

maxadamo commented Nov 22, 2019

First of all, my previous message was slightly wrong, however, I managed the create #43 (on top of #41) which works for me.

  • One issue was a trailing slash, which used to work with v1, but it doesn't with v2. I have added a chomp, in case someone re-uses an existing configuration.
  • the second part, consists of handling the hash in a different way compared to v1. The value is no longer a key called value, but a key whose name is the name of the initial key.

Please test it, improve it.

@maxadamo
Copy link
Contributor

maxadamo commented Nov 23, 2019

For your reference: Asciinema preview: Vault secrets shuffler
this is how I am using.
IMO it's only matter of agreeing if the key name should be value of the name of the key. In this case I'm using the name of the key.
p.s.: despite the name of the mount, this is v2, and not v1 😄

@arcenik
Copy link
Contributor

arcenik commented Nov 29, 2019

I'm using the default_field set to value and your solution does not work with my setup as you do not use the same mapping between hiera and vault.

My mapping:

hiera vault
mount + path secret engine name
lookup name secret name
default_field = "value" key

Your mapping:

hiera vault
mount secret engine name
path secret name
lookup name key

Depending of the mapping, you need to place the 'data' item (for v2) need to be placed on a different place.

@maxadamo
Copy link
Contributor

maxadamo commented Nov 29, 2019

I'll try to look at this ... now I'm commuting: Cambridge (UK), to Netherlands :) 

Because we really need to get this working :) 

The fact that "it crashes", I believe is fine and it does it with every backend, if the default value is not supplied. 

@petems
Copy link
Owner Author

petems commented Dec 15, 2019

Ok, I've merged #43 and in my tests, the code now works for v1 and v2 backends.

I've got one last PR with added tests #44, as the other code didn't take into account people not using the default_field option, where if you want to use data not being value it didn't work.

@bastelfreak
Copy link
Contributor

ping :)
@petems thanks for the awesome module! Can you tell me about the current state for v2 support? It seems to be somehow almost kind of readyish?

@petems
Copy link
Owner Author

petems commented Apr 14, 2020

Oh it's been ready and merged for ages now, I forgot to close this issue 😄

Closed by #52, #43, #44 👍

@petems petems closed this as completed Apr 14, 2020
@findmyname666
Copy link

@petems can you release latest version to forge please ?

@petems
Copy link
Owner Author

petems commented May 15, 2020

Hmm, I thought my pipeline should've done that already, let me check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants