You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@brooklyn.apache.org by ygy <gi...@git.apache.org> on 2015/11/26 10:41:19 UTC

[GitHub] incubator-brooklyn pull request: Fix SoftwareProcess.dontRequireTt...

GitHub user ygy opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/1071

    Fix SoftwareProcess.dontRequireTtyForSudo

    - it doesn't directly edit /etc/sudoers
    - uses visudo to verify the prepared file
    - added integration test
    - excluded test sudoers file from rat plugin checks

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ygy/incubator-brooklyn fix/sudo-tty

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/1071.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1071
    
----
commit 321e9c34bd0342ef3dd6dcdfe284832d4e4dc8f9
Author: Yavor Yanchev <ya...@yanchev.com>
Date:   2015-11-26T09:30:58Z

    Fix SoftwareProcess.dontRequireTtyForSudo
    
    - it doesn't directly edit /etc/sudoers
    - uses visudo to verify the prepared file
    - added integration test
    - excluded test sudoers file from rat plugin checks

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: SoftwareProcess.dontRequireTtyFor...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/1071


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: SoftwareProcess.dontRequireTtyFor...

Posted by ahgittin <gi...@git.apache.org>.

Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1071#issuecomment-160097198
  
    looks good.  is there a situation where in-place `sed` doesn't work?  or is the `visudo` only used to give fail-fast if we edit wrongly?  worth a comment in code to address.
    
    (btw you comment that you exclude test sudoers file from rat but i don't see that; it looks like it has the appropriate license header which is actually better.)
    
    nice tests.  good enough to merge, so merging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: SoftwareProcess.dontRequireTtyFor...

Posted by aledsage <gi...@git.apache.org>.

Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1071#issuecomment-161432178
  
    @ahgittin problems were reported when using `BashCommands.dontRequireTtyForSudo()` on a RHEL environment - the sed command had left `/etc/sudoers` wrong, so it was then impossible for anyone to sudo - i.e. we'd entirely broken the VM.
    
    Unfortunately I'm not sure what the original `/etc/sudoers` file looked like, such that our `sed` command would break it.
    
    But it highlighted that we were following *very* bad practices by modifying the file directly; switching to `visudo` is definitely the right thing to do.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---