From 4e6992f985ebfb6e6c3fb4a6fa7a2959d84ca243 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabinsk@redhat.com>
Date: Thu, 22 Jun 2017 15:30:41 +0200
Subject: [PATCH] smart card advises: use a wrapper around Bash `for` loops

Replace the raw `command` calls constructing the for loops in some
methods by a wrapper hiding this detail.

https://pagure.io/freeipa/issue/7036

Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipaserver/advise/base.py                    | 23 +++++++++++++++++++++++
 ipaserver/advise/plugins/smart_card_auth.py | 26 +++++++-------------------
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
index 581478fb75bc4f50b6bffe2e4cf9b51de46fa095..be7274417042fca521039b56af60831563f6952b 100644
--- a/ipaserver/advise/base.py
+++ b/ipaserver/advise/base.py
@@ -201,6 +201,24 @@ class UnbranchedIfStatement(IfBranch):
         self.advice_output.command('fi')
 
 
+class ForLoop(CompoundStatement):
+    """
+    Wrapper around the for loop
+    """
+    def __init__(self, advice_output, loop_variable, iterable):
+        super(ForLoop, self).__init__(advice_output)
+        self.loop_variable = loop_variable
+        self.iterable = iterable
+
+    def begin_statement(self):
+        self.advice_output.command(
+            'for {} in {}'.format(self.loop_variable, self.iterable))
+        self.advice_output.command('do')
+
+    def end_statement(self):
+        self.advice_output.command('done')
+
+
 class _AdviceOutput(object):
 
     def __init__(self):
@@ -334,6 +352,11 @@ class _AdviceOutput(object):
         with self._compound_statement(ElseIfBranch, predicate):
             yield
 
+    @contextmanager
+    def for_loop(self, loop_variable, iterable):
+        with self._compound_statement(ForLoop, loop_variable, iterable):
+            yield
+
 
 class Advice(Plugin):
     """
diff --git a/ipaserver/advise/plugins/smart_card_auth.py b/ipaserver/advise/plugins/smart_card_auth.py
index 2dc9ddb25ce41a8c85aab827a92a1143784d9457..3ff94be1e8b108668989602b1b406a39d23ff501 100644
--- a/ipaserver/advise/plugins/smart_card_auth.py
+++ b/ipaserver/advise/plugins/smart_card_auth.py
@@ -40,48 +40,36 @@ class common_smart_card_auth_config(Advice):
             ['You need to provide one or more paths to the PEM files '
              'containing CAs signing the Smart Cards']
         )
-        self.log.command(
-            "for {} in ${}".format(
-                single_ca_path_variable, ca_paths_variable))
-        self.log.command("do")
-        with self.log.indented_block():
+        with self.log.for_loop(single_ca_path_variable,
+                               '${}'.format(ca_paths_variable)):
             self.log.exit_on_predicate(
                 '[ ! -f "${}" ]'.format(single_ca_path_variable),
                 ['Invalid CA certificate filename: ${}'.format(
                     single_ca_path_variable),
                  'Please check that the path exists and is a valid file']
             )
-        self.log.command("done")
 
     def upload_smartcard_ca_certificates_to_systemwide_db(self):
-        self.log.command(
-            "for {} in ${}".format(
+        with self.log.for_loop(
                 self.single_ca_cert_variable_name,
-                self.smart_card_ca_certs_variable_name))
-        self.log.command("do")
-        with self.log.indented_block():
+                '${}'.format(self.smart_card_ca_certs_variable_name)):
             self.log.command(
                 'certutil -d {} -A -i ${} -n "Smart Card CA $(uuidgen)" '
                 '-t CT,C,C'.format(
                     self.systemwide_nssdb, self.single_ca_cert_variable_name
-                ),
+                )
             )
-        self.log.command("done")
 
     def install_smart_card_signing_ca_certs(self):
-        self.log.command(
-            "for {} in ${}".format(
+        with self.log.for_loop(
                 self.single_ca_cert_variable_name,
-                self.smart_card_ca_certs_variable_name))
-        self.log.command("do")
-        with self.log.indented_block():
+                '${}'.format(self.smart_card_ca_certs_variable_name)):
             self.log.exit_on_failed_command(
                 'ipa-cacert-manage install ${} -t CT,C,C'.format(
                     self.single_ca_cert_variable_name
                 ),
                 ['Failed to install external CA certificate to IPA']
             )
-        self.log.command("done")
 
     def update_ipa_ca_certificate_store(self):
         self.log.exit_on_failed_command(
-- 
2.9.4