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

Only run findProcess where required in order to avoid prompting for super user login #149

Merged
merged 2 commits into from
Nov 11, 2018

Conversation

mzedeler
Copy link
Contributor

@mzedeler mzedeler commented Nov 8, 2018

Summary

Avoid prompting for super user password when possible.

Test plan

TBD

@mzedeler
Copy link
Contributor Author

mzedeler commented Nov 8, 2018

I really have an issue with tools that suddenly prompt for a password. This is a perfect attack vector for hackers. I think it would be a good idea to ensure that nobody is ever prompted for password.

@gregberge
Copy link
Member

It looks good but tests are not passing, I will try it when I have time.

@gregberge
Copy link
Member

Thanks for your work!

@mzedeler
Copy link
Contributor Author

mzedeler commented Nov 8, 2018

I'll fix the tests tomorrow.

@mzedeler
Copy link
Contributor Author

mzedeler commented Nov 9, 2018

I've updated the code now. The tests should run and I've improved the handling of the case where the ports are in use, so calling findProcess is done as late as possible.

@mzedeler
Copy link
Contributor Author

mzedeler commented Nov 9, 2018

I've run some manual testing to verify that the different configuration options work, but it would be a good idea to add tests for them.

@gregberge gregberge merged commit 1701e9b into argos-ci:master Nov 11, 2018
@gregberge
Copy link
Member

@mzedeler thanks! Yes but it is very difficult to test it 😔

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.

2 participants