483b06
From 243f7ebf6a07fa54cbd5db6bf7f67fee04e4f14c Mon Sep 17 00:00:00 2001
483b06
From: Martin Babinsky <mbabinsk@redhat.com>
483b06
Date: Thu, 22 Jun 2017 13:20:05 +0200
483b06
Subject: [PATCH] delegate the indentation handling in advises to dedicated
483b06
 class
483b06
483b06
Indentation levels are now handled transparently by a dedicated class
483b06
and should not pollute the statement printing logic.
483b06
483b06
https://pagure.io/freeipa/issue/7036
483b06
483b06
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
483b06
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
483b06
---
483b06
 ipaserver/advise/base.py                    | 106 +++++++++++++++++++---------
483b06
 ipaserver/advise/plugins/smart_card_auth.py |  45 ++++++------
483b06
 2 files changed, 93 insertions(+), 58 deletions(-)
483b06
483b06
diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
483b06
index 639fd1807f72f11f46136999c4ce4c6eec6b3698..c320b002c83198cbb0fd73a5c158df07dd309242 100644
483b06
--- a/ipaserver/advise/base.py
483b06
+++ b/ipaserver/advise/base.py
483b06
@@ -19,6 +19,7 @@
483b06
 
483b06
 from __future__ import print_function
483b06
 
483b06
+from contextlib import contextmanager
483b06
 import os
483b06
 from textwrap import wrap
483b06
 
483b06
@@ -75,6 +76,8 @@ As a result, you can redirect the advice's output directly to a script file.
483b06
 # ./script.sh
483b06
 """
483b06
 
483b06
+DEFAULT_INDENTATION_INCREMENT = 2
483b06
+
483b06
 
483b06
 class _IndentationTracker(object):
483b06
     """
483b06
@@ -131,39 +134,77 @@ class _AdviceOutput(object):
483b06
         self.content = []
483b06
         self.prefix = '# '
483b06
         self.options = None
483b06
+        self._indentation_tracker = _IndentationTracker(
483b06
+            spaces_per_indent=DEFAULT_INDENTATION_INCREMENT)
483b06
+
483b06
+    def indent(self):
483b06
+        """
483b06
+        Indent the statements by one level
483b06
+        """
483b06
+        self._indentation_tracker.indent()
483b06
+
483b06
+    def dedent(self):
483b06
+        """
483b06
+        Dedent the statements by one level
483b06
+        """
483b06
+        self._indentation_tracker.dedent()
483b06
+
483b06
+    @contextmanager
483b06
+    def indented_block(self):
483b06
+        self.indent()
483b06
+        try:
483b06
+            yield
483b06
+        finally:
483b06
+            self.dedent()
483b06
 
483b06
     def comment(self, line, wrapped=True):
483b06
         if wrapped:
483b06
-            for wrapped_line in wrap(line, 70):
483b06
-                self.content.append(self.prefix + wrapped_line)
483b06
+            self.append_wrapped_and_indented_comment(line)
483b06
         else:
483b06
-            self.content.append(self.prefix + line)
483b06
+            self.append_comment(line)
483b06
+
483b06
+    def append_wrapped_and_indented_comment(self, line, character_limit=70):
483b06
+        """
483b06
+        append wrapped and indented comment to the output
483b06
+        """
483b06
+        for wrapped_indented_line in wrap(
483b06
+                self.indent_statement(line), character_limit):
483b06
+            self.append_comment(wrapped_indented_line)
483b06
+
483b06
+    def append_comment(self, line):
483b06
+        self.append_statement(self.prefix + line)
483b06
+
483b06
+    def append_statement(self, statement):
483b06
+        """
483b06
+        Append a line to the generated content indenting it by tracked number
483b06
+        of spaces
483b06
+        """
483b06
+        self.content.append(self.indent_statement(statement))
483b06
+
483b06
+    def indent_statement(self, statement):
483b06
+        return '{indent}{statement}'.format(
483b06
+            indent=self._indentation_tracker.indentation_string,
483b06
+            statement=statement)
483b06
 
483b06
     def debug(self, line):
483b06
         if self.options.verbose:
