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_existing_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 [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/capistrano
-~----------~----~----~----~------~----~------~--~---
