provisioner/chef: support recreating clients and configuring Chef Vaults

[svanharmelen]

Fixes #3605 and adds the functionality suggested in PR #7440.

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

Also did some cleaning and (in two files) removed the pathorcontent constructions, in order to make the code consistent with the documentation.

Lastly I deleted 3 deprecated attributes, as they are deprecated for quite some time now, and I needed to deprecate the newer attributes now as well 😉

[jmccann]

@svanharmelen I got around to trying this out and ran into SSL issues even though I specified ssl_verify_mode of 'verify_none' as well as fetch_chef_certificates to true.

Resource code

resource "openstack_compute_instance_v2" "test" {
  name = "test"
  image_name = "ubuntu-1604"
  flavor_name = "smem-2vcpu"
  key_pair = "test"
  security_groups = ["${openstack_compute_secgroup_v2.ssh.name}"]

  network {
    "uuid" = "${openstack_networking_network_v2.test.id}"
  }
  floating_ip = "${openstack_compute_floatingip_v2.test.address}"

  provisioner "chef" {
    fetch_chef_certificates = true
    node_name = "test"
    recreate_client = true
    run_list = []
    server_url = "https://chef.mycorp.com/organizations/${var.chef_org}"
    ssl_verify_mode = "verify_none"
    user_name = "test-validator"
    user_key = "${base64decode(var.chef_validator_key)}"
    vault_json = "{\"vault_test\":\"test\"}"

    connection {
      bastion_host = "${openstack_compute_floatingip_v2.bastion_host_test.address}"
      user = "ubuntu"
      private_key = "${base64decode(var.os_ssh_pk)}"
    }
  }

  user_data = <<EOF
#cloud-config
hostname: test
fqdn: test.mycorp.com
manage_etc_hosts: true
${var.cloud_init_cacerts}
EOF
}

Error

openstack_compute_instance_v2.test: Still creating... (9s elapsed)
openstack_compute_instance_v2.test: Still creating... (10s elapsed)
openstack_compute_instance_v2.test: Provisioning with 'chef'...
openstack_compute_instance_v2.test (chef): Generate the private key...
openstack_compute_instance_v2.test (chef): Configure chef vaults...
Error applying plan:

1 error(s) occurred:

* WARNING: No knife configuration file found
ERROR: SSL Validation failure connecting to host: chef.mycorp.com - SSL_connect returned=1 errno=0 state=error: certificate verify failed
ERROR: Could not establish a secure connection to the server.
Use `knife ssl check` to troubleshoot your SSL configuration.
If your Chef Server uses a self-signed certificate, you can use
`knife ssl fetch` to make knife trust the server's certificates.

Original Exception: OpenSSL::SSL::SSLError: SSL Error connecting to https://chef.mycorp.com/organizations/test/data/vault_test/test_keys - SSL_connect returned=1 errno=0 state=error: certificate verify failed


Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

[svanharmelen]

@jmccann thanks for testing!

I totally did not think about supporting the ssl_verify_mode attribute while configuring the vaults. Will add that tomorrow morning and give you ping, when the PR is updated.

Good find, thx!

[svanharmelen]

And just to make things clear... Did you only add the ssl_verify_mode and fetch_chef_certificates attributes in order to try and make this work? Or do you also need those attributes for the new nodes you are creating?

I would suggest the latter, but would like to make sure if that is the case?

I'm thinking to just always fetch the certificates locally so the knife vault calls will always work. That way the ssl and cert related attributes are only used to configure your new nodes... But maybe it's logical to use the attributes for both purposes... Any thoughts?

[jmccann]

@svanharmelen I was using either ssl_verify_mode or fetch_chef_certificates to get the vault part to work since our Chef server uses internally signed certs. I wasn't trying to test them specifically.

I would only fetch certs of someone specifies fetch_chef_certificates as that could be a security concern otherwise I think. Once I know that this new vault code works I'll probably be working to make sure certificates are installed in the container I'm using to run terraform in ... then this will be a none issue for me. But I thought you'd want to know regardless as I'd expect it to work.

I'm guessing you already have thought of the following, but I'll say it anyways. 😛 I think to get the fetch_chef_certificates to work as expected you just need to have that execute before the p.createChefClient and p.configureVaults functions.

