7223 - Include the agent and collective in requests

Improve portability by not relying on the structure of the
message targets to determine destination agent and collective.

Backward compatability with older security plugins and clients
is maintained.

lib/mcollective/client.rb

@@ -42,7 +42,14 @@ def sendreq(msg, agent, filter = {})
 
             reqid = Digest::MD5.hexdigest("#{@config.identity}-#{Time.now.to_f.to_s}-#{target}")
 
-            req = @security.encoderequest(@config.identity, target, msg, reqid, filter)
+            # Security plugins now accept an agent and collective, ones written for <= 1.1.4 dont
+            # but we still want to support them, try to call them in a compatible way if they
+            # dont support the new arguments
+            begin
+                req = @security.encoderequest(@config.identity, target, msg, reqid, filter, agent, collective)
+            rescue ArgumentError
+                req = @security.encoderequest(@config.identity, target, msg, reqid, filter)
+            end
 
             Log.debug("Sending request #{reqid} to #{target}")
 

lib/mcollective/runner.rb

@@ -57,16 +57,17 @@ def run
             loop do
                 begin
                     msg = receive
-                    dest = msg[:msgtarget]
-
-                    sep = Regexp.escape(@config.topicsep)
-                    prefix = Regexp.escape(@config.topicprefix)
-                    regex = "#{prefix}(.+?)#{sep}(.+?)#{sep}command"
-                    if dest.match(regex)
-                        collective = $1
-                        agent = $2
-                    else
-                        raise "Failed to handle message, could not figure out agent and collective from #{dest}"
+
+                    collective = msg[:collective]
+                    agent = msg[:agent]
+
+                    # requests from older clients would not include the
+                    # :collective and :agent this parses the target in
+                    # a backward compat way for them
+                    unless collective && agent
+                        parsed_dest = Util.parse_msgtarget(msg[:msgtarget])
+                        collective = parsed_dest[:collective]
+                        agent = parsed_dest[:agent]
                     end
 
                     if agent == "mcollective"

lib/mcollective/security/base.rb

@@ -133,14 +133,24 @@ def create_reply(reqid, agent, target, body)
                  :body => body}
             end
 
-            def create_request(reqid, target, filter, msg, initiated_by)
+            def create_request(reqid, target, filter, msg, initiated_by, target_agent=nil, target_collective=nil)
                 Log.debug("Encoding a request for '#{target}' with request id #{reqid}")
 
+                # for backward compat with <= 1.1.4 security plugins we parse the
+                # msgtarget to figure out the agent and collective
+                unless target_agent && target_collective
+                    parsed_target = Util.parse_msgtarget(target)
+                    target_agent = parsed_target[:agent]
+                    target_collective = parsed_target[:collective]
+                end
+
                 req = {:body => msg,
                        :senderid => @config.identity,
                        :requestid => reqid,
                        :msgtarget => target,
                        :filter => filter,
+                       :collective => target_collective,
+                       :agent => target_agent,
                        :msgtime => Time.now.to_i}
 
                 # if we're in use by a client add the callerid to the main client hashes

lib/mcollective/util.rb

@@ -238,6 +238,20 @@ def self.shellescape(str)
 
             return str
         end
+
+        # Parse the msgtarget as sent in 1.1.4 and newer to figure out the
+        # agent and collective that a request is targeted at
+        def self.parse_msgtarget(target)
+            sep = Regexp.escape(Config.instance.topicsep)
+            prefix = Regexp.escape(Config.instance.topicprefix)
+            regex = "#{prefix}(.+?)#{sep}(.+?)#{sep}command"
+
+            if target.match(regex)
+                return {:collective => $1, :agent => $2}
+            else
+                raise "Failed to handle message, could not figure out agent and collective from #{target}"
+            end
+        end
     end
 end
 

plugins/mcollective/security/aes_security.rb

@@ -102,10 +102,10 @@ def encodereply(sender, target, msg, requestid, requestcallerid)
             end
 
             # Encodes a request msg
