#as and #with don't work together

[leafac]

The following script triggers the error:

require 'sshkit/dsl'

SSHKit.config.output_verbosity = Logger::DEBUG

run_locally do
  as :root do
    with foo: 'bar' do
      execute :env
    end
  end
end

It doesn't print the FOO environment variable. This happens because environment variables should be set after calling /usr/bin/env, not before everything else in the command, as it is.

For example, the command generated by the script above is:

$ FOO=bar sudo su root -c "/usr/bin/env env"

It should be:

$ sudo su root -c "/usr/bin/env FOO=bar env"

For this reason, I believe the first example in README is broken:

require 'sshkit/dsl'

on %w{1.example.com 2.example.com}, in: :sequence, wait: 5 do
  within "/opt/sites/example.com" do
    as :deploy  do
      with rails_env: :production do
        rake   "assets:precompile"
        runner "S3::Sync.notify"
      end
    end
  end
end

Maybe this line in README is about this issue:

No environment handling (sshkit might not need to care)

If that is the case, I believe it would be nice to make it clear.

If this is really an issue, please let me know so I can work to fix it.

[leehambley]

If this is really an issue, please let me know so I can work to fix it.

Sounds like, although I'm sure we have tests for how commands are constructed. If it's broken (and I'll take your word for it!) Then I'd really appreciate a PR!

[leafac]

Fine, I'll get to that soon.

[leafac]

I wrote a solution to the issue. It's a bit of a hack, but it was the only way I found to adhere to the documentation of how env(1) is meant to be used.

[mmzoo]

I can confirm this issue, it really seems to be broken. Crazy that this has not been revealed earlier :)

[leehambley]

I'm sure we have tests for this, could be that they're broken. I noticed when I ran the suite there were broken tests. Perhaps some could take a look and PR to fix the test suite, maybe that fixes this issue and it's a silly typo or something.

[leafac]

When I cloned the repository and ran the tests four of them were failing.

I tried to fix them, but they had nothing to do with this issue. Therefore, I figured they would be fixed by someone touching other parts of the codebase.

This PR leaves those four failing tests untouched.

[leehambley]

Correct, everything is fine - will merge this or give feedback in the next
week or so
On 18 Nov 2013 19:24, "Leandro Facchinetti" [email protected]
wrote:

When I cloned the repository and ran the tests four of them were failing.

I tried to fix them, but they had nothing to do with this issue.
Therefore, I figured they would be fixed by someone touching other parts of
the codebase.

This PR leaves those four failing tests untouched.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/35#issuecomment-28723718
.

[leafac]

❤️

[mmzoo]

It seems to me that people use command_map more and more (instead of less and less). However, when switching to another user using sudo su, the command_map is not respected at all. So commands are not run the same way as another user as with the "primary" user.

I'm afraid of the sshkit codebase running into a corner with all these "quick" patches applied in parallel. But I'd have to blame myself if I don't participate with PRs :)

(Just as a side note, my use case is that I would like to source my .bash_profile whenever I switch to another user with sshkit's sudo su. Currently there is no way for me to do that.)

UPDATE: I very much agree with this issue that switching users should not be done using sudo but, e.g., by establishing a new ssh connection with another user. That would be a solution for this issue and for my use case, too.

[leehambley]

It seems to me that people use command_map more and more (instead of less and less).

The improved command map is a requirement for Capistrano, and @kirs is a core contributor there, there's been a lot of discussion around this feature.

However, when switching to another user using sudo su, the command_map is not respected at all. So commands are not run the same way as another user as with the "primary" user.

This is a matter of there being no correct way to do shell escaping if you go ahead and do some escaping yourself, you can pass options to execute() to force the command to be mapped, if you know what you are doing. (Check the source)

SSHKit also provides as() which is the correct way to do this, and has worked for every use case I've ever needed. I see from your message (commented below) that you are trying to source dotfiles, there's a reason we've made that essentially impossible.

I'm afraid of the sshkit codebase running into a corner with all these "quick" patches applied in parallel. But I'd have to blame myself if I don't participate with PRs :)

Likewise

(Just as a side note, my use case is that I would like to source my .bash_profile whenever I switch to another user with sshkit's sudo su. Currently there is no way for me to do that.)

I sincerely hope it never comes to that. Relying on system config that's outside of your (direct) control is fragile.

[leehambley]

If someone patches this to work properly (it's clearly a bug) then surely the original problem is solved? You'll still have your dotfile issue, but that's best solved by using the environment mechanisms in SSHKit to code your environment into the script you are running.

[mmzoo]

Hi! Thanks for your inspiration. As for this bug, yes, a proper pull request will solve this :)

As for my use case, let me try to clarify. Because I don't understand your reasoning (yet) for not relying on "system config that's outside of your (direct) control".

Let's say our server has an app and, much like Heroku, the app is configured entirely through environment variables. Also not uncommon, these variables exist in the .bash_profile of the corresponding user (we have one user per app). All configuration is done using chef. The apps are served using passenger, which also requires the app config environment variables to be in bash_profile.

Now, I don't see a way to use the ssh --interactive flag (probably there's a reason, too ;) and I need access to those environment variables during the capistrano ssh session to that I can boot my application(s).

Are you suggesting that there is a way, in a non-interactive shell, to get all those server environment variables into my session without me having to specify them explicitly on the local machine that executes capistrano?

If so, please enlighten me :)

