Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall back to ssh_options[:user] if host.user.nil? #66

Merged
merged 1 commit into from
Oct 31, 2015

Conversation

mattbrictson
Copy link
Owner

Up until now Airbrussh has displayed the username using host.name. However, this may be nil in favor of using ssh_options[:user]. Use the SSH options in this case.

If for some reason no user is specified with either method, then omit the @ symbol and just print the hostname instead of @hostname.

Addresses #65.

@felixbuenemann does this look good to you?
/cc @robd

@mattbrictson
Copy link
Owner Author

Hmm.. I just realized that this does not address the scenario where the user is specified in the global SSH options (as opposed to host-specific), like this:

set :ssh_options, auth_methods: %w(publickey), user: fetch(:application)

So perhaps this PR is a dud.

@felixbuenemann
Copy link
Contributor

Yes, I am using the global ssh_options, because we always use the same username as the apps name across all servers.

@mattbrictson
Copy link
Owner Author

Actually, maybe this does work after all!

It looks like SSHKit merges the global options and the host-specific ones and sets the result on the Host object, which is where Airbrussh reads them from.

See: https://github.com/capistrano/sshkit/blob/v1.7.1/lib/sshkit/backends/netssh.rb#L186

host.ssh_options = Netssh.config.ssh_options.merge(host.ssh_options || {})

@felixbuenemann Can you test using this branch and see if it works for your scenario?

@mattbrictson mattbrictson reopened this Oct 30, 2015
@felixbuenemann
Copy link
Contributor

I tried with:

gem 'airbrussh', require: false, git: 'https://github.com/mattbrictson/airbrussh', ref: '1d662dc8f9ce487d332c0bf0b899095da03d7d9d'

But I'm still seeing ✔ 01 @example.com 0.052s.

The strange thing, reading through your changelog I would have expected to see example.com not @example.com. So maybe user is an empty string and not nil?

@felixbuenemann
Copy link
Contributor

Hmm, or bundler is playing tricks on me, let me verify it's actually using the proper code.

@felixbuenemann
Copy link
Contributor

Yepp, bundler did fool me, or to be precise rubygems-bundler, which doesn't seem to pick up git checkouts. After using bundle exec cap instead of cap it's working fine.

@felixbuenemann
Copy link
Contributor

@mattbrictson Thanks for the quick fix and this great gem!

Up until now Airbrussh has displayed the username using `host.name`. However,
this may be `nil` in favor of using `ssh_options[:user]`. Use the SSH options
in this case.

If for some reason no user is specified with either method, then omit the `@`
symbol and just print the hostname instead of `@hostname`.

Addresses #65.
mattbrictson added a commit that referenced this pull request Oct 31, 2015
Fall back to ssh_options[:user] if host.user.nil?
@mattbrictson mattbrictson merged commit c7b73b5 into master Oct 31, 2015
felixbuenemann added a commit to felixbuenemann/airbrussh that referenced this pull request Nov 1, 2015
This accounts for changes in sshkit's local username lookup introduced
by capistrano/sshkit#293 and also accounts for the conditional
formatting changes introduced by mattbrictson#66.

The latter did not previously cause failed tests, because the username
was stubbed in the tests, causing the relevant code to not be exercised.

Now the username is looked up through sshkit, which results in an empty
username on sshkit < 1.7.0, so the no-username display is triggered.
felixbuenemann added a commit to felixbuenemann/airbrussh that referenced this pull request Nov 1, 2015
This accounts for changes in sshkit's local username lookup introduced
by capistrano/sshkit#293 and also accounts for the conditional
formatting changes introduced by mattbrictson#66.

The latter did not previously cause failed tests, because the username
was stubbed in the tests, causing the relevant code to not be exercised.

Now the username is looked up through sshkit, which results in an empty
username on sshkit < 1.7.0, so the no-username display is triggered.

Unfortunately we need to keep the Etc stub for now to avoid crashing on
Windows with sshkit 1.7.0 and 1.7.1.
@mattbrictson mattbrictson deleted the matt/more-robust-user-at-host branch January 2, 2017 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants