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.
The run method 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_connections
- 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 the run method 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 field.
>
> > > Anyway, if you think you'd be willing to add something like this, I
> > > could probably clean it up and just give you a patch.
>
> > > Thanks,
> > > Joe
>
> > smime.p7s
> > 3KDownload
--~--~---------~--~----~------------~-------~--~----~
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/capistrano
-~----------~----~----~----~------~----~------~--~---
