Joe,

I apologize for not getting back to you earlier on that. I haven't had a chance to look over the patch carefully, so I was waiting until that happened...but I've been so overwhelmed with things to do that it hasn't happened yet. :(

I'm not sure how quickly I'll be back to get to it, but since it's on trac, it won't get overlooked at least, like patches tend to be when they're only posted on the list.

- Jamis

On Oct 24, 2007, at 1:24 PM, Joe wrote:


Jamis,

I created a bug on Trac for this, #9933. When you get a chance, take a
look and let me know if it is along the correct lines. I added some
notes about things that are incomplete, but that I didn't want to
start on until I had a feel for whether it was headed in the right
direction.

Thanks,
Joe

On Oct 13, 3:13 pm, Joe <[EMAIL PROTECTED]> wrote:
Jamis,

Here's a first cut at a proper patch, done against the repo head.

A few things to note. I'm not sure if the user_for methods should be
pushed into the connection factories. For now, I just did that, but
since both eventually call SSH.connect, it could presumably be pulled
up.

The @sessions variable is now a hash of hashes, and the
@failed_sessions is now a hash of arrays. For both, the first hash key
is the server. For the @sessions variable, the second hash's key is
the current user. For @failed_sessions, this is stored in the array,
indexed by server.

The connect_to now lazily establishes a connection based on the server
and user for that server, with the user being determined by server
user, options user, or ssh_options[:username], in that order.

Therunmethod now takes :as, and restores the :user variable when it
is done.

Finally, this doesn't do anything with :password right now. I'm not
sure how that should be handled, and I wanted to see how you felt
about this first before delving into that.

This passes the full test suite, though there are no new tests for
this code yet.

Joe

Index: test/configuration/connections_test.rb
===================================================================
--- test/configuration/connections_test.rb      (revision 7860)
+++ test/configuration/connections_test.rb      (working copy)
@@ -87,8 +87,8 @@
   end

   def
test_establish_connections_to_should_not_attempt_to_reestablish_exist ing_co nnections
-    Capistrano::SSH.expects(:connect).times(2).returns(:success)
-    @config.sessions[server("cap1")] = :ok
+    Capistrano::SSH.expects(:connect).times(3).returns(:success)
+    @config.establish_connections_to(%w(cap1).map { |s| server(s) })
     @config.establish_connections_to(%w(cap1 cap2 cap3).map { |s|
server(s) })
     assert %w(cap1 cap2 cap3), @config.sessions.keys.sort.map { |s|
s.host }
   end
Index: lib/capistrano/ssh.rb
===================================================================
--- lib/capistrano/ssh.rb       (revision 7860)
+++ lib/capistrano/ssh.rb       (working copy)
@@ -105,5 +105,9 @@
         retry
       end
     end
+
+    def self.user_for(server, options={})
+      server.user || options[:user] || (options[:ssh_options] || {})
[:username]
+    end
   end
 end
Index: lib/capistrano/gateway.rb
===================================================================
--- lib/capistrano/gateway.rb   (revision 7860)
+++ lib/capistrano/gateway.rb   (working copy)
@@ -113,6 +113,10 @@
       connection
     end

+    def user_for(server)
+      SSH.user_for(server, @options)
+    end
+
     private

       def logger
Index: lib/capistrano/configuration/actions/invocation.rb
===================================================================
--- lib/capistrano/configuration/actions/invocation.rb (revision 7860) +++ lib/capistrano/configuration/actions/invocation.rb (working copy)
@@ -48,9 +48,19 @@

           options = add_default_command_options(options)

-          execute_on_servers(options) do |servers|
-            targets = servers.map { |s| sessions[s] }
-            Command.process(cmd, targets, options.merge(:logger =>
logger), &block)
+          as = options.delete(:as)
+          begin
+            u = fetch(:user, nil)
+            if as
+              reset!(:password)
+              set(:user, as) if as
+            end
+            execute_on_servers(options) do |servers|
+              targets = servers.map { |s| sessions[s] }
+              Command.process(cmd, targets, options.merge(:logger =>
logger), &block)
+            end
+          ensure
+            set(:user, u) if as
           end
         end

Index: lib/capistrano/configuration/connections.rb
===================================================================
--- lib/capistrano/configuration/connections.rb (revision 7860)
+++ lib/capistrano/configuration/connections.rb (working copy)
@@ -19,29 +19,45 @@
         def connect_to(server)
           SSH.connect(server, @options)
         end
+
+        def user_for(server)
+          SSH.user_for(server, @options)
+        end
       end
+
+      def initialize_with_connections(*args) #:nodoc:
+        initialize_without_connections(*args)
+        @sessions = {}
+        @failed_sessions = {}
+      end

