Repository: jboss-dockerfiles/dogen

Issue:
Better way for invoking plugins
Currently plugins are invoked via --plugin [name] command line option. it complicates stuff with requiring different command lines for project with different plugins and its unneeded.

Plugins can be invoked automatically and have its own trigger action:
* command line option
* key in image.yaml
* directory existence

With this approach we also need to document it/create plugin development guidelines

Discussion:

Patch:
diff --git a/dogen/cli.py b/dogen/cli.py
index 79eafa7..25200ef 100644
--- a/dogen/cli.py
+++ b/dogen/cli.py
@@ -43,12 +43,6 @@ class CLI(object):
 
         plugins = self.get_plugins()
 
-        for plugin in plugins:
-            key, description = plugins[plugin].info()
-            epilog += "\n  * %s:\t%s" % (key, description)
-
-        parser.epilog = epilog
-
         parser.add_argument(
             '-v', '--verbose', action='store_true', help='Verbose output')
 
@@ -60,11 +54,16 @@ class CLI(object):
         parser.add_argument('--scripts-path', help='Location of the scripts directory containing script packages.')
         parser.add_argument('--additional-script', action='append', help='Location of additional script (can be url). Can be specified multiple times.')
         parser.add_argument('--template', help='Path to custom template (can be url)')
-        parser.add_argument('--plugin', action='append', help='Plugin to be enabled. Can be specified multiple times.')
 
         parser.add_argument('path', help="Path to yaml descriptor to process")
         parser.add_argument('output', help="Path to directory where generated files should be saved")
 
+        for plugin in plugins:
+            key, description = plugins[plugin].info()
+            epilog += "\n  * %s:\t%s" % (key, description)
+            parser = plugins[plugin].inject_args(parser)
+
+        parser.epilog = epilog
         args = parser.parse_args()
 
         if args.verbose:
@@ -72,22 +71,16 @@ class CLI(object):
         else:
             self.log.setLevel(logging.INFO)
 
-        if args.skip_ssl_verification:
-            ssl_verify = False
-        else:
-            ssl_verify = None
 
         self.log.debug("Running version %s", version)
 
         enabled_plugins = []
 
-        if args.plugin:
-            for plugin in plugins:
-                if plugins[plugin].info()[0] in args.plugin:
-                    enabled_plugins.append(plugins[plugin])
+        for plugin in plugins:
+            enabled_plugins.append(plugins[plugin])
 
         try:
-            Generator(self.log, args.path, args.output, template=args.template, scripts_path=args.scripts_path, additional_scripts=args.additional_script, without_sources=args.without_sources, plugins=enabled_plugins, ssl_verify=ssl_verify).run()
+            Generator(self.log, args=args, plugins=enabled_plugins).run()
         except KeyboardInterrupt as e:
             pass
         except Error as e:
diff --git a/dogen/generator.py b/dogen/generator.py
index 0099e67..98fa73e 100644
--- a/dogen/generator.py
+++ b/dogen/generator.py
@@ -18,22 +18,25 @@ from dogen import version, DEFAULT_SCRIPT_EXEC, DEFAULT_SCRIPT_USER
 from dogen.errors import Error
 
 class Generator(object):
-    def __init__(self, log, descriptor, output, template=None, scripts_path=None, additional_scripts=None, without_sources=False, plugins=[], ssl_verify=None):
+    def __init__(self, log, args, plugins=[]):
         self.log = log
         self.pwd = os.path.realpath(os.path.dirname(os.path.realpath(__file__)))
-        self.descriptor = os.path.realpath(descriptor)
-        self.without_sources = without_sources
-        self.output = output
+        self.descriptor = os.path.realpath(args.path)
+        self.without_sources = args.without_sources
+        self.output = args.output
         self.dockerfile = os.path.join(self.output, "Dockerfile")
