From 8eb0e16afdcd328fd562a7ffa026a6481c14413c Mon Sep 17 00:00:00 2001 From: Michael Knox Date: Fri, 18 Mar 2011 22:37:00 +1100 Subject: (#2728) Add diff output for changes made by Augeas provider Utilising Augeas's SAVE_NEWFILE mode (similar to augtool -n) to determine the changes that will be made be made by Augeas. Output a unified diff to info handle non-default root, and multiple files correctly Adding tests for Augeas diff functionality Add test for non-default :root when diff'ing Ensure that multiple files are diffed if changed, not just one Signed-off-by: Josh Cooper Reviewed-by: Jacob Helwig --- lib/puppet/provider/augeas/augeas.rb | 59 ++++++++++++----- spec/unit/provider/augeas/augeas_spec.rb | 107 ++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 18 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index a16f54bd7..95142568e 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -15,9 +15,12 @@ require 'augeas' if Puppet.features.augeas? require 'strscan' +require 'puppet/util' +require 'puppet/util/diff' Puppet::Type.type(:augeas).provide(:augeas) do include Puppet::Util + include Puppet::Util::Diff confine :true => Puppet.features.augeas? @@ -25,6 +28,8 @@ Puppet::Type.type(:augeas).provide(:augeas) do SAVE_NOOP = "noop" SAVE_OVERWRITE = "overwrite" + SAVE_NEWFILE = "newfile" + SAVE_BACKUP = "backup" COMMANDS = { "set" => [ :path, :string ], @@ -254,11 +259,6 @@ Puppet::Type.type(:augeas).provide(:augeas) do @aug.set("/augeas/save", mode) end - def files_changed? - saved_files = @aug.match("/augeas/events/saved") - saved_files.size > 0 - end - # Determines if augeas acutally needs to run. def need_to_run? force = resource[:force] @@ -287,20 +287,33 @@ Puppet::Type.type(:augeas).provide(:augeas) do # actually do the save. if return_value and get_augeas_version >= "0.3.6" debug("Will attempt to save and only run if files changed") - set_augeas_save_mode(SAVE_NOOP) + set_augeas_save_mode(SAVE_NEWFILE) do_execute_changes save_result = @aug.save saved_files = @aug.match("/augeas/events/saved") - if save_result and not files_changed? - debug("Skipping because no files were changed") - return_value = false - else + if save_result and saved_files.size > 0 + root = resource[:root].sub(/^\/$/, "") + saved_files.each do |key| + saved_file = @aug.get(key).to_s.sub(/^\/files/, root) + if Puppet[:show_diff] + print diff(saved_file, saved_file + ".augnew") + end + if resource.noop? + File.delete(saved_file + ".augnew") + end + end debug("Files changed, should execute") + return_value = true + else + debug("Skipping because no files were changed or save failed") + return_value = false end end end ensure - close_augeas + if not return_value or resource.noop? + close_augeas + end end return_value end @@ -309,12 +322,24 @@ Puppet::Type.type(:augeas).provide(:augeas) do # Re-connect to augeas, and re-execute the changes begin open_augeas - set_augeas_save_mode(SAVE_OVERWRITE) if get_augeas_version >= "0.3.6" - - do_execute_changes - - success = @aug.save - fail("Save failed with return code #{success}") if success != true + saved_files = @aug.match("/augeas/events/saved") + if saved_files + saved_files.each do |key| + root = resource[:root].sub(/^\/$/, "") + saved_file = @aug.get(key).to_s.sub(/^\/files/, root) + if File.exists?(saved_file + ".augnew") + success = File.rename(saved_file + ".augnew", saved_file) + debug(saved_file + ".augnew moved to " + saved_file) + fail("Rename failed with return code #{success}") if success != 0 + end + end + else + debug("No saved files, re-executing augeas") + set_augeas_save_mode(SAVE_OVERWRITE) if get_augeas_version >= "0.3.6" + do_execute_changes + success = @aug.save + fail("Save failed with return code #{success}") if success != true + end ensure close_augeas end diff --git a/spec/unit/provider/augeas/augeas_spec.rb b/spec/unit/provider/augeas/augeas_spec.rb index 5bb98eadf..687043b31 100755 --- a/spec/unit/provider/augeas/augeas_spec.rb +++ b/spec/unit/provider/augeas/augeas_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' provider_class = Puppet::Type.type(:augeas).provider(:augeas) describe provider_class do - describe "command parsing" do before do @resource = stub("resource") @@ -252,6 +251,7 @@ describe provider_class do it "should handle no filters" do resource = stub("resource") resource.stubs(:[]).returns(false).then.returns("").then.returns("") + resource.stubs(:noop?).returns(false) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) augeas_stub.stubs("close") provider = provider_class.new(resource) @@ -263,6 +263,7 @@ describe provider_class do it "should return true when a get filter matches" do resource = stub("resource") resource.stubs(:[]).returns(false).then.returns("get path == value").then.returns("") + resource.stubs(:noop?).returns(false) provider = provider_class.new(resource) augeas_stub = stub("augeas", :get => "value") augeas_stub.stubs("close") @@ -285,6 +286,7 @@ describe provider_class do it "should return true when a match filter matches" do resource = stub("resource") resource.stubs(:[]).returns(false).then.returns("match path size == 3").then.returns("") + resource.stubs(:noop?).returns(false) provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) augeas_stub.stubs("close") @@ -320,6 +322,7 @@ describe provider_class do it "should return true when a size != the provided value" do resource = stub("resource") resource.stubs(:[]).returns(false).then.returns("match path size != 17").then.returns("") + resource.stubs(:noop?).returns(false) provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) augeas_stub.stubs("close") @@ -339,6 +342,107 @@ describe provider_class do provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == false end + + # Ticket 2728 (diff files) + describe "and Puppet[:show_diff] is set" do + before do + Puppet[:show_diff] = true + + @resource = Puppet::Type.type(:augeas).new(:name => "test") + @provider = provider_class.new(@resource) + @augeas_stub = stub("augeas") + @provider.aug = @augeas_stub + + @augeas_stub.stubs("get").with("/augeas/version").returns("0.7.2") + @augeas_stub.stubs(:set).returns(true) + @augeas_stub.stubs(:save).returns(true) + end + + it "should call diff when a file is shown to have been changed" do + file = "/etc/hosts" + + @resource[:context] = "/files" + @resource[:changes] = ["set #{file}/foo bar"] + + @augeas_stub.stubs(:match).with("/augeas/events/saved").returns(["/augeas/events/saved"]) + @augeas_stub.stubs(:get).with("/augeas/events/saved").returns(["/files#{file}"]) + @augeas_stub.expects(:set).with("/augeas/save", "newfile") + @augeas_stub.expects(:close).never() + + @provider.expects("diff").with("#{file}", "#{file}.augnew").returns("") + @provider.should be_need_to_run + end + + it "should call diff for each file thats changed" do + file1 = "/etc/hosts" + file2 = "/etc/resolv.conf" + + @resource[:context] = "/files" + @resource[:changes] = ["set #{file1}/foo bar", "set #{file2}/baz biz"] + + @augeas_stub.stubs(:match).with("/augeas/events/saved").returns(["/augeas/events/saved[1]", "/augeas/events/saved[2]"]) + @augeas_stub.stubs(:get).with("/augeas/events/saved[1]").returns(["/files#{file1}"]) + @augeas_stub.stubs(:get).with("/augeas/events/saved[2]").returns(["/files#{file2}"]) + @augeas_stub.expects(:set).with("/augeas/save", "newfile") + @augeas_stub.expects(:close).never() + + @provider.expects(:diff).with("#{file1}", "#{file1}.augnew").returns("") + @provider.expects(:diff).with("#{file2}", "#{file2}.augnew").returns("") + @provider.should be_need_to_run + end + + describe "and resource[:root] is set" do + it "should call diff when a file is shown to have been changed" do + root = "/tmp/foo" + file = "/etc/hosts" + + @resource[:context] = "/files" + @resource[:changes] = ["set #{file}/foo bar"] + @resource[:root] = root + + @augeas_stub.stubs(:match).with("/augeas/events/saved").returns(["/augeas/events/saved"]) + @augeas_stub.stubs(:get).with("/augeas/events/saved").returns(["/files#{file}"]) + @augeas_stub.expects(:set).with("/augeas/save", "newfile") + @augeas_stub.expects(:close).never() + + @provider.expects(:diff).with("#{root}#{file}", "#{root}#{file}.augnew").returns("") + @provider.should be_need_to_run + end + end + + it "should not call diff if no files change" do + file = "/etc/hosts" + + @resource[:context] = "/files" + @resource[:changes] = ["set #{file}/foo bar"] + + @augeas_stub.stubs(:match).with("/augeas/events/saved").returns([]) + @augeas_stub.expects(:set).with("/augeas/save", "newfile") + @augeas_stub.expects(:get).with("/augeas/events/saved").never() + @augeas_stub.expects(:close) + + @provider.expects(:diff).never() + @provider.should_not be_need_to_run + end + + it "should cleanup when in noop mode" do + file = "/etc/hosts" + + @resource[:noop] = true + @resource[:context] = "/files" + @resource[:changes] = ["set #{file}/foo bar"] + + @augeas_stub.stubs(:match).with("/augeas/events/saved").returns(["/augeas/events/saved"]) + @augeas_stub.stubs(:get).with("/augeas/events/saved").returns(["/files#{file}"]) + @augeas_stub.expects(:set).with("/augeas/save", "newfile") + @augeas_stub.expects(:close) + + File.expects(:delete).with(file + ".augnew") + + @provider.expects(:diff).with("#{file}", "#{file}.augnew").returns("") + @provider.should be_need_to_run + end + end end describe "augeas execution integration" do @@ -349,6 +453,7 @@ describe provider_class do @augeas = stub("augeas") @provider.aug= @augeas @provider.stubs(:get_augeas_version).returns("0.3.5") + @augeas.stubs(:match).with("/augeas/events/saved") end it "should handle set commands" do -- cgit From 77a598724f961600170d8ba22cd9a11bbb7ab1e1 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 2 Jun 2011 17:36:40 -0700 Subject: maint: Confine augeas specs to require the augeas feature --- spec/unit/provider/augeas/augeas_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/provider/augeas/augeas_spec.rb b/spec/unit/provider/augeas/augeas_spec.rb index 687043b31..434a99d70 100755 --- a/spec/unit/provider/augeas/augeas_spec.rb +++ b/spec/unit/provider/augeas/augeas_spec.rb @@ -344,7 +344,7 @@ describe provider_class do end # Ticket 2728 (diff files) - describe "and Puppet[:show_diff] is set" do + describe "and Puppet[:show_diff] is set", :if => Puppet.features.augeas? do before do Puppet[:show_diff] = true -- cgit From c5448b79e690d72536f4d98b636f450d37de627a Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Sun, 22 May 2011 21:54:58 -0400 Subject: (#7624) Auditing should not be enabled by default for purged resources. - Auditing creates logging messages as of 2.6.5 so it should not be enabled by default. - This patch removes the :audit => :all settting from resources created via self.instances (which is used for purging). Please note that we believe this change to be safe, and *should* not result in user-visible behavioural differences when you use the `instances` method on a type, but we can't give you a perfect assurance of that. If you do have code that depends on the current behaviour, and it misbehaves after this patch, please let us know so we can weep ^W find another solution that works for everyone. Reviewed-By: Daniel Pittman Reviewed-By: Nigel Kersten --- lib/puppet/type.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 1dbf12451..586bb46f6 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -886,7 +886,7 @@ class Type end provider_instances[instance.name] = instance - new(:name => instance.name, :provider => instance, :audit => :all) + new(:name => instance.name, :provider => instance) end end.flatten.compact end -- cgit From 31bf55c2f9c090c40e48f38c5f64e4373a402261 Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Mon, 23 May 2011 11:14:03 -0700 Subject: (#7632) Make secret_agent application compatible with secret_agent face Running `puppet secret_agent` was failing with an error in validate_args (in interface/action.rb), because the application was trying to pass the certname as an argument to the synchronize action. Also, it was trying to submit the report a second time. Reviewing the code with Nick0, we found that the application wasn't inheriting from FaceBase and was duplicating a lot of work, and were able to resolve the issue by basically deleting the whole thing. This patch makes secret_agent behave like the other Faces apps, and makes synchronize the default action of the secret_agent face. We left the `run_mode :agent` line in the application because of bug #7802. Paired-with: Nick Lewis --- lib/puppet/application/secret_agent.rb | 21 ++------------------- lib/puppet/face/secret_agent.rb | 1 + 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/lib/puppet/application/secret_agent.rb b/lib/puppet/application/secret_agent.rb index d704d14b2..6bdbc9232 100644 --- a/lib/puppet/application/secret_agent.rb +++ b/lib/puppet/application/secret_agent.rb @@ -1,23 +1,6 @@ -require 'puppet/application' +require 'puppet/application/face_base' require 'puppet/face' -class Puppet::Application::Secret_agent < Puppet::Application - should_parse_config +class Puppet::Application::Secret_agent < Puppet::Application::FaceBase run_mode :agent - - option("--debug", "-d") - option("--verbose", "-v") - - def setup - if options[:debug] or options[:verbose] - Puppet::Util::Log.level = options[:debug] ? :debug : :info - end - - Puppet::Util::Log.newdestination(:console) - end - - def run_command - report = Puppet::Face[:secret_agent, '0.0.1'].synchronize(Puppet[:certname]) - Puppet::Face[:report, '0.0.1'].submit(report) - end end diff --git a/lib/puppet/face/secret_agent.rb b/lib/puppet/face/secret_agent.rb index 105850cd3..79241da72 100644 --- a/lib/puppet/face/secret_agent.rb +++ b/lib/puppet/face/secret_agent.rb @@ -15,6 +15,7 @@ Puppet::Face.define(:secret_agent, '0.0.1') do EOT action(:synchronize) do + default summary "Run secret_agent once." arguments "[-v | --verbose] [-d | --debug]" # Remove this once options are introspectible description <<-'EOT' -- cgit From c8df02703ac29b00140648640f85ec044a9cfd6e Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 6 Jun 2011 14:40:39 -0700 Subject: (#7193) Fix path issues with acceptance tests that call old shell tests Some rearranging of the acceptance test directories caused some ruby tests that referenced old shell tests to lose their relative paths. Paired-with: Dominic Maraglia --- .../resource/service/ticket_4123_should_list_all_running_redhat.rb | 2 +- .../tests/resource/service/ticket_4124_should_list_all_disabled.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb b/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb index b4c2bc14d..9bedd6e04 100755 --- a/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb +++ b/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb @@ -7,6 +7,6 @@ hosts.each do |host| unless host['platform'].include? 'centos' or host['platform'].include? 'redhat' skip_test "Test not supported on this plaform" else - run_script_on(host,'tests/acceptance/resource/service/ticket_4123_should_list_all_running_redhat.sh') + run_script_on(host,'acceptance-tests/tests/resource/service/ticket_4123_should_list_all_running_redhat.sh') end end diff --git a/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb b/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb index 4add108ff..db96ad91c 100755 --- a/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb +++ b/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb @@ -7,6 +7,6 @@ hosts.each do |host| unless host['platform'].include? 'centos' or host['platform'].include? 'redhat' skip_test "Test not supported on this plaform" else - run_script_on(host,'tests/acceptance/resource/service/ticket_4124_should_list_all_disabled.sh') + run_script_on(host,'acceptance-tests/tests/resource/service/ticket_4124_should_list_all_disabled.sh') end end -- cgit From d4e6c268aecabdbf36ece126f960ce2384576fcc Mon Sep 17 00:00:00 2001 From: Luke Kaines Date: Mon, 6 Jun 2011 15:33:49 -0700 Subject: (#7624) Manually fetch all properties in instances. When we removed the `:audit => :all` flag on creation of an instance from a Puppet type, we stopped fetching all the properties. This had flow-on consequences that were visible from the outside; while some places did their own work to ensure that properties were fetched, others didn't. We now open-code the loop that creates and fetches those properties, to ensure that we have the same data without going through the :audit machinery. This resolves the excessive logging, and also eliminates the behavioural change that we introduced in the previous commit. Reviewed-By: Daniel Pittman --- lib/puppet/type.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 586bb46f6..558491a7f 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -876,6 +876,12 @@ class Type # Put the default provider first, then the rest of the suitable providers. provider_instances = {} providers_by_source.collect do |provider| + all_properties = self.properties.find_all do |property| + provider.supports_parameter?(property) + end.collect do |property| + property.name + end + provider.instances.collect do |instance| # We always want to use the "first" provider instance we find, unless the resource # is already managed and has a different provider set @@ -886,7 +892,9 @@ class Type end provider_instances[instance.name] = instance - new(:name => instance.name, :provider => instance) + result = new(:name => instance.name, :provider => instance) + properties.each { |name| result.newattr(name) } + result end end.flatten.compact end -- cgit