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.

Signed-off-by: R.I.Pienaar <[email protected]>
---
Local-branch: refactor/master/7223
 lib/mcollective/client.rb                    |    9 ++++++++-
 lib/mcollective/runner.rb                    |   21 +++++++++++----------
 lib/mcollective/security/base.rb             |   12 +++++++++++-
 lib/mcollective/util.rb                      |   14 ++++++++++++++
 plugins/mcollective/security/aes_security.rb |    4 ++--
 plugins/mcollective/security/psk.rb          |    4 ++--
 plugins/mcollective/security/sshkey.rb       |    4 ++--
 plugins/mcollective/security/ssl.rb          |    4 ++--
 spec/unit/security/base_spec.rb              |   16 ++++++++++++----
 spec/unit/util_spec.rb                       |   16 ++++++++++++++++
 website/changelog.md                         |    1 +
 website/reference/basic/messageformat.md     |   20 ++++++++++++++------
 12 files changed, 95 insertions(+), 30 deletions(-)

diff --git a/lib/mcollective/client.rb b/lib/mcollective/client.rb
index 052704f..89f43cc 100644
--- a/lib/mcollective/client.rb
+++ b/lib/mcollective/client.rb
@@ -42,7 +42,14 @@ module MCollective
 
             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}")
 
diff --git a/lib/mcollective/runner.rb b/lib/mcollective/runner.rb
index df5f12c..0d9720f 100644
--- a/lib/mcollective/runner.rb
+++ b/lib/mcollective/runner.rb
@@ -57,16 +57,17 @@ module MCollective
             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"
diff --git a/lib/mcollective/security/base.rb b/lib/mcollective/security/base.rb
index cebf5b5..9c9d611 100644
--- a/lib/mcollective/security/base.rb
+++ b/lib/mcollective/security/base.rb
@@ -133,14 +133,24 @@ module MCollective
                  :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
diff --git a/lib/mcollective/util.rb b/lib/mcollective/util.rb
index 67f3b18..dde8944 100644
--- a/lib/mcollective/util.rb
+++ b/lib/mcollective/util.rb
@@ -238,6 +238,20 @@ module MCollective
 
             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
 
diff --git a/plugins/mcollective/security/aes_security.rb 
b/plugins/mcollective/security/aes_security.rb
index 736fa03..b695f1b 100644
--- a/plugins/mcollective/security/aes_security.rb
+++ b/plugins/mcollective/security/aes_security.rb
@@ -102,10 +102,10 @@ module MCollective
             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"
diff --git a/plugins/mcollective/security/psk.rb 
b/plugins/mcollective/security/psk.rb
index 093af0a..0b02c7a 100644
--- a/plugins/mcollective/security/psk.rb
+++ b/plugins/mcollective/security/psk.rb
@@ -41,11 +41,11 @@ module MCollective
             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)
diff --git a/plugins/mcollective/security/sshkey.rb 
b/plugins/mcollective/security/sshkey.rb
index 5b64dde..1739ada 100644
--- a/plugins/mcollective/security/sshkey.rb
+++ b/plugins/mcollective/security/sshkey.rb
@@ -87,11 +87,11 @@ module MCollective
             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)
diff --git a/plugins/mcollective/security/ssl.rb 
b/plugins/mcollective/security/ssl.rb
index c4d8ff5..085b05b 100644
--- a/plugins/mcollective/security/ssl.rb
+++ b/plugins/mcollective/security/ssl.rb
@@ -108,11 +108,11 @@ module MCollective
             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)
diff --git a/spec/unit/security/base_spec.rb b/spec/unit/security/base_spec.rb
index 5304371..5c1adab 100644
--- a/spec/unit/security/base_spec.rb
+++ b/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
 
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index a9cfbfd..91e49b2 100644
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -239,5 +239,21 @@ module MCollective
                 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
diff --git a/website/changelog.md b/website/changelog.md
index fb48042..5b9c99c 100644
--- a/website/changelog.md
+++ b/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|
diff --git a/website/reference/basic/messageformat.md 
b/website/reference/basic/messageformat.md
index fe88782..3bd5324 100644
--- a/website/reference/basic/messageformat.md
+++ b/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.
-- 
1.7.1

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to