-        self.template = template
-        self.scripts_path = scripts_path
-        self.additional_scripts = additional_scripts
+        self.template = args.template
+        self.scripts_path = args.scripts_path
+        self.additional_scripts = args.additional_script
+
+        ssl_verify = None
+        if args.skip_ssl_verification:
+            ssl_verify = False
         self.ssl_verify = ssl_verify
 
         self.plugins = []
-
         for plugin in plugins:
-            self.plugins.append(plugin(self))
+            self.plugins.append(plugin(self, args))
 
     def _fetch_file(self, location, output=None):
         """
diff --git a/dogen/plugin.py b/dogen/plugin.py
index ef231a3..66269bf 100644
--- a/dogen/plugin.py
+++ b/dogen/plugin.py
@@ -1,10 +1,15 @@
 
 class Plugin(object):
-    def __init__(self, dogen):
+    def __init__(self, dogen, args):
         self.dogen = dogen
         self.log = dogen.log
         self.descriptor = dogen.descriptor
         self.output = dogen.output
+        self.args = args
+
+    @staticmethod
+    def inject_args(parser):
+        return parser
 
     def prepare(self, **kwargs):
         pass
diff --git a/dogen/plugins/cct.py b/dogen/plugins/cct.py
index 05e66f6..c5bd31d 100644
--- a/dogen/plugins/cct.py
+++ b/dogen/plugins/cct.py
@@ -11,8 +11,8 @@ class CCT(Plugin):
     def info():
         return "cct", "Support for configuring images via cct"
 
-    def __init__(self, dogen):
-        super(CCT, self).__init__(dogen)
+    def __init__(self, dogen, args):
+        super(CCT, self).__init__(dogen, args)
 
     def extend_schema(self, parent_schema):
         """
diff --git a/dogen/plugins/dist_git.py b/dogen/plugins/dist_git.py
index 0c119cd..3ba47f6 100644
--- a/dogen/plugins/dist_git.py
+++ b/dogen/plugins/dist_git.py
@@ -11,15 +11,26 @@ class DistGitPlugin(Plugin):
     def info():
         return "dist-git", "Support for dist-git repositories"
 
-    def __init__(self, dogen):
-        super(DistGitPlugin, self).__init__(dogen)
+    @staticmethod
+    def inject_args(parser):
+        parser.add_argument('--enable-dist-git', action='store_true', help='Enables dist-git plugin')
+        return parser
+
+    def __init__(self, dogen, args):
+        super(DistGitPlugin, self).__init__(dogen, args)
+        if  not self.args.enable_dist_git:
+            return
         self.git = Git(self.log, os.path.dirname(self.descriptor), self.output)
 
     def prepare(self, cfg):
+        if not self.args.enable_dist_git:
+            return
         self.git.prepare()
         self.git.clean_scripts()
 
     def after_sources(self, files):
+        if not self.args.enable_dist_git:
+            return
         self.git.update_lookaside_cache(files)
         self.git.update()
 
diff --git a/dogen/plugins/rpm.py b/dogen/plugins/rpm.py
index 30fa047..0f6abf2 100644
--- a/dogen/plugins/rpm.py
+++ b/dogen/plugins/rpm.py
@@ -9,8 +9,8 @@ class RPM(Plugin):
     def info():
         return "rpm","Support for injecting custom rpms"
 
-    def __init__(self, dogen):
-        super(RPM, self).__init__(dogen)
+    def __init__(self, dogen, args):
+        super(RPM, self).__init__(dogen, args)
         self.rpms_directory = os.path.join(os.path.dirname(self.descriptor), "rpms")
 
     def extend_schema(self, parent_schema):


Test patch:
diff --git a/tests/test_cct_plugin.py b/tests/test_cct_plugin.py
index 1cc4c24..ff2954c 100644
--- a/tests/test_cct_plugin.py
+++ b/tests/test_cct_plugin.py
@@ -14,7 +14,7 @@ class MockDogen():
 class TestCCTPlugin(unittest.TestCase):
     def setUp(self):
         self.workdir = tempfile.mkdtemp(prefix='test_cct_plugin')