As for the ssl_verify_mode I'm not sure as there are no arguments it seems to control that from knife vault command.

$ knife vault update --help | grep ssl

I'll try to get my cert chain in order tomorrow to finish testing the new code outside of this issue and let you know. I can also test whatever fix you come up with as well once you have one. 😉

[svanharmelen]

@jmccann I just got around to updating this PR... I choose a slightly different approach in order to lose the requirement for have knife vault locally (and with that the possible certificate challenges that come with that).

So if you could have another go at it to confirm it now also works as expected for you, that would be great!

I did run all kinds of tests from both Windows and Linux creating both Windows and Linux instances and all seems to work perfectly for me. But I don't have a Chef server with certs I don't trust, so verifying that part would be nice.

If you're up for it, please test both certificate related options (fetch_chef_certificates and ssl_verify_mode) as they should now both work as expected.

Thx!

[jmccann]

@svanharmelen Working on trying it now. Will post back with results once I have them. Thanks!

[jmccann]

@svanharmelen So I ran into the following error:

openstack_compute_instance_v2.test (chef): Generate the private key...
openstack_compute_instance_v2.test: Still creating... (1m58s elapsed)
openstack_compute_instance_v2.test: Still creating... (2m0s elapsed)
openstack_compute_instance_v2.test (chef): WARN: Failed to read the private key /etc/chef/client.pem: #<Errno::ENOENT: No such file or directory @ rb_sysopen - /etc/chef/client.pem>
openstack_compute_instance_v2.test (chef): ERROR: Your private key could not be loaded from /etc/chef/client.pem
openstack_compute_instance_v2.test (chef): Check your configuration file and ensure that your private key is readable
openstack_compute_instance_v2.test (chef): Cleanup user key...
Error applying plan:

1 error(s) occurred:

* Command "sudo knife client create test -d -f /etc/chef/client.pem -c /etc/chef/client.rb -u drone-validator -k /etc/chef/drone-validator.pem" exited with non-zero exit status: 100

I think the options being passed for the knife client create are wrong or outdated. If I run the above failed command manually I get the same error. However i seems to work with:

root@test:/var/tmp# knife client create test -d -f /etc/chef/client.pem -c /etc/chef/client.rb -u drone-validator --key /etc/chef/drone-validator.pem
Created client[test]

Notice I changed -k to --key as in the knife help for me -k vs --key does:

        --key KEY                    API Client Key
    -k, --prevent-keygen             API V1 (Chef Server 12.1+) only. Prevent server from generating a default key pair for you. Cannot be passed with --public-key.

This may have been a change in behavior across different versions of Chef? This was with:

root@test:/var/tmp# knife --version
Chef: 12.13.37

[svanharmelen]

@jmccann that weird, as the current (for v12.13.x) Chef documentation says otherwise: https://docs.chef.io/release/12-13/knife_common_options.html

-k KEY, --key KEY
  The USER.pem file that knife uses to sign requests made by the API client to the Chef server.

But I double checked, and I'm indeed testing with an older Chef-Client version. So it seems I've been bitten by the docs here 😞 Will update in a few minutes!

[svanharmelen]

@jmccann updated so it now reads --key...

[jmccann]

@svanharmelen Things seem to be working for me now. 😄 👍

[svanharmelen]

@jmccann nice! Thank you very much for starting this up by opening PR #7440 and again sorry for all the delays...

And also thanks for helping to verify/test this solution. Really very much appreciated!!

@phinze could you do a review on the code itself? Functionally it's tested pretty well by now with all sorts of different setups and with all kind of Chef versions, so that part should be good.

[jen20]

This LGTM from a code perspective - I haven't tried running any tests however. If you're happy with it from the functional testing perspective (as the comments seems to suggest) @svanharmelen then feel free to merge!

[svanharmelen]

Yes, both me and @jmccann have tested this with various scenarios, so will go ahead and merge this one. Thx!

[jeffbyrnes]

@svanharmelen when might we expect this to land in a release version of Terraform?

[svanharmelen]

As it's now merged, it will be in the very next release...

[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.