Fixing #3139 - all properties can now be audited

This provides a full audit trail for any parameter on any
resource Puppet can manage.  Just use:

    file { "/my/file": audit => [content, owner] }

And Puppet will generate an event any time either of
those properties change.

This commit also deprecates the 'check' parameter in favor of
a new 'audit' parameter.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>

4 files changed, 131 insertions, 3 deletions

diff --git a/spec/unit/transaction/change.rb b/spec/unit/transaction/change.rb
index 183414777..9419bbab9 100755
--- a/spec/unit/transaction/change.rb
+++ b/spec/unit/transaction/change.rb
@@ -41,6 +41,11 @@ describe Puppet::Transaction::Change do
             @change.noop?.should be_true
         end
 
+        it "should be auditing if set so" do
+            @change.auditing = true
+            @change.must be_auditing
+        end
+
         it "should set its resource to the proxy if it has one" do
             @change.proxy = :myresource
             @change.resource.should == :myresource
@@ -107,6 +112,27 @@ describe Puppet::Transaction::Change do
                 end
             end
 
+            describe "in audit mode" do
+                before { @change.auditing = true }
+
+                it "should log that it is in audit mode" do
+                    @property.expects(:is_to_s)
+                    @property.expects(:should_to_s)
+
+                    @event.expects(:message=).with { |msg| msg.include?("audit") }
+
+                    @change.apply
+                end
+
+                it "should produce a :audit event and return" do
+                    @property.stub_everything
+
+                    @event.expects(:status=).with("audit")
+
+                    @change.apply.should == @event
+                end
+            end
+
             it "should sync the property" do
                 @property.expects(:sync)
 
diff --git a/spec/unit/transaction/event.rb b/spec/unit/transaction/event.rb
index 6a837b50f..85811c105 100755
--- a/spec/unit/transaction/event.rb
+++ b/spec/unit/transaction/event.rb
@@ -33,7 +33,7 @@ describe Puppet::Transaction::Event do
         event.status.should == "success"
     end
 
-    it "should fail if the status is not to 'noop', 'success', or 'failure" do
+    it "should fail if the status is not to 'audit', 'noop', 'success', or 'failure" do
         event = Puppet::Transaction::Event.new
         lambda { event.status = "foo" }.should raise_error(ArgumentError)
     end
diff --git a/spec/unit/transaction/resource_harness.rb b/spec/unit/transaction/resource_harness.rb
index ee2726d07..cbb796cde 100755
--- a/spec/unit/transaction/resource_harness.rb
+++ b/spec/unit/transaction/resource_harness.rb
@@ -25,6 +25,38 @@ describe Puppet::Transaction::ResourceHarness do
         Puppet::Transaction::ResourceHarness.new(@transaction).relationship_graph.should == "relgraph"
     end
 
+    describe "when copying audited parameters" do
+        before do
+            @resource = Puppet::Type.type(:file).new :path => "/foo/bar", :audit => :mode
+        end
+
+        it "should do nothing if no parameters are being audited" do
+            @resource[:audit] = []
+            @harness.expects(:cached).never
+            @harness.copy_audited_parameters(@resource, {}).should == []
+        end
+
+        it "should do nothing if an audited parameter already has a desired value set" do
+            @resource[:mode] = "755"
+            @harness.expects(:cached).never
+            @harness.copy_audited_parameters(@resource, {}).should == []
+        end
+
+        it "should copy any cached values to the 'should' values" do
+            @harness.cache(@resource, :mode, "755")
+            @harness.copy_audited_parameters(@resource, {}).should == [:mode]
+
+            @resource[:mode].should == 0755
+        end
+
+        it "should cache and log the current value if no cached values are present" do
+            @resource.expects(:notice)
+            @harness.copy_audited_parameters(@resource, {:mode => "755"}).should == []
+
+            @harness.cached(@resource, :mode).should == "755"
+        end
+    end
+
     describe "when evaluating a resource" do
         it "should create and return a resource status instance for the resource" do
             @harness.evaluate(@resource).should be_instance_of(Puppet::Resource::Status)