+      # Obtain the username that is used for logins based on the
current settings
+      def user_for(server)
+        connection_factory.user_for(server)
+      end
+
       # A hash of the SSH sessions that are currently open and
available.
       # Because sessions are constructed lazily, this will only
contain
       # connections to those servers that have been the targets of
one or more
       # executed tasks.
-      attr_reader :sessions
-
-      def initialize_with_connections(*args) #:nodoc:
-        initialize_without_connections(*args)
-        @sessions = {}
-        @failed_sessions = []
+      def sessions
+        @sessions.keys.inject({}) do |result, server|
+          result[server] = @sessions[server][user_for(server)]
+          result
+        end
       end
-
+
       # Indicate that the given server could not be connected to.
       def failed!(server)
-        @failed_sessions << server
+        @failed_sessions[server] ||= []
+        @failed_sessions[server] << user_for(server)
       end

       # Query whether previous connection attempts to the given
server have
       # failed.
       def has_failed?(server)
-        @failed_sessions.include?(server)
+        return false if not @failed_sessions.has_key?(server)
+        @failed_sessions[server].include?(user_for(server))
       end

       # Used to force connections to be made to the current task's
servers.
@@ -141,6 +157,8 @@

       private

+        attr_writer :sessions
+
         # We establish the connection by creating a thread in a new
method--this
         # prevents problems with the thread's scope seeing the wrong
'server'
         # variable if the thread just happens to take too long to
start up.
@@ -149,7 +167,8 @@
         end

         def safely_establish_connection_to(server, failures=nil)
-          sessions[server] ||= connection_factory.connect_to(server)
+          @sessions[server] ||= {}
+          @sessions[server][user_for(server)] ||=
connection_factory.connect_to(server)
         rescue Exception => err
           raise unless failures
           failures << { :server => server, :error => err }

On Oct 13, 12:35 pm, Joe <[EMAIL PROTECTED]> wrote:

Jamis,

Ok, thanks, I will. I think the main thing will be to check if the
user has changed for the session, and close it conditionally, or cache
the open one with the different username. Additionally, it'd have to
cache the password for the different users.

Joe

On Oct 12, 4:16 pm, Jamis Buck <[EMAIL PROTECTED]> wrote:

I wouldn't be opposed to a patch for this, but as you discovered,
Capistrano currently makes some assumptions that make it hard to do
this "right". Feel free to write up a patch, though, and we can
negotiate the correct implementation. :)

- Jamis

On Oct 11, 2007, at 6:24 PM, Joe wrote:

Hi Jamis,

I have another feature request for you. I have a deployment where I have a series of tasks that get executed on various machines, but I need to login to those machines with public key auth with different user ids. In this particular scenario, using sudo does not really work
as needed. What I want to do is something like:

run"some_command", :as => 'root'

which temporarily changes the 'user' variable for the duration of the
execution of the task.

I accomplish this by aliasing therunmethod as follows:

module CapistranoExt
  class Configuration
    module Actions
      module Invocation
        def self.append_features(base)
          super
          base.extend(ClassMethods)
          base.class_eval do
            alias_method_chain :run, :user
          end
        end

        module ClassMethods
        end

# TODO: this really only works if the authentication method is # publickey, since the password is cached from a call to the # proc to read it. Need to have a mechanism to cache that per
        # user, and set it appropriately in here (if we need to
support
        # this mechanism with password-based auth).
        def run_with_user(cmd, options={}, &block)
          u = fetch(:user)
          as = options.delete(:as) || u
          begin
            set :user, as
            run_without_user(cmd, options, &block)
          ensure
            set :user, u
          end
        end
      end
    end
  end
end

This also requires a change to the following method in connections.rb
(from Cap 2.0.100):

def safely_establish_connection_to(server, failures=nil)
  if fetch(:ssh_options)[:no_session_cache]
    sessions[server].close if sessions[server]
    sessions[server] = connection_factory.connect_to(server)
  else
    sessions[server] ||= connection_factory.connect_to(server)
  end
rescue Exception => err
  raise unless failures
  failures << { :server => server, :error => err }
end

Then I set ssh_options[:no_session_cache] to true in my cap deployment
file, and everything works fine.

Per my comments in the code, there is a problem in that this only
works for non-password auth for now, since the password is obtained in
a Proc and I did not add code to cache it per user. This could
probably be done with a few more additions.

It's probably a little inefficient. I looked to see if there was a way
to drop the session only if the username had changed, but I don't
think net/ssh exposes that

...

read more ยป


--~--~---------~--~----~------------~-------~--~----~
To unsubscribe from this group, send email to capistrano- [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/ group/capistrano
-~----------~----~----~----~------~----~------~--~---


Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to