Simone Tiraboschi has uploaded a new change for review. Change subject: packaging: setup: fixing iptables review dialog ......................................................................
packaging: setup: fixing iptables review dialog Writing the will to review iptables changes into answer file, fixing a bug Change-Id: I8f16a90a074052226882d76fc80918ed276d379c Bug-url: https://bugzilla.redhat.com/1109326 Signed-off-by: Simone Tiraboschi <stira...@redhat.com> --- M packaging/setup/ovirt_engine_setup/constants.py M packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py M packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py 3 files changed, 104 insertions(+), 49 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/11/34011/1 diff --git a/packaging/setup/ovirt_engine_setup/constants.py b/packaging/setup/ovirt_engine_setup/constants.py index e7bff1e..47d6efb 100644 --- a/packaging/setup/ovirt_engine_setup/constants.py +++ b/packaging/setup/ovirt_engine_setup/constants.py @@ -385,7 +385,14 @@ return 'OVESETUP_CONFIG/updateFirewall' FIREWALL_MANAGERS = 'OVESETUP_CONFIG/firewallManagers' - SKIP_FIREWALL_REVIEW = 'OVESETUP_CONFIG/skipFirewallReview' + + @osetupattrs( + answerfile=True, + summary=False, + ) + def FIREWALL_CHANGES_REVIEW(self): + return 'OVESETUP_CONFIG/firewallChangesReview' + VALID_FIREWALL_MANAGERS = 'OVESETUP_CONFIG/validFirewallManagers' FQDN_REVERSE_VALIDATION = 'OVESETUP_CONFIG/fqdnReverseValidation' FQDN_NON_LOOPBACK_VALIDATION = 'OVESETUP_CONFIG/fqdnNonLoopback' diff --git a/packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py b/packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py index 22cb72e..9a53997 100644 --- a/packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py +++ b/packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py @@ -22,6 +22,7 @@ import difflib import gettext +import os _ = lambda m: gettext.dgettext(message=m, domain='ovirt-engine-setup') @@ -127,44 +128,97 @@ ) def review_config(self): - diffl = '' - with open( - osetupcons.FileLocations.SYSCONFIG_IPTABLES, - 'r' - ) as current: - diff = difflib.unified_diff( - current.readlines(), - self._get_rules().splitlines(True), - fromfile=_('current'), - tofile=_('proposed'), - ) - diffl = ''.join(diff) - if len(diffl) > 0: - confirmed = dialog.queryBoolean( - dialog=self.plugin.dialog, - name='OVESETUP_CONFIRM_IPTABLES_CHANGES', - note=_( - 'Generated iptables rules are different ' - 'from current ones.\n' - 'Please review the changes:\n\n' - '{diff}\n\n' - 'Do you want to proceed with firewall configuration? ' - '(@VALUES@) [@DEFAULT@]: ' - ).format( - diff=diffl - ), - prompt=True, - true=_('Yes'), - false=_('No'), - default=True, - ) - if not confirmed: - raise RuntimeError( + diff_lines = '' + current_rules = '' + current_modified_since_prev_setup = False + interactive = True + if os.path.isfile(osetupcons.FileLocations.SYSCONFIG_IPTABLES): + with open( + osetupcons.FileLocations.SYSCONFIG_IPTABLES, + 'r' + ) as current: + current_rules = current.read().splitlines() + if os.path.isfile(osetupcons.FileLocations.OVIRT_IPTABLES_EXAMPLE): + with open( + osetupcons.FileLocations.OVIRT_IPTABLES_EXAMPLE, + 'r' + ) as prev_setup_example: + prev_setup_rules = prev_setup_example.read().splitlines() + diff_prev_cur = difflib.unified_diff( + current_rules, + prev_setup_rules, + lineterm='', + ) + diff_lines = '\n'.join(diff_prev_cur) + if len(diff_lines) > 0: + current_modified_since_prev_setup = True + diff = difflib.unified_diff( + current_rules, + self._get_rules().splitlines(), + lineterm='', + fromfile=_('current'), + tofile=_('proposed'), + ) + diff_lines = '\n'.join(diff) + if len(diff_lines) > 0: + if current_modified_since_prev_setup: + self.logger.warning( _( - 'iptables proposed configuration ' - 'was rejected by user' + "It seams that previously generated iptables " + "configuration was manually edited,\n" + "please carefully review the proposed " + "configuration" ) ) + if self.environment[ + osetupcons.ConfigEnv.FIREWALL_CHANGES_REVIEW + ] is None: + self.environment[ + osetupcons.ConfigEnv.FIREWALL_CHANGES_REVIEW + ] = dialog.queryBoolean( + dialog=self.plugin.dialog, + name='OVESETUP_REVIEW_IPTABLES_CHANGES', + note=_( + 'Generated iptables rules are different ' + 'from current ones.\n' + 'Do you want to review them? ' + '(@VALUES@) [@DEFAULT@]: ' + ), + prompt=True, + true=_('Yes'), + false=_('No'), + default=False, + ) + else: + interactive = False + + if self.environment[ + osetupcons.ConfigEnv.FIREWALL_CHANGES_REVIEW + ] and interactive: + confirmed = dialog.queryBoolean( + dialog=self.plugin.dialog, + name='OVESETUP_CONFIRM_IPTABLES_CHANGES', + note=_( + 'Please review the changes:\n\n' + '{diff}\n\n' + 'Do you want to proceed with firewall ' + 'configuration? ' + '(@VALUES@) [@DEFAULT@]: ' + ).format( + diff=diff_lines + ), + prompt=True, + true=_('Yes'), + false=_('No'), + default=True, + ) + if not confirmed: + raise RuntimeError( + _( + 'iptables proposed configuration ' + 'was rejected by user' + ) + ) @plugin.event( stage=plugin.Stages.STAGE_SETUP, diff --git a/packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py b/packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py index b2ba4e6..3b4fe8a 100644 --- a/packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py +++ b/packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py @@ -43,6 +43,7 @@ super(Plugin, self).__init__(context=context) self._detected_managers = [] self._available_managers = [] + self._selected_manager = None @plugin.event( stage=plugin.Stages.STAGE_INIT, @@ -57,8 +58,8 @@ None ) self.environment.setdefault( - osetupcons.ConfigEnv.SKIP_FIREWALL_REVIEW, - False + osetupcons.ConfigEnv.FIREWALL_CHANGES_REVIEW, + None ) self.environment.setdefault( osetupcons.ConfigEnv.VALID_FIREWALL_MANAGERS, @@ -220,12 +221,13 @@ ], ), ) - next( + self._selected_manager = [ m for m in self._available_managers if m.name == self.environment[ osetupcons.ConfigEnv.FIREWALL_MANAGER ] - ).enable() + ][0] + self._selected_manager.enable() @plugin.event( stage=plugin.Stages.STAGE_VALIDATION, @@ -238,15 +240,7 @@ ), ) def _review_config(self): - if not self.environment[ - osetupcons.ConfigEnv.SKIP_FIREWALL_REVIEW - ]: - next( - m for m in self._available_managers - if m.name == self.environment[ - osetupcons.ConfigEnv.FIREWALL_MANAGER - ] - ).review_config() + self._selected_manager.review_config() @plugin.event( stage=plugin.Stages.STAGE_MISC, -- To view, visit http://gerrit.ovirt.org/34011 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8f16a90a074052226882d76fc80918ed276d379c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Simone Tiraboschi <stira...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
