From db20f399fbbad856fcf53d36750b48162457ca92 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 13 Jan 2011 17:26:37 -0800 Subject: [PATCH] Improved puppet config validation --- lib/vagrant/provisioners/puppet.rb | 46 ++++++----- templates/locales/en.yml | 4 +- test/vagrant/provisioners/puppet_test.rb | 98 ++++++++++++++---------- 3 files changed, 84 insertions(+), 64 deletions(-) diff --git a/lib/vagrant/provisioners/puppet.rb b/lib/vagrant/provisioners/puppet.rb index d5343f034..1827bd4b8 100644 --- a/lib/vagrant/provisioners/puppet.rb +++ b/lib/vagrant/provisioners/puppet.rb @@ -14,29 +14,46 @@ module Vagrant attr_accessor :options def initialize - @manifest_file = "" + @manifest_file = nil @manifests_path = "manifests" @pp_path = "/tmp/vagrant-puppet" @options = [] end + + # Returns the manifests path expanded relative to the root path of the + # environment. + def expanded_manifests_path + Pathname.new(manifests_path).expand_path(env.root_path) + end + + # Returns the manifest file if set otherwise returns the box name pp file + # which may or may not exist. + def computed_manifest_file + manifest_file || "#{top.vm.box}.pp" + end + + def validate(errors) + super + + if !expanded_manifests_path.directory? + errors.add(I18n.t("vagrant.provisioners.puppet.manifests_path_missing", :path => expanded_manifests_path)) + return + end + + errors.add(I18n.t("vagrant.provisioners.puppet.manifest_missing", :manifest => computed_manifest_file)) if !expanded_manifests_path.join(computed_manifest_file).file? + end end def prepare - check_manifest_dir share_manifests end def provision! verify_binary("puppet") create_pp_path - set_manifest run_puppet_client end - def check_manifest_dir - Dir.mkdir(config.manifests_path) unless File.directory?(config.manifests_path) - end - def share_manifests env.config.vm.share_folder("manifests", config.pp_path, config.manifests_path) end @@ -54,25 +71,14 @@ module Vagrant end end - def set_manifest - @manifest = !config.manifest_file.empty? ? config.manifest_file : "#{env.config.vm.box}.pp" - - if File.exists?("#{config.manifests_path}/#{@manifest}") - env.ui.info I18n.t("vagrant.provisioners.puppet.manifest_to_run", :manifest => @manifest) - return @manifest - else - raise PuppetError, :_key => :manifest_missing, :manifest => @manifest - end - end - def run_puppet_client options = [config.options].flatten - options << @manifest + options << config.computed_manifest_file options = options.join(" ") command = "sudo -i 'cd #{config.pp_path}; puppet #{options}'" - env.ui.info I18n.t("vagrant.provisioners.puppet.running_puppet") + env.ui.info I18n.t("vagrant.provisioners.puppet.running_puppet", :manifest => config.computed_manifest_file) vm.ssh.execute do |ssh| ssh.exec! command do |ch, type, data| diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 2da5a74c5..cb85db533 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -472,9 +472,9 @@ en: could be because the PATH is not properly setup or perhaps Puppet is not installed on this guest. Puppet provisioning can not continue without Puppet properly installed. - running_puppet: "Running Puppet..." - manifest_to_run: "Puppet will use the %{manifest} manifest to configure your box." + running_puppet: "Running Puppet with %{manifest}..." manifest_missing: "The Puppet %{manifest} manifest is missing. You cannot configure this box." + manifests_path_missing: "The manifests path specified for Puppet does not exist: %{path}" puppet_server: not_detected: |- diff --git a/test/vagrant/provisioners/puppet_test.rb b/test/vagrant/provisioners/puppet_test.rb index 88175cbbe..8217707fa 100644 --- a/test/vagrant/provisioners/puppet_test.rb +++ b/test/vagrant/provisioners/puppet_test.rb @@ -2,19 +2,71 @@ require "test_helper" class PuppetProvisionerTest < Test::Unit::TestCase setup do + clean_paths + @klass = Vagrant::Provisioners::Puppet @action_env = Vagrant::Action::Environment.new(vagrant_env.vms[:default].env) @config = @klass::Config.new + @config.top = Vagrant::Config::Top.new(@action_env.env) + @config.top.vm.box = "foo" @action = @klass.new(@action_env, @config) @env = @action.env @vm = @action.vm end + context "config" do + setup do + @errors = Vagrant::Config::ErrorRecorder.new + + # Set a box + @config.top.vm.box = "foo" + + # Start in a valid state (verified by the first test) + @config.expanded_manifests_path.mkdir + File.open(@config.expanded_manifests_path.join(@config.computed_manifest_file), "w") { |f| f.puts "HELLO" } + end + + should "expand the manifest path relative to the root path" do + assert_equal File.expand_path(@config.manifests_path, @env.root_path), @config.expanded_manifests_path.to_s + end + + should "default the manifest file to the box name" do + assert_equal "#{@config.top.vm.box}.pp", @config.computed_manifest_file + end + + should "use the custom manifest file if set" do + @config.manifest_file = "woot.pp" + assert_equal "woot.pp", @config.computed_manifest_file + end + + should "be valid" do + @config.validate(@errors) + assert @errors.errors.empty? + end + + should "be invalid if the manifests path doesn't exist" do + @config.expanded_manifests_path.rmtree + @config.validate(@errors) + assert !@errors.errors.empty? + end + + should "be invalid if a custom manifests path doesn't exist" do + @config.manifests_path = "dont_exist" + @config.validate(@errors) + assert !@errors.errors.empty? + end + + should "be invalid if the manifest file doesn't exist" do + @config.expanded_manifests_path.join(@config.computed_manifest_file).unlink + @config.validate(@errors) + assert !@errors.errors.empty? + end + end + context "preparing" do should "share manifests" do - @action.expects(:check_manifest_dir).once @action.expects(:share_manifests).once @action.prepare end @@ -25,29 +77,11 @@ class PuppetProvisionerTest < Test::Unit::TestCase prov_seq = sequence("prov_seq") @action.expects(:verify_binary).with("puppet").once.in_sequence(prov_seq) @action.expects(:create_pp_path).once.in_sequence(prov_seq) - @action.expects(:set_manifest).once.in_sequence(prov_seq) @action.expects(:run_puppet_client).once.in_sequence(prov_seq) @action.provision! end end - context "check manifest_dir" do - setup do - @config.manifests_path = "manifests" - end - - should "should not create the manifest directory if it exists" do - File.expects(:directory?).with(@config.manifests_path).returns(true) - @action.check_manifest_dir - end - - should "create the manifest directory if it does not exist" do - File.stubs(:directory?).with(@config.manifests_path).returns(false) - Dir.expects(:mkdir).with(@config.manifests_path).once - @action.check_manifest_dir - end - end - context "share manifests folder" do setup do @manifests_path = "manifests" @@ -86,26 +120,6 @@ class PuppetProvisionerTest < Test::Unit::TestCase end end - context "setting the manifest" do - setup do - @config.stubs(:manifests_path).returns("manifests") - @config.stubs(:manifest_file).returns("foo.pp") - @env.config.vm.stubs(:box).returns("base") - end - - should "set the manifest if it exists" do - File.stubs(:exists?).with("#{@config.manifests_path}/#{@config.manifest_file}").returns(true) - @action.set_manifest - end - - should "raise an error if the manifest does not exist" do - File.stubs(:exists?).with("#{@config.manifests_path}/#{@config.manifest_file}").returns(false) - assert_raises(Vagrant::Provisioners::PuppetError) { - @action.set_manifest - } - end - end - context "running puppet client" do setup do @ssh = mock("ssh") @@ -117,19 +131,19 @@ class PuppetProvisionerTest < Test::Unit::TestCase end should "cd into the pp_path directory and run puppet" do - expect_puppet_command("puppet #{@manifest}") + expect_puppet_command("puppet #{@config.computed_manifest_file}") @action.run_puppet_client end should "cd into the pp_path directory and run puppet with given options when given as an array" do @config.options = ["--modulepath", "modules", "--verbose"] - expect_puppet_command("puppet --modulepath modules --verbose #{@manifest}") + expect_puppet_command("puppet --modulepath modules --verbose #{@config.computed_manifest_file}") @action.run_puppet_client end should "cd into the pp_path directory and run puppet with the options when given as a string" do @config.options = "--modulepath modules --verbose" - expect_puppet_command("puppet --modulepath modules --verbose #{@manifest}") + expect_puppet_command("puppet --modulepath modules --verbose #{@config.computed_manifest_file}") @action.run_puppet_client end end