-            def encoderequest(sender, target, msg, requestid, filter={})
+            def encoderequest(sender, target, msg, requestid, filter={}, target_agent=nil, target_collective=nil)
                 crypted = encrypt(serialize(msg), callerid)
 
-                req = create_request(requestid, target, filter, crypted[:data], @initiated_by)
+                req = create_request(requestid, target, filter, crypted[:data], @initiated_by, target_agent, target_collective)
                 req[:sslkey] = crypted[:key]
 
                 if @config.pluginconf.include?("aes.send_pubkey") && @config.pluginconf["aes.send_pubkey"] == "1"

plugins/mcollective/security/psk.rb

@@ -41,11 +41,11 @@ def encodereply(sender, target, msg, requestid, requestcallerid=nil)
             end
 
             # Encodes a request msg
-            def encoderequest(sender, target, msg, requestid, filter={})
+            def encoderequest(sender, target, msg, requestid, filter={}, target_agent=nil, target_collective=nil)
                 serialized = Marshal.dump(msg)
                 digest = makehash(serialized)
 
-                req = create_request(requestid, target, filter, serialized, @initiated_by)
+                req = create_request(requestid, target, filter, serialized, @initiated_by, target_agent, target_collective)
                 req[:hash] = digest
 
                 Marshal.dump(req)

plugins/mcollective/security/sshkey.rb

@@ -87,11 +87,11 @@ def encodereply(sender, target, msg, requestid, requestcallerid=nil)
             end
 
             # Encodes a request msg
-            def encoderequest(sender, target, msg, requestid, filter={})
+            def encoderequest(sender, target, msg, requestid, filter={}, target_agent=nil, target_collective=nil)
                 serialized = Marshal.dump(msg)
                 digest = makehash(serialized)
 
-                req = create_request(requestid, target, filter, serialized, @initiated_by)
+                req = create_request(requestid, target, filter, serialized, @initiated_by, target_agent, target_collective)
                 req[:hash] = digest
 
                 Marshal.dump(req)

plugins/mcollective/security/ssl.rb

@@ -108,11 +108,11 @@ def encodereply(sender, target, msg, requestid, requestcallerid=nil)
             end
 
             # Encodes a request msg
-            def encoderequest(sender, target, msg, requestid, filter={})
+            def encoderequest(sender, target, msg, requestid, filter={}, target_agent=nil, target_collective=nil)
                 serialized = serialize(msg)
                 digest = makehash(serialized)
 
-                req = create_request(requestid, target, filter, serialized, @initiated_by)
+                req = create_request(requestid, target, filter, serialized, @initiated_by, target_agent, target_collective)
                 req[:hash] = digest
 
                 serialize(req)

spec/unit/security/base_spec.rb

@@ -8,6 +8,8 @@ module MCollective::Security
             @config = mock("config")
             @config.stubs(:identity).returns("test")
             @config.stubs(:configured).returns(true)
+            @config.stubs(:topicsep).returns(".")
+            @config.stubs(:topicprefix).returns("/topic/")
 
             @stats = mock("stats")
 
@@ -151,24 +153,30 @@ module MCollective::Security
                 expected = {:body => "body",
                             :senderid => "test",
                             :requestid => "reqid",
-                            :msgtarget => "target",
+                            :msgtarget => "/topic/mcollective.discovery.command",
+                            :agent => "discovery",
+                            :collective => "mcollective",
                             :filter => "filter",
                             :msgtime => @time}
 
-                @plugin.create_request("reqid", "target", "filter", "body", :server).should == expected
+                @plugin.create_request("reqid", "/topic/mcollective.discovery.command", "filter", "body", :server, "discovery", "mcollective").should == expected
+                @plugin.create_request("reqid", "/topic/mcollective.discovery.command", "filter", "body", :server).should == expected
             end
 
             it "should set the callerid when appropriate" do
                 expected = {:body => "body",
                             :senderid => "test",
                             :requestid => "reqid",
-                            :msgtarget => "target",
+                            :msgtarget => "/topic/mcollective.discovery.command",
+                            :agent => "discovery",
+                            :collective => "mcollective",
                             :filter => "filter",
                             :callerid => "callerid",
                             :msgtime => @time}
 
                 @plugin.stubs(:callerid).returns("callerid")