483b06
             self.comment('DEBUG: ' + line)
483b06
 
483b06
-    def command(self, line, indent_spaces=0):
483b06
-        self.content.append(
483b06
-            '{}{}'.format(self._format_indent(indent_spaces), line))
483b06
-
483b06
-    def _format_indent(self, num_spaces):
483b06
-        return ' ' * num_spaces
483b06
+    def command(self, line):
483b06
+        self.append_statement(line)
483b06
 
483b06
-    def echo_error(self, error_message, indent_spaces=0):
483b06
-        self.command(
483b06
-            self._format_error(error_message), indent_spaces=indent_spaces)
483b06
+    def echo_error(self, error_message):
483b06
+        self.command(self._format_error(error_message))
483b06
 
483b06
     def _format_error(self, error_message):
483b06
         return 'echo "{}" >&2'.format(error_message)
483b06
 
483b06
     def exit_on_failed_command(self, command_to_run,
483b06
-                               error_message_lines, indent_spaces=0):
483b06
-        self.command(command_to_run, indent_spaces=indent_spaces)
483b06
+                               error_message_lines):
483b06
+        self.command(command_to_run)
483b06
         self.exit_on_predicate(
483b06
             '[ "$?" -ne "0" ]',
483b06
-            error_message_lines,
483b06
-            indent_spaces=indent_spaces)
483b06
+            error_message_lines)
483b06
 
483b06
     def exit_on_nonroot_euid(self):
483b06
         self.exit_on_predicate(
483b06
@@ -171,8 +212,7 @@ class _AdviceOutput(object):
483b06
             ["This script has to be run as root user"]
483b06
         )
483b06
 
483b06
-    def exit_on_predicate(self, predicate, error_message_lines,
483b06
-                          indent_spaces=0):
483b06
+    def exit_on_predicate(self, predicate, error_message_lines):
483b06
         commands_to_run = [
483b06
             self._format_error(error_message_line)
483b06
             for error_message_line in error_message_lines]
483b06
@@ -180,30 +220,26 @@ class _AdviceOutput(object):
483b06
         commands_to_run.append('exit 1')
483b06
         self.commands_on_predicate(
483b06
             predicate,
483b06
-            commands_to_run,
483b06
-            indent_spaces=indent_spaces)
483b06
+            commands_to_run)
483b06
 
483b06
     def commands_on_predicate(self, predicate, commands_to_run_when_true,
483b06
-                              commands_to_run_when_false=None,
483b06
-                              indent_spaces=0):
483b06
+                              commands_to_run_when_false=None):
483b06
         if_command = 'if {}'.format(predicate)
483b06
-        self.command(if_command, indent_spaces=indent_spaces)
483b06
-        self.command('then', indent_spaces=indent_spaces)
483b06
-
483b06
-        indented_block_spaces = indent_spaces + 2
483b06
+        self.command(if_command)
483b06
+        self.command('then')
483b06
 
483b06
-        for command_to_run_when_true in commands_to_run_when_true:
483b06
-            self.command(
483b06
-                command_to_run_when_true, indent_spaces=indented_block_spaces)
483b06
+        with self.indented_block():
483b06
+            for command_to_run_when_true in commands_to_run_when_true:
483b06
+                self.command(
483b06
+                    command_to_run_when_true)
483b06
 
483b06
         if commands_to_run_when_false is not None:
483b06
-            self.command("else", indent_spaces=indent_spaces)
483b06
-            for command_to_run_when_false in commands_to_run_when_false:
483b06
-                self.command(
483b06
-                    command_to_run_when_false,
483b06
-                    indent_spaces=indented_block_spaces)
483b06
+            self.command("else")
483b06
+            with self.indented_block():
483b06
+                for command_to_run_when_false in commands_to_run_when_false:
483b06
+                    self.command(command_to_run_when_false)
483b06
 
483b06
-        self.command('fi', indent_spaces=indent_spaces)
483b06
+        self.command('fi')
483b06
 
483b06
 
483b06
 class Advice(Plugin):
