1123446 - Syncing against a directory uses 'modulename' rather than '…

…authorname-modulename' as the module name

pulp_puppet_common/pulp_puppet/common/model.py

@@ -112,7 +112,10 @@ def from_unit(cls, pulp_unit):
     @classmethod
     def from_json(cls, module_json):
         """
-        Converts a module's metadata.json to a Module representation.
+        Converts a module's metadata.json to a Module representation. This
+        method does not use the 'author' field of metadata.json. The author
+        is retrieved from the 'name' field, which is expected to be in the
+        format 'authorname-modulename' or 'authorname/modulename'.
 
         :param module_json: dict with values from metadata.json
         :type  module_json: dict
@@ -121,7 +124,12 @@ def from_json(cls, module_json):
         :rtype:  Module
         """
         # The unique identifier fields are all required and should be present
-        author, name = module_json.get('name').split("-", 1)
+        try:
+            author, name = module_json.get('name').split("-", 1)
+        except ValueError:
+            # This is the old naming format, but Puppet still allows it
+            author, name = module_json.get('name').split("/", 1)
+
         version = module_json.get('version')
 
         module = cls(name, version, author)

pulp_puppet_common/test/unit/test_common_model.py

@@ -139,10 +139,6 @@ def test_to_json(self):
 
 class ModuleTests(unittest.TestCase):
 
-    def test_from_dict(self):
-        # Setup
-        pass
-
     def test_update_from_json(self):
         # Setup
         module = Module('jdob-valid', '1.0.0', 'jdob')
@@ -177,6 +173,24 @@ def test_from_json(self):
         module.name = "jdob-valid" # rename the module to use the assert
         self.assert_valid_module(module)
 
+    def test_from_json_old_name(self):
+        """
+        Test that the Module.from_json method handles the old naming style
+        """
+        # Setup
+        metadata = {
+            'name': 'oldauthor/oldmodule',
+            'version': '0.1.0',
+        }
+
+        # Test
+        module = Module.from_json(metadata)
+
+        # Verify
+        self.assertEqual(module.author, 'oldauthor')
+        self.assertEqual(module.name, 'oldmodule')
+        self.assertEqual(module.version, '0.1.0')
+
     def assert_valid_module(self, module):
         self.assertEqual(module.name, 'jdob-valid')
         self.assertEqual(module.version, '1.0.0')

pulp_puppet_plugins/pulp_puppet/plugins/importers/directory.py

@@ -271,7 +271,7 @@ def _import_modules(self, inventory, module_paths):
             if self.canceled:
                 return []
             puppet_manifest = self._extract_metadata(module_path)
-            module = Module.from_dict(puppet_manifest)
+            module = Module.from_json(puppet_manifest)
             if inventory.already_associated(module):
                 continue
             _LOG.info(IMPORT_MODULE % dict(mod=module_path))

pulp_puppet_plugins/test/unit/plugins/importer/test_directory.py

@@ -399,34 +399,39 @@ def test_import_modules(self, mock_extract, mock_add):
         mock_inventory = Mock()
         mock_inventory.already_associated.side_effect = [False, True, False]
 
+        # These manifests represent the parsed metadata.json file. These contain a 'name'
+        # field, where we retrieve both the unit key's 'name' and 'author' field.
         manifests = [
+            {'name': 'john-pulp1', 'author': 'Johnathon', 'version': '1.0'},
+            {'name': 'john-pulp2', 'author': 'Johnathon', 'version': '2.0'},
+            {'name': 'john/pulp3', 'author': 'Johnathon', 'version': '3.0'},
+        ]
+        mock_extract.side_effect = manifests
+
+        unit_keys = [
             {'name': 'pulp1', 'author': 'john', 'version': '1.0'},
             {'name': 'pulp2', 'author': 'john', 'version': '2.0'},
             {'name': 'pulp3', 'author': 'john', 'version': '3.0'},
         ]
 
-        mock_extract.side_effect = manifests
-
         module_paths = [
             '/tmp/module_1',
             '/tmp/module_2',
             '/tmp/module_3',
         ]
 
         # test
-
         method = SynchronizeWithDirectory(conduit, config)
         imported_modules = method._import_modules(mock_inventory, module_paths)
 
         # validation
-
         mock_add.assert_any_with(module_paths[0], ANY)
         mock_add.assert_any_with(module_paths[2], ANY)
 
         # should only be modules 1 and 3.  2 already associated.
         self.assertEqual(len(imported_modules), 2)
-        self.assertEqual(imported_modules[0], manifests[0])
-        self.assertEqual(imported_modules[1], manifests[2])
+        self.assertEqual(imported_modules[0], unit_keys[0])
+        self.assertEqual(imported_modules[1], unit_keys[2])
 
     @patch('pulp_puppet.plugins.importers.directory.SynchronizeWithDirectory._extract_metadata')
     def test_import_modules_cancelled(self, mock_extract):

pulp_puppet_tools/pulp_puppet/tools/puppet_module_builder.py

@@ -47,20 +47,20 @@
 
 DESCRIPTION = _(
     'Build puppet modules.'
-    'Search the working directory and build all puppet modules found. The working'
-    'directory is the current directory unless the (-w|--working-dir) option is specified.'
-    'The (-p|--path) option may be used to specify a directory to search/build and'
-    'can be either an absolute path or a path relative to the working directory.'
-    'The archive built using \'puppet module build\' is copied to the output directory'
-    'The output directory is the current directory unless (-o|--output-dir) is'
-    'specified.  The output directory may be either an absolute path or a path that is'
-    'relative to the working directory.'
-    '\nSeveral options are provided for working with git.  Repositories may be cloned'
-    'by specifying the (-u|--url) option. After cloning git repositories, the (-p|--path)'
-    'is set to the root of the cloned repository unless specified explicitly.'
-    'The repository branch may be selected by using the (-b|--branch) option.'
-    'In all cases, when the working directory is a git repository, a \'git pull\' is'
-    'performed to ensure that the repository is up to date.'
+    ' Search the working directory and build all puppet modules found. The working'
+    ' directory is the current directory unless the (-w|--working-dir) option is specified.'
+    ' The (-p|--path) option may be used to specify a directory to search/build and'
+    ' can be either an absolute path or a path relative to the working directory.'
+    ' The archive built using \'puppet module build\' is copied to the output directory'
+    ' The output directory is the current directory unless (-o|--output-dir) is'
+    ' specified.  The output directory may be either an absolute path or a path that is'
+    ' relative to the working directory.'
+    ' \nSeveral options are provided for working with git.  Repositories may be cloned'
+    ' by specifying the (-u|--url) option. After cloning git repositories, the (-p|--path)'
+    ' is set to the root of the cloned repository unless specified explicitly.'
+    ' The repository branch may be selected by using the (-b|--branch) option.'
+    ' In all cases, when the working directory is a git repository, a \'git pull\' is'
+    ' performed to ensure that the repository is up to date.'
     '\n')
 
 BAD_PATH = _('(-p|--path) must be a relative path')
@@ -215,7 +215,7 @@ def git_checkout(options):
 def find_modules():
     """
     Search for puppet (source) modules to build and return a list of paths.
-    Puppet modules are identified by finding '<module>/modules/init.pp'
+    Puppet modules are identified by finding '<module>/manifests/init.pp'
     files.  Once found, the *module* directory path is included in the result.
 
     :return: A set of puppet module directory paths.