PS: Note that there was a update addon on my previous comment. Wouldn't it solve many problems to not rely on sudo in the first place? Just curious.

Thank you for your time. You guys are heroes after all :)

[leehambley]

Are you suggesting that there is a way, in a non-interactive shell, to get all those server environment variables into my session without me having to specify them explicitly on the local machine that executes capistrano?

Of course: source (http://www.tldp.org/HOWTO/Bash-Prompt-HOWTO/x237.html) (aliased as . in some older shells), or EXPORT (http://ss64.com/bash/export.html), or chpst -e (http://smarden.org/runit/chpst.8.html),

The Heroku 12FactorApp manifesto is a little extreme, and moreover they're referring to using things like /etc/defaults when writing your own init.d script (read more: http://www.debian.org/doc/debian-policy/ch-opersys.html)

The shell dot files are a file designed to contain helpers for making your interactive shell experience more comfortable. When your application is started by runit, upstart, monit, init.d, god, bluepill and friends, there is no shell, it is not an interactive environment and importantly there are few, or no environment variables (try it by writing a stub init.d file, and having it run env and pipe it out to a file for your to read) available. This is by design.

Thus the reasoning is clear, if two people have different .bashrc files, they're rarely in version control, are considered to be "user space" (i.e free for your user accounts to modify) then your app will behave differently/break depending on who starts it.

That's why we script the starting of services, and they always come up in a clean environment, and load their environment, depending on the tooling, from the place that that tool defines that application variables should be stored.

[leehambley]

Comment on #13, this isn't a proposal to "use a different ssh session" (for that use a different on() block, with a overwritten username), but rather an issue of checking if sudo is on the system before expecting it to work.

[mmzoo]

Hi! Thanks again for your time. I still believe I don't fully understand.

Whether my file with the environment variable exports is called /etc/default or ~/.bash_profile or /mnt/variables/my_user, the problem remains that I need to source it before I run a command on the remote server.

In this command example, constructed by sshkit:

sudo su my_user -c "/usr/bin/env whoami"

...how would you use source (which you gave as an example) so that the variables are known to whoami? It seems to me it would have too look something like this:

sudo su my_user -c "source /file/with/environment/variable/exports; /usr/bin/env whoami"

But how would I hook that source command in using the configuration techniques sshkit provides?

As for EXPORT, the command is compiled on my local machine, I simply don't have access to the values of the env variables, so I cannot add them one by one from my local machine.

In our upstart scripts we define every environment variable explicitly using the env statement. We can do that because, being located on the remote server, we know these variables when we run chef. Even if we would not have direct access to those env variables, we would simply use source /some/file; whoami to load them in the exec statement of upstart so that whoami would know about them. I'm trying to achieve just that using sshkit.

The /etc/default manual you refer to says the same thing:

Instead, they should be placed in a file in /etc/default, ... This extra file --> should be sourced by the script <-- when the script runs. It must contain only variable settings

So, I believe we talk about the same thing, but maybe just misunderstand what the other means :)

As for #13, thanks, I got it :)

[leehambley]

The point is that you shouldn't start your daemons via SSHkit (or Capistrano, etc). They should be started by something that controls their environment.

