From ad34d474bcb8079fdc5516f3eaad5962d0df68e9 Mon Sep 17 00:00:00 2001 From: Teemu Matilainen Date: Thu, 16 Jan 2014 23:03:38 -0300 Subject: [PATCH 1/2] provisioners/chef: DRY and unify encrypted data bag secret handling Pull up encrypted data bag secret management into the base classes, thus also unifying the functionality. Especially this complements the issues: * Upload the secret to provisioning path also with chef-solo [GH-1246] * Delete the secret also with chef-client [GH-2712] * Remove the secret before uploading also with chef-client [GH-1111] --- plugins/provisioners/chef/config/base.rb | 12 ++++++ .../provisioners/chef/config/chef_client.rb | 9 ---- plugins/provisioners/chef/config/chef_solo.rb | 9 ---- plugins/provisioners/chef/provisioner/base.rb | 29 +++++++++++++ .../chef/provisioner/chef_client.rb | 19 +-------- .../chef/provisioner/chef_solo.rb | 22 +--------- .../chef/provisioner/base_test.rb | 42 +++++++++++++++++++ 7 files changed, 86 insertions(+), 56 deletions(-) create mode 100644 test/unit/plugins/provisioners/chef/provisioner/base_test.rb diff --git a/plugins/provisioners/chef/config/base.rb b/plugins/provisioners/chef/config/base.rb index 5b1ebff83..532372ed4 100644 --- a/plugins/provisioners/chef/config/base.rb +++ b/plugins/provisioners/chef/config/base.rb @@ -11,6 +11,7 @@ module VagrantPlugins attr_accessor :binary_path attr_accessor :binary_env attr_accessor :custom_config_path + attr_accessor :encrypted_data_bag_secret_key_path attr_accessor :formatter attr_accessor :http_proxy attr_accessor :http_proxy_user @@ -36,6 +37,7 @@ module VagrantPlugins @binary_path = UNSET_VALUE @binary_env = UNSET_VALUE @custom_config_path = UNSET_VALUE + @encrypted_data_bag_secret_key_path = UNSET_VALUE @formatter = UNSET_VALUE @http_proxy = UNSET_VALUE @http_proxy_user = UNSET_VALUE @@ -55,6 +57,12 @@ module VagrantPlugins @run_list = [] end + def encrypted_data_bag_secret=(value) + puts "DEPRECATION: Chef encrypted_data_bag_secret has no effect anymore." + puts "Remove this from your Vagrantfile since it'll be removed in the next" + puts "Vagrant version." + end + def finalize! @arguments = nil if @arguments == UNSET_VALUE @attempts = 1 if @attempts == UNSET_VALUE @@ -76,6 +84,10 @@ module VagrantPlugins @file_cache_path = "/var/chef/cache" if @file_cache_path == UNSET_VALUE @verbose_logging = false if @verbose_logging == UNSET_VALUE + if @encrypted_data_bag_secret_key_path == UNSET_VALUE + @encrypted_data_bag_secret_key_path = nil + end + # Make sure the log level is a symbol @log_level = @log_level.to_sym diff --git a/plugins/provisioners/chef/config/chef_client.rb b/plugins/provisioners/chef/config/chef_client.rb index ba72aecb5..c009064d5 100644 --- a/plugins/provisioners/chef/config/chef_client.rb +++ b/plugins/provisioners/chef/config/chef_client.rb @@ -10,7 +10,6 @@ module VagrantPlugins attr_accessor :client_key_path attr_accessor :delete_client attr_accessor :delete_node - attr_accessor :encrypted_data_bag_secret_key_path attr_accessor :environment attr_accessor :validation_key_path attr_accessor :validation_client_name @@ -22,18 +21,11 @@ module VagrantPlugins @client_key_path = UNSET_VALUE @delete_client = UNSET_VALUE @delete_node = UNSET_VALUE - @encrypted_data_bag_secret_key_path = UNSET_VALUE @environment = UNSET_VALUE @validation_key_path = UNSET_VALUE @validation_client_name = UNSET_VALUE end - def encrypted_data_bag_secret=(value) - puts "DEPRECATION: Chef encrypted_data_bag_secret has no effect anymore." - puts "Remove this from your Vagrantfile since it'll be removed in the next" - puts "Vagrant version." - end - def finalize! super @@ -41,7 +33,6 @@ module VagrantPlugins @client_key_path = "/etc/chef/client.pem" if @client_key_path == UNSET_VALUE @delete_client = false if @delete_client == UNSET_VALUE @delete_node = false if @delete_node == UNSET_VALUE - @encrypted_data_bag_secret_key_path = nil if @encrypted_data_bag_secret_key_path == UNSET_VALUE @environment = nil if @environment == UNSET_VALUE @validation_client_name = "chef-validator" if @validation_client_name == UNSET_VALUE @validation_key_path = nil if @validation_key_path == UNSET_VALUE diff --git a/plugins/provisioners/chef/config/chef_solo.rb b/plugins/provisioners/chef/config/chef_solo.rb index a0e369f47..6142f2269 100644 --- a/plugins/provisioners/chef/config/chef_solo.rb +++ b/plugins/provisioners/chef/config/chef_solo.rb @@ -6,8 +6,6 @@ module VagrantPlugins class ChefSolo < Base attr_accessor :cookbooks_path attr_accessor :data_bags_path - attr_accessor :encrypted_data_bag_secret_key_path - attr_accessor :encrypted_data_bag_secret attr_accessor :environments_path attr_accessor :environment attr_accessor :recipe_url @@ -24,8 +22,6 @@ module VagrantPlugins @recipe_url = UNSET_VALUE @roles_path = UNSET_VALUE @synced_folder_type = UNSET_VALUE - @encrypted_data_bag_secret = UNSET_VALUE - @encrypted_data_bag_secret_key_path = UNSET_VALUE end def nfs=(value) @@ -67,11 +63,6 @@ module VagrantPlugins @data_bags_path = prepare_folders_config(@data_bags_path) @roles_path = prepare_folders_config(@roles_path) @environments_path = prepare_folders_config(@environments_path) - - @encrypted_data_bag_secret = "/tmp/encrypted_data_bag_secret" if \ - @encrypted_data_bag_secret == UNSET_VALUE - @encrypted_data_bag_secret_key_path = nil if \ - @encrypted_data_bag_secret_key_path == UNSET_VALUE end def validate(machine) diff --git a/plugins/provisioners/chef/provisioner/base.rb b/plugins/provisioners/chef/provisioner/base.rb index 8ce22bdd5..db09973a0 100644 --- a/plugins/provisioners/chef/provisioner/base.rb +++ b/plugins/provisioners/chef/provisioner/base.rb @@ -57,6 +57,7 @@ module VagrantPlugins config_file = Vagrant::Util::TemplateRenderer.render(template, { :custom_configuration => remote_custom_config_path, + :encrypted_data_bag_secret => guest_encrypted_data_bag_secret_key_path, :file_cache_path => @config.file_cache_path, :file_backup_path => @config.file_backup_path, :log_level => @config.log_level.to_sym, @@ -104,6 +105,34 @@ module VagrantPlugins comm.upload(temp.path, remote_file) end end + + def upload_encrypted_data_bag_secret + return if !@config.encrypted_data_bag_secret_key_path + + @machine.env.ui.info I18n.t( + "vagrant.provisioners.chef.upload_encrypted_data_bag_secret_key") + + remote_file = guest_encrypted_data_bag_secret_key_path + @machine.communicate.tap do |comm| + comm.sudo("rm -f #{remote_file}", error_check: false) + comm.upload(encrypted_data_bag_secret_key_path, remote_file) + end + end + + def delete_encrypted_data_bag_secret + @machine.communicate.sudo( + "rm -f #{guest_encrypted_data_bag_secret_key_path}", + error_check: false) + end + + def encrypted_data_bag_secret_key_path + File.expand_path(@config.encrypted_data_bag_secret_key_path, + @machine.env.root_path) + end + + def guest_encrypted_data_bag_secret_key_path + File.join(@config.provisioning_path, "encrypted_data_bag_secret_key") + end end end end diff --git a/plugins/provisioners/chef/provisioner/chef_client.rb b/plugins/provisioners/chef/provisioner/chef_client.rb index 30913b0c5..084e39458 100644 --- a/plugins/provisioners/chef/provisioner/chef_client.rb +++ b/plugins/provisioners/chef/provisioner/chef_client.rb @@ -20,10 +20,11 @@ module VagrantPlugins chown_provisioning_folder create_client_key_folder upload_validation_key - upload_encrypted_data_bag_secret if @config.encrypted_data_bag_secret_key_path + upload_encrypted_data_bag_secret setup_json setup_server_config run_chef_client + delete_encrypted_data_bag_secret end def cleanup @@ -43,12 +44,6 @@ module VagrantPlugins @machine.communicate.upload(validation_key_path, guest_validation_key_path) end - def upload_encrypted_data_bag_secret - @machine.env.ui.info I18n.t("vagrant.provisioners.chef.upload_encrypted_data_bag_secret_key") - @machine.communicate.upload(encrypted_data_bag_secret_key_path, - guest_encrypted_data_bag_secret_key_path) - end - def setup_server_config setup_config("provisioners/chef_client/client", "client.rb", { :node_name => @config.node_name, @@ -57,7 +52,6 @@ module VagrantPlugins :validation_key => guest_validation_key_path, :client_key => @config.client_key_path, :environment => @config.environment, - :encrypted_data_bag_secret => guest_encrypted_data_bag_secret_key_path, }) end @@ -98,15 +92,6 @@ module VagrantPlugins File.expand_path(@config.validation_key_path, @machine.env.root_path) end - def encrypted_data_bag_secret_key_path - File.expand_path(@config.encrypted_data_bag_secret_key_path, @machine.env.root_path) - end - - def guest_encrypted_data_bag_secret_key_path - File.join(@config.provisioning_path, - "encrypted_data_bag_secret_key.pem") - end - def guest_validation_key_path File.join(@config.provisioning_path, "validation.pem") end diff --git a/plugins/provisioners/chef/provisioner/chef_solo.rb b/plugins/provisioners/chef/provisioner/chef_solo.rb index 5830ab0ff..f26a66025 100644 --- a/plugins/provisioners/chef/provisioner/chef_solo.rb +++ b/plugins/provisioners/chef/provisioner/chef_solo.rb @@ -48,7 +48,7 @@ module VagrantPlugins chown_provisioning_folder verify_shared_folders(check) verify_binary(chef_binary_path("chef-solo")) - upload_encrypted_data_bag_secret if @config.encrypted_data_bag_secret_key_path + upload_encrypted_data_bag_secret setup_json setup_solo_config run_chef_solo @@ -115,21 +115,6 @@ module VagrantPlugins end end - def delete_encrypted_data_bag_secret - @machine.communicate.tap do |comm| - comm.sudo("rm -f #{@config.encrypted_data_bag_secret}", error_check: false) - end - end - - def upload_encrypted_data_bag_secret - @machine.env.ui.info I18n.t("vagrant.provisioners.chef.upload_encrypted_data_bag_secret_key") - @machine.communicate.tap do |comm| - comm.sudo("rm -f #{@config.encrypted_data_bag_secret}", :error_check => false) - comm.upload(encrypted_data_bag_secret_key_path, - @config.encrypted_data_bag_secret) - end - end - def setup_solo_config cookbooks_path = guest_paths(@cookbook_folders) roles_path = guest_paths(@role_folders).first @@ -141,7 +126,6 @@ module VagrantPlugins :recipe_url => @config.recipe_url, :roles_path => roles_path, :data_bags_path => data_bags_path, - :encrypted_data_bag_secret => @config.encrypted_data_bag_secret, :environments_path => environments_path, :environment => @config.environment, }) @@ -197,10 +181,6 @@ module VagrantPlugins end end - def encrypted_data_bag_secret_key_path - File.expand_path(@config.encrypted_data_bag_secret_key_path, @machine.env.root_path) - end - protected # Extracts only the remote paths from a list of folders diff --git a/test/unit/plugins/provisioners/chef/provisioner/base_test.rb b/test/unit/plugins/provisioners/chef/provisioner/base_test.rb new file mode 100644 index 000000000..1047c8e41 --- /dev/null +++ b/test/unit/plugins/provisioners/chef/provisioner/base_test.rb @@ -0,0 +1,42 @@ +require_relative "../../../../base" + +require Vagrant.source_root.join("plugins/provisioners/chef/provisioner/base") + +describe VagrantPlugins::Chef::Provisioner::Base do + include_context "unit" + + let(:machine) { double("machine") } + let(:config) { double("config") } + + subject { described_class.new(machine, config) } + + describe "#encrypted_data_bag_secret_key_path" do + let(:env) { double("env") } + let(:root_path) { "/my/root" } + + before do + machine.stub(:env).and_return(env) + env.stub(:root_path).and_return(root_path) + end + + it "returns absolute path as is" do + config.should_receive(:encrypted_data_bag_secret_key_path). + and_return("/foo/bar") + expect(subject.encrypted_data_bag_secret_key_path).to eq "/foo/bar" + end + + it "returns relative path joined to root_path" do + config.should_receive(:encrypted_data_bag_secret_key_path). + and_return("secret") + expect(subject.encrypted_data_bag_secret_key_path).to eq "/my/root/secret" + end + end + + describe "#guest_encrypted_data_bag_secret_key_path" do + it "returns path under config.provisioning_path" do + config.stub(:provisioning_path).and_return("/tmp/foo") + expect(File.dirname(subject.guest_encrypted_data_bag_secret_key_path)). + to eq "/tmp/foo" + end + end +end From eea9c07029887cb9d6734e0e6540dc8aaecfc0e2 Mon Sep 17 00:00:00 2001 From: Teemu Matilainen Date: Sun, 16 Feb 2014 18:14:56 -0300 Subject: [PATCH 2/2] provisioners/chef: set `encrypted_data_bag_secret` to `nil` if it's not uploaded /cc @shanegibbs Fixes #2984 --- plugins/provisioners/chef/provisioner/base.rb | 15 +++++++++------ templates/provisioners/chef_client/client.erb | 2 +- templates/provisioners/chef_solo/solo.erb | 2 +- .../provisioners/chef/provisioner/base_test.rb | 7 +++++++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/plugins/provisioners/chef/provisioner/base.rb b/plugins/provisioners/chef/provisioner/base.rb index db09973a0..6243eb927 100644 --- a/plugins/provisioners/chef/provisioner/base.rb +++ b/plugins/provisioners/chef/provisioner/base.rb @@ -107,12 +107,12 @@ module VagrantPlugins end def upload_encrypted_data_bag_secret - return if !@config.encrypted_data_bag_secret_key_path + remote_file = guest_encrypted_data_bag_secret_key_path + return if !remote_file @machine.env.ui.info I18n.t( "vagrant.provisioners.chef.upload_encrypted_data_bag_secret_key") - remote_file = guest_encrypted_data_bag_secret_key_path @machine.communicate.tap do |comm| comm.sudo("rm -f #{remote_file}", error_check: false) comm.upload(encrypted_data_bag_secret_key_path, remote_file) @@ -120,9 +120,10 @@ module VagrantPlugins end def delete_encrypted_data_bag_secret - @machine.communicate.sudo( - "rm -f #{guest_encrypted_data_bag_secret_key_path}", - error_check: false) + remote_file = guest_encrypted_data_bag_secret_key_path + if remote_file + @machine.communicate.sudo("rm -f #{remote_file}", error_check: false) + end end def encrypted_data_bag_secret_key_path @@ -131,7 +132,9 @@ module VagrantPlugins end def guest_encrypted_data_bag_secret_key_path - File.join(@config.provisioning_path, "encrypted_data_bag_secret_key") + if @config.encrypted_data_bag_secret_key_path + File.join(@config.provisioning_path, "encrypted_data_bag_secret_key") + end end end end diff --git a/templates/provisioners/chef_client/client.erb b/templates/provisioners/chef_client/client.erb index d2e4be31e..a1765b217 100644 --- a/templates/provisioners/chef_client/client.erb +++ b/templates/provisioners/chef_client/client.erb @@ -11,7 +11,7 @@ validation_client_name "<%= validation_client_name %>" validation_key "<%= validation_key %>" client_key "<%= client_key %>" -encrypted_data_bag_secret "<%= encrypted_data_bag_secret %>" +encrypted_data_bag_secret <%= encrypted_data_bag_secret.inspect %> <% if environment %> environment "<%= environment %>" diff --git a/templates/provisioners/chef_solo/solo.erb b/templates/provisioners/chef_solo/solo.erb index 95c8a30f4..a19cdcc03 100644 --- a/templates/provisioners/chef_solo/solo.erb +++ b/templates/provisioners/chef_solo/solo.erb @@ -10,7 +10,7 @@ role_path <%= roles_path.inspect %> log_level <%= log_level.inspect %> verbose_logging <%= verbose_logging.inspect %> -encrypted_data_bag_secret "<%= encrypted_data_bag_secret %>" +encrypted_data_bag_secret <%= encrypted_data_bag_secret.inspect %> <% if data_bags_path -%> data_bag_path <%= data_bags_path.inspect %> diff --git a/test/unit/plugins/provisioners/chef/provisioner/base_test.rb b/test/unit/plugins/provisioners/chef/provisioner/base_test.rb index 1047c8e41..7f2861b13 100644 --- a/test/unit/plugins/provisioners/chef/provisioner/base_test.rb +++ b/test/unit/plugins/provisioners/chef/provisioner/base_test.rb @@ -33,7 +33,14 @@ describe VagrantPlugins::Chef::Provisioner::Base do end describe "#guest_encrypted_data_bag_secret_key_path" do + it "returns nil if host path is not configured" do + config.stub(:encrypted_data_bag_secret_key_path).and_return(nil) + config.stub(:provisioning_path).and_return("/tmp/foo") + expect(subject.guest_encrypted_data_bag_secret_key_path).to be_nil + end + it "returns path under config.provisioning_path" do + config.stub(:encrypted_data_bag_secret_key_path).and_return("secret") config.stub(:provisioning_path).and_return("/tmp/foo") expect(File.dirname(subject.guest_encrypted_data_bag_secret_key_path)). to eq "/tmp/foo"