-        self.cct = cct.CCT(dogen=MockDogen())
+        self.cct = cct.CCT(dogen=MockDogen(), args=None)
 
     def teardown(self):
         shutil.rmtree(self.workdir)
diff --git a/tests/test_schema.py b/tests/test_schema.py
index ea3acf0..25abf46 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -1,3 +1,4 @@
+import argparse
 import unittest
 import mock
 import os
@@ -10,7 +11,10 @@ class TestSchemaMeta(type):
     def __new__(mcls, name, bases, dict):
         def gen_test(path, good):
             def test(self):
-                generator = Generator(self.log, path, "target")
+                args = argparse.Namespace(path=path, output="target", without_sources=None,
+                                          template=None, scripts_path=None, additional_script=None,
+                                          skip_ssl_verification=None)
+                generator = Generator(self.log, args)
                 if good:
                     generator.configure()
                 else:
diff --git a/tests/test_unit_generate.py b/tests/test_unit_generate.py
index a1660c9..06b0b97 100644
--- a/tests/test_unit_generate.py
+++ b/tests/test_unit_generate.py
@@ -1,3 +1,4 @@
+import argparse
 import unittest
 import mock
 import tempfile
@@ -19,7 +20,10 @@ class TestGenerateCustomRepoFiles(unittest.TestCase):
             f.write(self.basic_config.encode())
             f.write("dogen:\n  ssl_verify: true".encode())
 
-        self.generator = Generator(self.log, self.descriptor.name, "target")
+        args = argparse.Namespace(path=self.descriptor.name, output="target", without_sources=None,
+                                  template=None, scripts_path=None, additional_script=None,
+                                  skip_ssl_verification=None)
+        self.generator = Generator(self.log, args)
         self.generator.configure()
 
     def tearDown(self):
diff --git a/tests/test_unit_generate_configuration.py b/tests/test_unit_generate_configuration.py
index 401ecab..adcbafd 100644
--- a/tests/test_unit_generate_configuration.py
+++ b/tests/test_unit_generate_configuration.py
@@ -1,3 +1,4 @@
+import argparse
 import unittest
 import mock
 import six
@@ -19,12 +20,14 @@ class TestConfig(unittest.TestCase):
         self.log = mock.Mock()
         self.descriptor = tempfile.NamedTemporaryFile(delete=False)
         self.descriptor.write(self.basic_config.encode())
-
+        self.args = argparse.Namespace(path=self.descriptor.name, output="target", without_sources=False,
+                                       template=None, scripts_path=None, additional_script=None,
+                                       skip_ssl_verification=None)
     def tearDown(self):
         os.remove(self.descriptor.name)
 
     def test_default_values(self):
-        self.generator = Generator(self.log, self.descriptor.name, "target")
+        self.generator = Generator(self.log, self.args)
         self.assertEqual(self.generator.output, "target")
         self.assertEqual(self.generator.dockerfile, "target/Dockerfile")
         self.assertEqual(self.generator.descriptor, self.descriptor.name)
@@ -39,7 +42,7 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  version: 99999.9.9-dev1".encode())
 
-        self.generator = Generator(self.log, self.descriptor.name, "target")
+        self.generator = Generator(self.log, self.args)
 
         with self.assertRaises(Error) as cm:
             self.generator.configure()
@@ -51,7 +54,7 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  ssl_verify: false".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         self.assertFalse(generator.ssl_verify)
 
@@ -59,7 +62,7 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  ssl_verify: true".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         self.assertTrue(generator.ssl_verify)
 
