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

add Connection:update_engine #227

Merged
merged 3 commits into from
Oct 24, 2016
Merged

Conversation

nbirnel
Copy link
Contributor

@nbirnel nbirnel commented May 4, 2016

Allows updating of a single engine.

@nbirnel
Copy link
Contributor Author

nbirnel commented May 5, 2016

I should have started a feature-request issue before I submitted this! Let me share my use case:

My engines are in my customer's hands, so I cannot ensure that they are on during updates. Thus, sometimes an engine needs a one-off update. "update now" from the screen session does not work if the customer has blocked port 80 out.

I can automate detecting that an engine is out of date. I can automate a repair install. But I would like to be able to automate a single engine update from the console. This would save me several hours per week.

Please let me know if there is any way I can help make this easier to merge. I have this monkey-patched in now, but I would rather have it right in the gem.

@sgreen-r7
Copy link
Contributor

Thanks for the background info @nbirnel we were having a full week, so sorry for not responding sooner.

I would like to do a little testing around this before we pull it into master. It is a good idea, but I would like to make it a bit more focused on the negative test case scenario. For example, rescuing the whole Nexpose object is not a standard that I would really like to promote.

I will put some work into this tomorrow (5/6) and see what we can work out.

@nbirnel
Copy link
Contributor Author

nbirnel commented May 10, 2016

@sgreen-r7 On reconsidering, I'd like to pull out the begin rescue end block entirely. Catching exceptions on that should be the user's problem. (I have use cases where I would like to catch exceptions, and others where I am happy to have it bomb out.) If this sounds reasonable to you, I will change it, and add a warning about exceptions to the doc string.

# or if the engine is offline or unresponsive.
#
# @param [Fixnum] engine_id Unique ID of the engine.
# @return [Boolean] true if the update was sent

Choose a reason for hiding this comment

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

Trailing whitespace detected.

@sgreen-r7
Copy link
Contributor

@nbirnel that doesn't sound like a bad idea, however what I'm still trying to work out is how we return to the user if the update passed or failed. I think that's what it's missing, let's see if we can find a good way to show that to a user -- if not, then i think what you have should be okay.

@nbirnel
Copy link
Contributor Author

nbirnel commented May 10, 2016

@sgreen-r7 That is a tough nut to crack. One way out of it is to rename the function "send_update_to_engine" so it is explicit that this only sends an update; we succeed if it was sent and received; what the engine does with it is out of scope.

@sgreen-r7 sgreen-r7 merged commit 9eee3cb into rapid7:master Oct 24, 2016
@nbirnel nbirnel deleted the update-engine branch December 12, 2016 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants