-
Notifications
You must be signed in to change notification settings - Fork 46
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
Skips running the updater if last update was good, a reboot isn't needed, and a config-specified delta isn't defined. #450
Changes from 10 commits
7a25389
d90890e
591ca58
09b7cb1
7005388
83df22c
ea7a9af
ed92761
25df3de
115a541
c719c34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0.1.3 | ||
0.1.4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,33 @@ | |
from sdw_updater_gui.UpdaterApp import UpdaterApp | ||
from sdw_util import Util | ||
from sdw_updater_gui import Updater | ||
|
||
from sdw_updater_gui.UpdaterApp import launch_securedrop_client | ||
from sdw_updater_gui.Updater import should_launch_updater | ||
import logging | ||
import sys | ||
import argparse | ||
|
||
DEFAULT_INTERVAL = 28800 | ||
|
||
|
||
def parse_argv(argv): | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument("--skip-delta", type=int) | ||
return parser.parse_args(argv) | ||
|
||
|
||
def launch_updater(): | ||
""" | ||
Start the updater GUI | ||
""" | ||
|
||
app = QtGui.QApplication(sys.argv) | ||
form = UpdaterApp() | ||
form.show() | ||
sys.exit(app.exec_()) | ||
|
||
|
||
def main(): | ||
def main(argv): | ||
sdlog = logging.getLogger(__name__) | ||
Util.configure_logging(Updater.LOG_FILE) | ||
lock_handle = Util.obtain_lock(Updater.LOCK_FILE) | ||
|
@@ -17,11 +38,24 @@ def main(): | |
# Logged. | ||
sys.exit(1) | ||
sdlog.info("Starting SecureDrop Launcher") | ||
app = QtGui.QApplication(sys.argv) | ||
form = UpdaterApp() | ||
form.show() | ||
sys.exit(app.exec_()) | ||
|
||
args = parse_argv(argv) | ||
|
||
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps setting the default in argparse will simplify the logic here: https://docs.python.org/3.7/library/argparse.html#default |
||
args.skip_delta | ||
except NameError: | ||
args.skip_delta = DEFAULT_INTERVAL | ||
|
||
if args.skip_delta is None: | ||
args.skip_delta = DEFAULT_INTERVAL | ||
|
||
interval = int(args.skip_delta) | ||
|
||
if should_launch_updater(interval): | ||
launch_updater() | ||
else: | ||
launch_securedrop_client() | ||
|
||
|
||
if __name__ == "__main__": | ||
main() | ||
main(sys.argv[1:]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -430,6 +430,65 @@ def _safely_start_vm(vm): | |
sdlog.error(str(e)) | ||
|
||
|
||
def should_launch_updater(interval): | ||
sdlog.info("Starting SecureDrop Launcher") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This results in a duplicate log line (since the same message is logged from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope, merge cruft. will delete. |
||
|
||
status = read_dom0_update_flag_from_disk(with_timestamp=True) | ||
|
||
if _valid_status(status): | ||
if _interval_expired(interval, status): | ||
sdlog.info("Update interval expired: launching updater.") | ||
return True | ||
else: | ||
if status["status"] == UpdateStatus.UPDATES_OK.value: | ||
sdlog.info("Updates OK and interval not expired, launching client.") | ||
return False | ||
elif status["status"] == UpdateStatus.REBOOT_REQUIRED.value: | ||
if last_required_reboot_performed(): | ||
sdlog.info( | ||
"Required reboot performed, updating status and launching client." | ||
) | ||
_write_updates_status_flag_to_disk(UpdateStatus.UPDATES_OK) | ||
return False | ||
else: | ||
sdlog.info("Required reboot pending, launching updater") | ||
return True | ||
else: | ||
sdlog.info( | ||
"Update status is {}, launching updater.".format( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This results in log lines like "Update status is 1, launching updater" which is of course true, but is only useful to anyone familiar with the implementation internals -- I think for states we know the updater can enter, we should have corresponding human-readable log lines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✔️ |
||
str(status["status"]) | ||
) | ||
) | ||
return True | ||
else: | ||
sdlog.info("Update status not available, launching updater.") | ||
return True | ||
|
||
|
||
def _valid_status(status): | ||
""" | ||
status should contain 2 items, the update flag and a timestamp. | ||
""" | ||
if isinstance(status, dict) and len(status) == 2: | ||
return True | ||
return False | ||
|
||
|
||
def _interval_expired(interval, status): | ||
""" | ||
Check if specified update interval has expired. | ||
""" | ||
|
||
try: | ||
update_time = datetime.strptime(status["last_status_update"], DATE_FORMAT) | ||
except ValueError: | ||
# Broken timestamp? run the updater. | ||
return True | ||
if (datetime.now() - update_time) < timedelta(seconds=interval): | ||
return False | ||
return True | ||
|
||
|
||
class UpdateStatus(Enum): | ||
""" | ||
Standardizes return codes for update/upgrade methods | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,19 @@ | |
logger = logging.getLogger(__name__) | ||
|
||
|
||
def launch_securedrop_client(): | ||
""" | ||
Helper function to launch the SecureDrop Client | ||
""" | ||
try: | ||
logger.info("Launching SecureDrop client") | ||
subprocess.Popen(["qvm-run", "sd-app", "gtk-launch securedrop-client"]) | ||
except subprocess.CalledProcessError as e: | ||
logger.error("Error while launching SecureDrop client") | ||
logger.error(str(e)) | ||
sys.exit(0) | ||
|
||
|
||
class UpdaterApp(QtGui.QMainWindow, Ui_UpdaterDialog): | ||
def __init__(self, parent=None): | ||
super(UpdaterApp, self).__init__(parent) | ||
|
@@ -19,7 +32,7 @@ def __init__(self, parent=None): | |
self.setupUi(self) | ||
self.clientOpenButton.setEnabled(False) | ||
self.clientOpenButton.hide() | ||
self.clientOpenButton.clicked.connect(self.launch_securedrop_client) | ||
self.clientOpenButton.clicked.connect(launch_securedrop_client) | ||
emkll marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.rebootButton.setEnabled(False) | ||
self.rebootButton.hide() | ||
self.rebootButton.clicked.connect(self.reboot_workstation) | ||
|
@@ -173,19 +186,6 @@ def get_vms_that_need_upgrades(self, results): | |
vms_to_upgrade.append(vm) | ||
return vms_to_upgrade | ||
|
||
def launch_securedrop_client(self): | ||
""" | ||
Helper method to launch the SecureDrop Client | ||
""" | ||
try: | ||
logger.info("Launching SecureDrop client") | ||
subprocess.Popen(["qvm-run", "sd-app", "gtk-launch securedrop-client"]) | ||
except subprocess.CalledProcessError as e: | ||
self.proposedActionDescription.setText(strings.descri) | ||
logger.error("Error while launching SecureDrop client") | ||
logger.error(str(e)) | ||
sys.exit(0) | ||
|
||
def apply_all_updates(self): | ||
""" | ||
Method used by the applyUpdatesButton that will create and start an | ||
|
@@ -249,7 +249,7 @@ def run(self): | |
# write the flags to disk | ||
run_results = Updater.overall_update_status(results) | ||
Updater._write_updates_status_flag_to_disk(run_results) | ||
if run_results == UpdateStatus.UPDATES_OK: | ||
if run_results in {UpdateStatus.UPDATES_OK, UpdateStatus.REBOOT_REQUIRED}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would recommend adding a code comment above this line, explaining the behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✔️ |
||
Updater._write_last_updated_flags_to_disk() | ||
# populate signal contents | ||
message = results # copy all the information from results | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always make a habit of annotating seconds values, i.e.
# 8 hours
in this caseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️