@@ -67,31 +70,25 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  template: custom-template.jinja".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         self.assertEqual(generator.template, "custom-template.jinja")
 
     def test_custom_template_in_cli_should_override_in_descriptor(self):
         with self.descriptor as f:
             f.write("dogen:\n  template: custom-template.jinja".encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", template="cli-template.jinja")
+        args = self.args
+        args.template="cli-template.jinja"
+        generator = Generator(self.log, args)
         generator.configure()
         self.assertEqual(generator.template, "cli-template.jinja")
 
-    def test_do_not_skip_ssl_verification_in_cli_true_should_override_descriptor(self):
-        with self.descriptor as f:
-            f.write("dogen:\n  ssl_verify: false".encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", ssl_verify=True)
-        generator.configure()
-        self.assertTrue(generator.ssl_verify)
-
     def test_do_not_skip_ssl_verification_in_cli_false_should_override_descriptor(self):
         with self.descriptor as f:
             f.write("dogen:\n  ssl_verify: true".encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", ssl_verify=False)
+        args = self.args
+        args.skip_ssl_verification=True
+        generator = Generator(self.log, args)
         generator.configure()
         self.assertFalse(generator.ssl_verify)
 
@@ -100,7 +97,7 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  scripts_path: custom-scripts".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         mock_patch.assert_called_with('custom-scripts')
         self.assertEqual(generator.scripts_path, "custom-scripts")
@@ -109,8 +106,9 @@ class TestConfig(unittest.TestCase):
     def test_custom_scripts_dir_in_cli_should_override_in_descriptor(self, mock_patch):
         with self.descriptor as f:
             f.write("dogen:\n  template: custom-scripts".encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", scripts_path="custom-scripts-cli")
+        args = self.args
+        args.scripts_path="custom-scripts-cli"
+        generator = Generator(self.log, args)
         generator.configure()
         mock_patch.assert_called_with('custom-scripts-cli')
         self.assertEqual(generator.scripts_path, "custom-scripts-cli")
@@ -120,7 +118,7 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  scripts_path: custom-scripts".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         mock_patch.assert_called_with('custom-scripts')
         self.assertEqual(generator.scripts_path, "custom-scripts")
@@ -129,15 +127,16 @@ class TestConfig(unittest.TestCase):
         with self.descriptor as f:
             f.write("dogen:\n  additional_scripts:\n    - http://host/somescript".encode())
 
-        generator = Generator(self.log, self.descriptor.name, "target")
+        generator = Generator(self.log, self.args)
         generator.configure()
         self.assertEqual(generator.additional_scripts, ["http://host/somescript"])
 
     def test_custom_additional_scripts_in_cli_should_override_in_descriptor(self):
         with self.descriptor as f:
             f.write("dogen:\n  additional_scripts:\n    - http://host/somescript".encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", additional_scripts=["https://otherhost/otherscript"])
+        args = self.args
+        args.additional_script=["https://otherhost/otherscript"]
+        generator = Generator(self.log, args)
         generator.configure()
         self.assertEqual(generator.additional_scripts, ["https://otherhost/otherscript"])
 
@@ -146,8 +145,9 @@ class TestConfig(unittest.TestCase):
         """Helper method for tests around script exec value"""
         with self.descriptor as f:
             f.write(cfg.encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", scripts_path="scripts")
+        args = self.args
+        args.scripts_path="scripts"
+        generator = Generator(self.log, args)
         generator.configure()
         generator._handle_scripts()
         self.assertEqual(generator.cfg['scripts'][0]['exec'], exec_to_test)
@@ -202,8 +202,9 @@ class TestConfig(unittest.TestCase):
 
         with self.descriptor as f:
             f.write(cfg.encode())
-
-        generator = Generator(self.log, self.descriptor.name, "target", scripts_path="scripts")
+        args = self.args
+        args.scripts_path="scripts"
+        generator = Generator(self.log, args)
         generator.configure()
         generator._handle_scripts()
         self.assertEqual(generator.cfg['scripts'][0]['user'], user_to_test)
@@ -255,7 +256,9 @@ class TestConfig(unittest.TestCase):
         "make sure _handle_scripts doesn't blow up when there are no scripts"
 
         self.descriptor.close()
-        generator = Generator(self.log, self.descriptor.name, "target", scripts_path="scripts")
+        args = self.args
+        args.scripts_path="scripts"
+        generator = Generator(self.log, args)
         generator.configure()
         generator._handle_scripts()
         # success if no stack trace thrown
diff --git a/tests/test_unit_generate_handle_files.py b/tests/test_unit_generate_handle_files.py
index 1956c2a..51bd803 100644
--- a/tests/test_unit_generate_handle_files.py
+++ b/tests/test_unit_generate_handle_files.py
@@ -1,3 +1,4 @@
+import argparse
 import unittest
 import mock
 import six
@@ -9,7 +10,10 @@ from dogen.tools import Tools
 class TestURL(unittest.TestCase):
     def setUp(self):
         self.log = mock.Mock()
-        self.generator = Generator(self.log, "image.yaml", "target")
+        args = argparse.Namespace(path="image.yaml", output="target", without_sources=None,
+                                  template=None, scripts_path=None, additional_script=None,
+                                  skip_ssl_verification=None)
+        self.generator = Generator(self.log, args)
 
     def test_local_file(self):
         self.assertFalse(Tools.is_url("a_file.tmp"))
@@ -23,7 +27,10 @@ class TestURL(unittest.TestCase):
 class TestFetchFile(unittest.TestCase):
     def setUp(self):
         self.log = mock.Mock()
-        self.generator = Generator(self.log, "image.yaml", "target")
+        args = argparse.Namespace(path="image.yaml", output="target", without_sources=None,
+                                  template=None, scripts_path=None, additional_script=None,
+                                  skip_ssl_verification=None)
+        self.generator = Generator(self.log, args)
 
     @mock.patch('dogen.generator.requests.get')
     def test_fetching_with_filename(self, mock_requests):
@@ -55,10 +62,17 @@ class TestFetchFile(unittest.TestCase):
 class TestCustomTemplateHandling(unittest.TestCase):
     def setUp(self):
         self.log = mock.Mock()
-        self.generator = Generator(self.log, "image.yaml", "target", template="http://host/custom-template")
+        args = argparse.Namespace(path="image.yaml", output="target", without_sources=None,
+                                  template="http://host/custom-template", scripts_path=None,
+                                  additional_script=None, skip_ssl_verification=None)
+        self.generator = Generator(self.log, args)
 
     def test_do_not_fail_if_no_template_is_provided(self):
-        self.generator = Generator(self.log, "image.yaml", "target")
+        args = argparse.Namespace(path="image.yaml", output="target", without_sources=None,
+                                  template=None, scripts_path=None, additional_script=None,
+                                  skip_ssl_verification=None)
+        self.generator = Generator(self.log, args)
+
         fetch_file_mock = mock.Mock()
         self.generator._fetch_file = fetch_file_mock
 
diff --git a/tests/test_user.py b/tests/test_user.py
index 1c87429..e261886 100644
--- a/tests/test_user.py
+++ b/tests/test_user.py
@@ -1,3 +1,4 @@
+import argparse
 import tempfile
 import unittest
 import mock
@@ -28,7 +29,9 @@ class TestUser(unittest.TestCase):
         self.yaml = os.path.join(self.workdir, "image.yaml")
         self.target = os.path.join(self.workdir, "target")
         os.mkdir(self.target)
-
+        self.args = argparse.Namespace(path=self.yaml, output=self.target, without_sources=None,
+                                       template=None, scripts_path=None, additional_script=None,
+                                       skip_ssl_verification=None)
         with open(self.yaml, 'wb') as f:
             f.write(self.basic_config.encode())
 
@@ -44,7 +47,7 @@ class TestUser(unittest.TestCase):
         with open(self.yaml, 'ab') as f:
             f.write("user: 1347".encode())
 
-        generator = Generator(self.log, self.yaml, self.target)
+        generator = Generator(self.log, self.args)
         generator.configure()
         generator.render_from_template()
 
@@ -61,7 +64,7 @@ class TestUser(unittest.TestCase):
         instruction in the Dockerfile, immediately before the CMD,
         defaulting to uid 0.
         """
-        generator = Generator(self.log, self.yaml, self.target)
+        generator = Generator(self.log, self.args)
         generator.configure()
         generator.render_from_template()