Skip to content

add support for --allow-use-as-root-and-accept-consequences - #2123

Merged
boegel merged 3 commits into
easybuilders:developfrom
boegel:allow_use_as_root
Mar 24, 2017
Merged

add support for --allow-use-as-root-and-accept-consequences#2123
boegel merged 3 commits into
easybuilders:developfrom
boegel:allow_use_as_root

Conversation

@boegel

@boegel boegel commented Feb 15, 2017

Copy link
Copy Markdown
Member

cfr. #420

@boegel boegel added this to the 3.2.0 milestone Feb 15, 2017
@boegel boegel mentioned this pull request Feb 15, 2017
@boegel

boegel commented Feb 15, 2017

Copy link
Copy Markdown
Member Author

@rjeschmi please review?

Comment thread test/framework/options.py
"""Test --allow-use-as-root"""

# pretend we're running as root by monkey patching os.getuid used in main
easybuild.main.os.getuid = lambda: 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some linting tools don't like it when you assign lambda's, but this one is so trivial it doesn't require a def imho.

@JensTimmerman

Copy link
Copy Markdown

lgtm

@boegel

boegel commented Feb 16, 2017

Copy link
Copy Markdown
Member Author

@JensTimmerman Any concerns about running everything as root vs only the steps that would really require root access (cfr. your comments in #420)?

The latter would be a lot more difficult, and may not be worth the effort, since it wouldn't provide much protection against nasty things from happening during make install for example...

@rjeschmi

Copy link
Copy Markdown
Contributor

I think this is fine in general. I think another useful option is to demote from root to a particular user. I have a wrapper that does that for me so I can run easybuild as root more easily.

@JensTimmerman

Copy link
Copy Markdown

@boegel I would add -i-know-this-is-a-bad-idea to the end of the option to make it clear this is an option that actually isn't supposed to be used.

@pescobar

Copy link
Copy Markdown
Member

hahahaha +1 for @JensTimmerman suggestion ;)

@boegel

boegel commented Feb 16, 2017

Copy link
Copy Markdown
Member Author

@JensTimmerman That's what the warning is for. Making the option annoyingly long/silly is something we can do, but it won't make any difference if you set it via the environment or in a configuration file (which makes it do once and forget).

@JensTimmerman

Copy link
Copy Markdown

@boegel sure you can once and forget, but the point is that a new user can immediately identify this option as something that shouldn't be just be used lightheartedly.
The warning surely helps here, but once the warning is shown it's already to late...

@boegel

boegel commented Feb 16, 2017

Copy link
Copy Markdown
Member Author

OK, fair point @JensTimmerman...

So, pick one:

  • --allow-use-as-root-even-though-it-is-a-bad-idea
  • --allow-use-as-root-despite-being-not-recommended
  • --allow-use-as-root-and-accept-consequences

@pescobar

Copy link
Copy Markdown
Member

--allow-use-as-root-even-though-it-is-a-bad-idea-but-I-am-a-shell-cowboy

;)

@JensTimmerman

Copy link
Copy Markdown

first one

@boegel boegel changed the title add support for --allow-use-as-root add support for --allow-use-as-root-and-accept-consequences Mar 1, 2017
@boegel

boegel commented Mar 1, 2017

Copy link
Copy Markdown
Member Author

We briefly discussed this during today's EasyBuild conf call, where @pescobar mentioned that there may actually be good reasons to install something with root privileges, e.g. installing Singularity with EasyBuild (which requires enabling setuid on a binary).

It would be nicer to have support to only leverage admin privileges when really needed, i.e. only when actually enabled setuid on something, rather than doing the whole installation as root, but this would require quite a bit more work.

There also seemed to be a consensus that --allow-use-as-root-and-accept-consequences is a better option name, since it makes it clear that this configuration setting should be used with care, while not making claims that doing so is always wrong (as opposed to --allow-use-as-root-even-though-it-is-a-bad-idea). So, I renamed the option to --allow-use-as-root-and-accept-consequences.

The idea is to include this support for --allow-use-as-root-and-accept-consequences in the upcoming EasyBuild v3.2.0.

@boegel
boegel merged commit c9a9c13 into easybuilders:develop Mar 24, 2017
@boegel
boegel deleted the allow_use_as_root branch March 24, 2017 08:23
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.

4 participants