483b06
diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
483b06
index 16c01204444883ed949db73b2314ba5c404124df..75efa6f854acd5f746111ea44957a538117381ae 100644
483b06
--- a/ipaserver/advise/plugins/smart_card_auth.py
483b06
+++ b/ipaserver/advise/plugins/smart_card_auth.py
483b06
@@ -44,13 +44,13 @@ class common_smart_card_auth_config(Advice):
483b06
             "for {} in ${}".format(
483b06
                 single_ca_path_variable, ca_paths_variable))
483b06
         self.log.command("do")
483b06
-        self.log.exit_on_predicate(
483b06
-            '[ ! -f "${}" ]'.format(single_ca_path_variable),
483b06
-            ['Invalid CA certificate filename: ${}'.format(
483b06
-                single_ca_path_variable),
483b06
-             'Please check that the path exists and is a valid file'],
483b06
-            indent_spaces=2
483b06
-        )
483b06
+        with self.log.indented_block():
483b06
+            self.log.exit_on_predicate(
483b06
+                '[ ! -f "${}" ]'.format(single_ca_path_variable),
483b06
+                ['Invalid CA certificate filename: ${}'.format(
483b06
+                    single_ca_path_variable),
483b06
+                 'Please check that the path exists and is a valid file']
483b06
+            )
483b06
         self.log.command("done")
483b06
 
483b06
     def upload_smartcard_ca_certificates_to_systemwide_db(self):
483b06
@@ -59,13 +59,13 @@ class common_smart_card_auth_config(Advice):
483b06
                 self.single_ca_cert_variable_name,
483b06
                 self.smart_card_ca_certs_variable_name))
483b06
         self.log.command("do")
483b06
-        self.log.command(
483b06
-            'certutil -d {} -A -i ${} -n "Smart Card CA $(uuidgen)" '
483b06
-            '-t CT,C,C'.format(
483b06
-                self.systemwide_nssdb, self.single_ca_cert_variable_name
483b06
-            ),
483b06
-            indent_spaces=2
483b06
-        )
483b06
+        with self.log.indented_block():
483b06
+            self.log.command(
483b06
+                'certutil -d {} -A -i ${} -n "Smart Card CA $(uuidgen)" '
483b06
+                '-t CT,C,C'.format(
483b06
+                    self.systemwide_nssdb, self.single_ca_cert_variable_name
483b06
+                ),
483b06
+            )
483b06
         self.log.command("done")
483b06
 
483b06
     def install_smart_card_signing_ca_certs(self):
483b06
@@ -74,13 +74,13 @@ class common_smart_card_auth_config(Advice):
483b06
                 self.single_ca_cert_variable_name,
483b06
                 self.smart_card_ca_certs_variable_name))
483b06
         self.log.command("do")
483b06
-        self.log.exit_on_failed_command(
483b06
-            'ipa-cacert-manage install ${} -t CT,C,C'.format(
483b06
-                self.single_ca_cert_variable_name
483b06
-            ),
483b06
-            ['Failed to install external CA certificate to IPA'],
483b06
-            indent_spaces=2
483b06
-        )
483b06
+        with self.log.indented_block():
483b06
+            self.log.exit_on_failed_command(
483b06
+                'ipa-cacert-manage install ${} -t CT,C,C'.format(
483b06
+                    self.single_ca_cert_variable_name
483b06
+                ),
483b06
+                ['Failed to install external CA certificate to IPA']
483b06
+            )
483b06
         self.log.command("done")
483b06
 
483b06
     def update_ipa_ca_certificate_store(self):
483b06
@@ -221,8 +221,7 @@ class config_server_for_smart_card_auth(common_smart_card_auth_config):
483b06
         self.log.command('else')
483b06
         self.log.exit_on_failed_command(
483b06
             'ipa-pkinit-manage enable',
483b06
-            ['Failed to issue PKINIT certificates to local KDC'],
483b06
-            indent_spaces=2)
483b06
+            ['Failed to issue PKINIT certificates to local KDC'])
483b06
         self.log.command('fi')
483b06
 
483b06
     def enable_ok_to_auth_as_delegate_on_http_principal(self):
483b06
-- 
483b06
2.9.4
483b06