Fixing #2789 puppetrun fails without --tag

Puppet::Transaction was handling "tags" strings differently depending on
whether they came in from Puppet[:tags] or another source.
This was causing puppetrun's tags to be misparsed if there was not
exactly one --tag parameter.

I've moved the code to Util::Tagging.

3 files changed, 39 insertions, 12 deletions

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index a0d5b16a9..e132b7238 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -2,6 +2,7 @@
 # and performs them
 
 require 'puppet'
+require 'puppet/util/tagging'
 
 module Puppet
 class Transaction
@@ -18,6 +19,7 @@ class Transaction
     attr_reader :events
 
     include Puppet::Util
+    include Puppet::Util::Tagging
 
     # Add some additional times for reporting
     def addtimes(hash)
@@ -602,20 +604,17 @@ class Transaction
     # The tags we should be checking.
     def tags
         unless defined? @tags
-            tags = Puppet[:tags]
-            if tags.nil? or tags == ""
-                @tags = []
-            else
-                @tags = tags.split(/\s*,\s*/)
-            end
+            self.tags = Puppet[:tags]
         end
 
-        @tags
+        super
     end
 
-    def tags=(tags)
-        tags = [tags] unless tags.is_a?(Array)
-        @tags = tags
+    def handle_qualified_tags( qualified )
+        # The default behavior of Puppet::Util::Tagging is
+        # to split qualified tags into parts. That would cause
+        # qualified tags to match too broadly here.
+        return
     end
 
     # Is this resource tagged appropriately?
diff --git a/lib/puppet/util/tagging.rb b/lib/puppet/util/tagging.rb
index 03a8b8a9a..9ee90799f 100644
--- a/lib/puppet/util/tagging.rb
+++ b/lib/puppet/util/tagging.rb
@@ -16,8 +16,7 @@ module Puppet::Util::Tagging
             @tags << tag unless @tags.include?(tag)
         end
 
-        # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com]
-        qualified.collect { |name| x = name.split("::") }.flatten.each { |tag| @tags << tag unless @tags.include?(tag) }
+        handle_qualified_tags( qualified )
     end
 
     # Are we tagged with the provided tag?
@@ -32,8 +31,27 @@ module Puppet::Util::Tagging
         @tags.dup
     end
 
+    def tags=(tags)
+        @tags = []
+
+        return if tags.nil? or tags == ""
+
+        if tags.is_a?(String)
+            tags = tags.strip.split(/\s*,\s*/)
+        end
+
+        tags.each do |t|
+            tag(t)
+        end
+    end
+
     private
 
+    def handle_qualified_tags( qualified )
+        # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com]
+        qualified.collect { |name| x = name.split("::") }.flatten.each { |tag| @tags << tag unless @tags.include?(tag) }
+    end
+
     def valid_tag?(tag)
         tag =~ /^\w[-\w:.]*$/
     end
diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb
index ff01eac22..1b3562153 100755
--- a/spec/unit/transaction.rb
+++ b/spec/unit/transaction.rb
@@ -133,4 +133,14 @@ describe Puppet::Transaction, " when determining tags" do
         @transaction.tags = "one::two"
         @transaction.tags.should == %w{one::two}
     end
+
+    it "should accept a comma-delimited string" do
+        @transaction.tags = "one, two"
+        @transaction.tags.should == %w{one two}
+    end
+
+    it "should accept an empty string" do
+        @transaction.tags = ""
+        @transaction.tags.should == []
+    end
 end