Problem with vault_json implementation for chef provisioner

[whiteley]

Terraform Version

0.7.4

Affected Resource(s)

n/a

Affected Provisioner(s)

chef

Expected Behavior

When using the vault_json argument, the behavior should mimic the --bootstrap-vault-file/--bootstrap-vault-json feature of knife bootstrap.

Actual Behavior

The provisioned node is added as an admin not a client of the vault item(s).

Provisioned node is visible in knife vault show VAULT ITEM -p admins and not knife vault show VAULT ITEM -p clients.

Privileged operations such as knife vault update VAULT ITEM -S "" -k /etc/chef/client.pem are possible on the provisioned node.

Steps to Reproduce

1. terraform apply

Important Factoids

Chef Vault has two roles of access; client (ro) and admin (rw). Adding the provisioned node as an admin allows it to both modify the vault item contents and manage it's access controls.

As far as I'm aware, there is not functionality to efficiently add a single node to client level access for a vault using knife vault from the command line. You have basically two options:

• create a client that satisfies the search_query for the item and run knife vault refresh
• use knife bootstrap with one of the above mentioned options

The first option has the downside that it can be very slow due to the core mechanisms in chef-vault for search and returning large node objects. In production environments, it's not uncommon for this operation to take multiple minutes if run on or near the chef server and even an hour or more if run from a developer workstation. There are long standing chef-vault issues surrounding this behavior.

The second is much faster since it only checks that the node being bootstrapped satisfies the search query and inserts it without refreshing the rest of the item's access. The current implementation in terraform for the chef provisioner doesn't use knife bootstrap so this would require significant changes.

Before this feature was available in terraform my workaround was:

1. use the chef provisioner with an empty run list
2. use a local-exec with custom code to mimic the insert of knife bootstrap
3. use a local-exec to update the run list
4. use a remote-exec to run chef on the node

This is all necessary since the search_query will generally be something that is more complex than just name:foo and so the node object must exist with the required tags, attributes, environment etc. which doesn't fit well in the knife client create ... chef-client workflow.

Due to that complexity I was quite excited to see this support in 0.7.4 but I'm afraid that it's not working as intended.

References

This feature was added in #8577
https://github.com/chef/chef-vault/blob/master/KNIFE_EXAMPLES.md
https://docs.chef.io/knife_bootstrap.html#options

terraform/builtin/provisioners/chef/resource_provisioner.go

Line 548 in 968472a

updateCmd := fmt.Sprintf("%s vault update %s %s -A %s -M client %s",

Hopefully this makes sense (and is correct), let me know if I can provide more information. I'd be happy to help work on the refactor after it's decided upon.

Thanks!

[svanharmelen]

@whiteley thanks for your extensive report. I was actually already aware of this, but is seems (IMO) that with the currently available options, this is the best we can do. Let me explain a little...

As you noted yourself, the knife vault command currently does not allow you to simply add a single client. It only allows you to do so by giving a single Chef search which would make it extremely hard (if even possible) to figure out the right thing to do when you want to give access on a node-by-node basis.

knife bootstrap does use a node-by-node approach, but does this by leveraging the fact both knife and chef-vault are written in Ruby. So it is including chef-vault and uses it's existing classes and methods to update the vault is a custom way (manually adding the single node directly to the vault, see here).

As far as I can see in the code, it does not actually verify if the new node satisfies the search query as you mentioned:

The second is much faster since it only checks that the node being bootstrapped satisfies the search query and inserts it without refreshing the rest of the item's access

But it just adds the node as you requested.

So if we could find a way to add a single client to a vault, we should be good and have the exact same behaviour as we do with knife bootstrap. The problem is that the only way I see this happening would be by implementing a new feature in knife vault.

But the more I think about it, the less I see that as a problem so I guess I will taking a look at that (when time permits). Still not an easy path and still the question if they would be willing to add/merge such a feature, but IMO the only real/correct solution.

[whiteley]

As you noted yourself, the knife vault command currently does not allow you to simply add a single client. It only allows you to do so by giving a single Chef search which would make it extremely hard (if even possible) to figure out the right thing to do when you want to give access on a node-by-node basis.

You could use node_name:foo OR node_name:bar OR ... if you wanted to do that but it's certainly not going to scale well. My intention was to say that it does not allow you to refresh a single client. Granting initial access to a single client disregarding the search_query is outside the intended use of the tool as I understand it.

knife bootstrap does use a node-by-node approach, but does this by leveraging the fact both knife and chef-vault are written in Ruby. So it is including chef-vault and uses it's existing classes and methods to update the vault is a custom way (manually adding the single node directly to the vault, see here).
As far as I can see in the code, it does not actually verify if the new node satisfies the search query as you mentioned:

Indeed, my understanding of that was incorrect. Again, I think this contradicts the documentation for chef-vault and I'll have to open an issue for clarification about that with them.

But the more I think about it, the less I see that as a problem so I guess I will taking a look at that (when time permits). Still not an easy path and still the question if they would be willing to add/merge such a feature, but IMO the only real/correct solution.

I think the difference in adding as admin vs. client is a large enough concern that it should be at the least conspicuously called out in the documentation.

• the functionality of --clean-unknown-clients no longer works
• further management of a chef vault item after deleting an admin can result in ChefVault::Exceptions::AdminNotFound while it's an expected situation for clients
• knife vault refresh cannot be used to update access for the node if its key has changed
• using this functionality to gain access on a reload of a node in an existing environment will likely wind up with the node in both clients and admins further confusing things
• management of admins for compliance is much more difficult

I think we'll have to wait to use it until we can adjust our current vault management practices and I think others might be surprised by the behavior as well. Thanks for the correction above and extending the provisioner, it's a great direction to head in!

[svanharmelen]

I clearly see your points and (for the most part) agree with what you saying 😉

So I spent about half an hour earlier this evening to check out how much work it would be to add support for this use casewithin chef-vault, and came to the conclusion it's really, really simple to add (I actually already tested with a working solution).

So I was thinking about making a quick WIP PR tomorrow or Monday and then discuss their take on it based on the proposed changes. Think that would make the discussion a lot more concrete.

As for the intended use of the tool and if this would fit their vision, I think we have a strong case since they (Chef) already do the exact same thing using knife bootstrap, so that clearly shows it is a valid use case IMO.

But let's just open the PR and with that open the discussion to see if we can come to a resolution that works for all involved.

Additionally I'm thinking of making and publishing a Terraform compatible fork/gem and use that gem in the provisioning for the time being. That would give us a quick fix, solving the problem right away for the very next release of Terraform.

Guess we should just do that in any case so that the issues with the current implementation are gone and we have time to try and get things fixed upstream.

Will keep you posted...

[svanharmelen]

@whiteley see chef/chef-vault#227 for any discussion/progress on this one.

[sysbot]

That's a great way to move forward with this. I was thinking about some hackish ways to add individual clients via search and rebuild the search string and refresh but this is simply way awesome.

[svanharmelen]

@whiteley @sysbot the required chef-vault changes are already merged upstream, so right after the next release from chef-vault we can fix this in Terraform in a proper way as well...

[rneu31]

Is there any progress on this issue? I am running into a similar problem. Noticed chef/chef-vault#227 has been merged.

[svanharmelen]

@rneu31 the PR is indeed merged, but the new version isn't released yet. We have to wait until version 3.0 is released (version 3.0RC1 is currently the latest).

As soon as 3.0 is released we can update this code and use that new version. So hang in there, will not be too long now...

[donwlewis]

I wanted to know what the status of this is. Looks like chef-vault 3.0 is released, but haven't seen an update on this. Would love to start using this, and it is kinda hinging on whether we move forward with leveraging Terraform, since we leverage vaults quite a bit. Thanks!

[svanharmelen]

@donwlewis I also noticed Chef-Vault 3.0 was released about 2 hours ago... So an update/fix for this will be in the next Terraform release.

[donwlewis]

LOL! I didn't even noticed that it had only been 2 hours. Thanks for the update! Looking forward to it!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.