@@ -133,6 +165,20 @@ describe Puppet::Transaction::ResourceHarness do
             @harness.changes_to_perform(@status, @resource)
         end
 
+        it "should copy audited parameters" do
+            @resource[:audit] = :mode
+            @harness.cache(@resource, :mode, "755")
+            @harness.changes_to_perform(@status, @resource)
+            @resource[:mode].should == 0755
+        end
+
+        it "should mark changes created as a result of auditing as auditing changes" do
+            @current_state[:mode] = 0644
+            @resource[:audit] = :mode
+            @harness.cache(@resource, :mode, "755")
+            @harness.changes_to_perform(@status, @resource)[0].must be_auditing
+        end
+
         describe "and the 'ensure' parameter is present but not in sync" do
             it "should return a single change for the 'ensure' parameter" do
                 @resource[:ensure] = :present
@@ -204,8 +250,8 @@ describe Puppet::Transaction::ResourceHarness do
 
     describe "when applying changes" do
         before do
-            @change1 = stub 'change1', :apply => stub("event", :status => "success")
-            @change2 = stub 'change2', :apply => stub("event", :status => "success")
+            @change1 = stub 'change1', :apply => stub("event", :status => "success"), :auditing? => false
+            @change2 = stub 'change2', :apply => stub("event", :status => "success"), :auditing? => false
             @changes = [@change1, @change2]
         end
 
@@ -228,6 +274,17 @@ describe Puppet::Transaction::ResourceHarness do
             @status.events.should be_include(@change1.apply)
             @status.events.should be_include(@change2.apply)
         end
+
+        it "should cache the new value if it is an auditing change" do
+            @change1.expects(:auditing?).returns true
+            property = stub 'property', :name => "foo", :resource => "myres"
+            @change1.stubs(:property).returns property
+            @change1.stubs(:is).returns "myval"
+
+            @harness.apply_changes(@status, @changes)
+
+            @harness.cached("myres", "foo").should == "myval"
+        end
     end
 
     describe "when determining whether the resource can be changed" do
diff --git a/spec/unit/type.rb b/spec/unit/type.rb
index e7888a389..e3ae5e62d 100755
--- a/spec/unit/type.rb
+++ b/spec/unit/type.rb
@@ -482,3 +482,48 @@ describe Puppet::Type::RelationshipMetaparam do
         param.validate_relationship
     end
 end
+
+describe Puppet::Type.metaparamclass(:check) do
+    it "should warn and create an instance of ':audit'" do
+        file = Puppet::Type.type(:file).new :path => "/foo"
+        file.expects(:warning)
+        file[:check] = :mode
+        file[:audit].should == [:mode]
+    end
+end
+
+describe Puppet::Type.metaparamclass(:audit) do
+    before do
+        @resource = Puppet::Type.type(:file).new :path => "/foo"
+    end
+
+    it "should default to being nil" do
+        @resource[:audit].should be_nil
+    end
+
+    it "should specify all possible properties when asked to audit all properties" do
+        @resource[:audit] = :all
+
+        list = @resource.class.properties.collect { |p| p.name }
+        @resource[:audit].should == list
+    end
+
+    it "should fail if asked to audit an invalid property" do
+        lambda { @resource[:audit] = :foobar }.should raise_error(Puppet::Error)
+    end
+
+    it "should create an attribute instance for each auditable property" do
+        @resource[:audit] = :mode
+        @resource.parameter(:mode).should_not be_nil
+    end
+
+    it "should accept properties specified as a string" do
+        @resource[:audit] = "mode"
+        @resource.parameter(:mode).should_not be_nil
+    end
+
+    it "should not create attribute instances for parameters, only properties" do
+        @resource[:audit] = :noop
+        @resource.parameter(:noop).should be_nil
+    end
+end