-                @plugin.create_request("reqid", "target", "filter", "body", :client).should == expected
+                @plugin.create_request("reqid", "/topic/mcollective.discovery.command", "filter", "body", :client, "discovery", "mcollective").should == expected
+                @plugin.create_request("reqid", "/topic/mcollective.discovery.command", "filter", "body", :client).should == expected
             end
         end
 

spec/unit/util_spec.rb

@@ -239,5 +239,21 @@ class MCollective::Connector::Stomp<MCollective::Connector::Base; end
                 Util.parse_fact_string("foo == bar").should == {:fact => "foo", :value => "bar", :operator => "=="}
             end
         end
+
+        describe "#parse_msgtarget" do
+            it "should correctly parse supplied targets based on config" do
+                Config.any_instance.stubs("topicsep").returns(".")
+                Config.any_instance.stubs("topicprefix").returns("/topic/")
+
+                Util.parse_msgtarget("/topic/mcollective.discovery.command").should == {:collective => "mcollective", :agent => "discovery"}
+            end
+
+            it "should raise an error on failure" do
+                Config.any_instance.stubs("topicsep").returns(".")
+                Config.any_instance.stubs("topicprefix").returns("/topic/")
+
+                expect { Util.parse_msgtarget("foo") }.to raise_error(/could not figure out agent and collective from foo/)
+            end
+        end
     end
 end

website/changelog.md

@@ -11,6 +11,7 @@ title: Changelog
 
 |Date|Description|Ticket|
 |----|-----------|------|
+|2011/04/23|Encode the target agent and collective in requests|7223|
 |2011/04/20|Make the SSL Cipher used a config option|7191|
 |2011/04/20|Add a clear method to the PluginManager that deletes all plugins, improve test isolation|7176|
 |2011/04/19|Abstract the creation of request and reply hashes to simplify connector plugin development|5701|

website/reference/basic/messageformat.md

@@ -26,6 +26,12 @@ There is also a [screencast][ScreenCast] that shows this process and message for
 ## Message Flow
 For details of the flow of messages and how requests / replies travel around the network see the [MessageFlow] page.
 
+## History
+
+|Date|Description|Ticket|
+|----|-----------|------|
+|2011/04/23|Add _agent_ and _collective_ to the request hashes|7113|
+
 ### Requests sent to agents
 A sample request that gets sent to the connector can be seen here, each component is described below:
 
@@ -34,12 +40,14 @@ A sample request that gets sent to the connector can be seen here, each componen
   {"cf_class"      => ["common::linux"],
    "fact"          => [{:fact=>"country", :value=>"uk"}],
    "agent"         => ["package"]},
- :senderid  => "devel.your.com",
- :msgtarget => "/topic/mcollective.discovery.command",
- :body      => body,
- :hash      => "2d437f2904980ac32d4ebb7ca1fd740b",
- :msgtime   => 1258406924,
- :requestid => "0b54253cb5d04eb8b26ea75bbf468cbc"}
+ :senderid    => "devel.your.com",
+ :msgtarget   => "/topic/mcollective.discovery.command",
+ :agent:      => 'discovery',
+ :collective' => 'mcollective',
+ :body        => body,
+ :hash        => "2d437f2904980ac32d4ebb7ca1fd740b",
+ :msgtime     => 1258406924,
+ :requestid   => "0b54253cb5d04eb8b26ea75bbf468cbc"}
 {% endhighlight %}
 
 Once this request is created the security plugin will serialize it and sent it to the connector, in the case of the PSK security plugin this is done using Marshal.