To cut a long story short; (§10.4.3 The Art of Unix Programming, Raymond, 2003)

When to Use Environment Variables

What both user and system environment variables have in common is that it would be annoying to have to replicate the information they contain in a large number of application run-control files, and extremely annoying to have to change that information everywhere when your preference changes.

Typically, the user sets these variables in his or her shell session startup file.

So if your app doesn't do the right thing with no env vars, it's incorrect, as far as UNIX conventions go (I heard some guys at Heroku who wrote a manifesto disagree ;-)) if your env vars aren't in the place the system expects them, you are doing something wrong too.

It's a matter of taste, I prefer to make sure that my programs don't accidentally misbehave. And in part, you are right, it doesn't matter whether you source ~/.bash_env or /usr/local/myapp/production.env (which also isn't a thing), but it's a matter of using the files for their intended purpose.

I'd consider adding a source() option or wrapper to SSHKit, but in the end your problem is "can't map anything with strings in it", which isn't solvable.

[mmzoo]

Ok, I feel we are talking about the same thing now :)

I'm not starting any deamons and completely agree with that capistrano should not do such weird things.

All I need to do is to run a simple rake db:migrate and Rails started supporting the DATABASE_URL variable for database configuration as early as two years ago.

I don't see how this cannot be an extremely common capistrano use case :) If not the use case, haha.

[leehambley]

That's why there's with() in the API to export environmental variables. If you REALLY wanted to you could read a file with capture() and re-export them with with(). I'm not saying it's sane, but it's only choice you have really!

What's wrong with ./config/database.yml, exactly? (Rails is hardly a good example to hold up of the correct way to do things ;-))

[mmzoo]

Thank you for your feedback. I feel less dumb now :)

Actually, it is insane to capture() the variables haha. I will opt for monkey patching with() for now to add the source command. It's not ideal but it won't bite me back so fast. Better than no working cap at all ;)

What is wrong with yaml files is that there is no way to maintain them in your local development environment when you share the repo with developer colleagues (since all those files will be in the .gitignore). Here you really want a no-config application that you can just boot (I don't want all my colleagues and external consultants to ask "why is the app not booting on my computer?" all the time).

Note that this does not only apply to database.yml, but to dozens of configuration options, secret keys, URL endpoints, etc. It would be (and indeed it was) painful to maintain yaml files without them being part of the git repository. Even if you could get around that problem by using sane defaults in development, it is painful to maintain the creation of so many yaml files in different places and also write the parsers for them that fetch the options, etc. Well, at least I feel that I'm not the only one thinking about which way would be best to solve this problem :)

And yes, I feel that Rails is a good example to hold up the correct way to do things ;)

Again, thank you for your inspiration. I am a big fan of the capistrano 3 changes. 👍

[leehambley]

Even if you could get around that problem by using sane defaults in development, it is painful to maintain the creation of so many yaml files in different places and also write the parsers for them that fetch the options, etc. Well, at least I feel that I'm not the only one thinking about which way would be best to solve this problem :)

I'm pretty sure that chef and puppet (or ansible, or salt, or etcd) are all the ways this should be handled correctly, env vars scale to medium sized apps, but not to real hardcore stuff.

[mtrudel]

@leehambley above you say

This is a matter of there being no correct way to do shell escaping if you go ahead and do some escaping yourself, you can pass options to execute() to force the command to be mapped, if you know what you are doing. (Check the source)

yet I can't seem to find any evidence of this in sshkit/lib/sshkit/command.rb. Am I looking in the wrong place?

[leehambley]

Yet I can't seem to find any evidence of this in sshkit/lib/sshkit/command.rb. Am I looking in the wrong place?

Indeed, apparently we removed it. There used to be an option passable as part of the hash to execute() which would be used in SSHKit::Command#should_map? in addition to the whitespace check.

[leafac]

I can confirm that this issue still happens on master and that #144 solves it.

Maybe we could accept it, as it less of a hack than my solution, and close this issue.

[mtrudel]

@leafac oh, mine's the same amount of hack, don't worry 😄

[leafac]

❤️

[mtrudel]

@leafac can you confirm that this solves your issue? What ended up being merged for this was a slightly different approach than when you tested above.

[leafac]

Yep, all is fine! Thanks, @mtrudel