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

provisioner/chef: Add bootstrapping with knife to support chef-vault #7440

Closed

Conversation

jmccann
Copy link

@jmccann jmccann commented Jun 30, 2016

Use Case

I would like the ability to bootstrap a node that has a run_list that utilizes vaults from chef-vault

Problem

Current chef provisioner bootstraps a node by using a validator and running chef-client on the node. This does not allow you to add the node to chef-vaults and therefore can not have an initial run_list that includes cookbooks/recipes that utilize a chef-vault.

Fix

I am adding code to the chef provisioner to allow bootstrapping a node with knife bootstrap rather then running chef-client and using a validator. Newer versions of knife included with Chef allow "validatorless" bootstrapping and supports flag --bootstrap-vault-json which allows specifying chef-vaults to add the node to.

Allows setting up node with `knife bootstrap` to gain access
to chef-vault vaults while bootstrapping nodes
@jmccann jmccann changed the title Add bootstrapping with knife to chef provisioner to support chef-vault provisioner/chef: Add bootstrapping with knife to support chef-vault Jun 30, 2016
@svanharmelen
Copy link
Contributor

@jmccann thanks for the PR! I can tell you invested some time into this one, but I must admit that it feels a little weird to me to use the knife bootstrap approach for this... IMO it would make much more sense to just call the knife bootstrap command from a local-exec provisioner, instead of calling it from the remote host with a target address of 127.0.0.1.

I understand that you miss some functionality in the current Chef provisioner, but I wonder if this is the best way to add that functionality. I will need to find some time to take a closer look at this PR (will try to get around to that shortly) and will also do some thinking about if there are better approaches or that this is the only way to achieve your goals...

@jmccann
Copy link
Author

jmccann commented Jul 5, 2016

@svanharmelen Thanks for taking a quick look at this and for your comments. I understand some of the concerns you bring up and initially had the same concerns. I'll explain why I feel this is an OK approach and how it helps me and people I work with.

First let me clarify I'm by no means a terraform expert and am learning things as I go. ;)

I'm trying to use source control and CI/CD to manage my infrastructure. Specifically I'm using Drone with drone-terraform for managing my infra with terraform. Unfortunately drone-terraform does not contain the Chef stack inside of it so local-exec using that would not work.

What I had been doing prior to creating this code and PR was passing user_data to my openstack instances that did pretty much the same thing. I have multiple openstack instances that I do not have direct access to. The problem with this approach was every time I try to update some minor Chef attribute it wants to rebuild the system. Another problem was it was hard to explain to people what was going on.

I then saw other co-workers using the chef provisioner to get around both of these issues by setting a bastion host for the connection. This was pretty much exactly what I wanted and was very easy to use except that it does not support chef-vaults.

One thing I did think of before working on this was using remote-exec to run the knife bootstrap. But that also by default runs on the system I am provisioning. So I would basically end up doing in remote-exec what I've tried to add to provisioner/chef (bootstrapping a node from itself). But it would be harder for people to consume later.

For me having a good pattern to give people I work with for provisioning Chef, configuring Chef and having it have access to chef-vaults on the first Chef run is what I'm trying to accomplish. A lot of people are already familiar with using provisioner/chef. So I thought extending it to also support bootstrapping a node into a chef-vault would be good.

@jmccann
Copy link
Author

jmccann commented Aug 17, 2016

@svanharmelen I was wondering if you have some time to review this and think it over more. Again, my use case is the ability to bootstrap a system with access to chef-vaults using the chef provisioner.

I think the primary concern should be attributes I've added to the provisioner (bootstrap and vaults_json). If the frontend is good then people can start using this feature as is. Then as people have time and/or find better ways to implement the backend it can then be refactored without impact to the user. For example, I've thought of removing bootstrap and having the bootstrap method determined solely based on the presence of vaults_json.

I'm hoping this is a wanted feature and that we can work through any implementation details.

Thanks!

@svanharmelen
Copy link
Contributor

@jmccann sorry for the lack of response here! I'm currently on holiday, but will be back coming week (Tuesday) and will make time to process this one so we can move it forward.

So please give me another few days and then see if we can get this (or a slightly adjusted version) merged end of next week.

Thx!

@svanharmelen
Copy link
Contributor

So I ended up ill the day before getting back from holiday an so I didn't have a look yet... Did think about possible solutions and I think I have a good approach which I will try to work out today/tomorrow.

Will keep you posted!

@svanharmelen
Copy link
Contributor

So I think I have a very nice solution, but I will need to run some tests on OSX, Linux and Windows to confirm it all works as expected first... Will update and share (if all works as expected) the solution after I'm done testing.

@jmccann
Copy link
Author

jmccann commented Aug 30, 2016

@svanharmelen Awesome! Excited to see what you have.

@svanharmelen
Copy link
Contributor

The solution I'm working on also fixes issue #3605. I managed to test and verify that part, but got disturbed with other work when I was about to go and test the vault part... So that will have to wait until tomorrow, but it looks good 😉

@svanharmelen
Copy link
Contributor

@jmccann again sorry for all the delays, but please have a look at PR #8577.

The only code changes that are directly related to this PR are in this file and as you can see there are effectively two new functions here. One for fixing issue #3605 and one for adding Chef Vault support.

The rest is mainly updating and cleaning code and tests in order to get the Chef provisioner up-to-spec again.

Let me know what you think of this approach and if you are able to build from this branch and give it a test run, that would be great.

@mrmarbury
Copy link

Hi,

does this also include vaults that are already on the server and would be added to the bootstrap command via --bootstrap-vault-item or is it just for vaults that are created with the new Node?

Cheers

@jmccann
Copy link
Author

jmccann commented Sep 6, 2016

@mrmarbury This is to allow bootstrapping a new node to existing vaults on the server. This work is being done under #8577 now.

@bottkv488
Copy link

@jmccann, @svanharmelen
Thank you so much for taking care of this, as
running knife vault refresh foo bar inside a local-exec block is such a time consuming nuisance

@jmccann
Copy link
Author

jmccann commented Sep 13, 2016

Closing in favor of #8577

@jmccann jmccann closed this Sep 13, 2016
svanharmelen pushed a commit that referenced this pull request Sep 15, 2016
Fixes #3605 and adds the functionality suggested in PR #7440.

This PR is using a different appraoch that (IMHO) feels cleaner and (even more important) adds support for Windows at the same time.
sharmaansh21 pushed a commit to sharmaansh21/terraform that referenced this pull request Sep 15, 2016
Fixes hashicorp#3605 and adds the functionality suggested in PR hashicorp#7440.

This PR is using a different appraoch that (IMHO) feels cleaner and (even more important) adds support for Windows at the same time.
@ghost
Copy link

ghost commented Apr 22, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants