[WIP] Remove sh depedency

README.md

@@ -15,7 +15,7 @@ The interface has been developed vs. Packer v0.7.5.
 
 ## Installation
 
-You must have Packer installed prior to using this client though as installer class is provided to install packer for you.
+You must have Packer installed prior to using this client.
 
 ```shell
  pip install python-packer
@@ -153,19 +153,6 @@ p = packer.Packer(packerfile, ...)
 print(p.version())
 ```
 
-### PackerInstaller.install()
-
-This installs packer to `packer_path` using the `installer_path` and verifies that the installation was successful.
-
-```python
-
-packer_path = '/usr/bin/'
-installer_path = 'Downloads/packer_0.7.5_linux_amd64.zip'
-
-p = packer.Installer(packer_path, installer_path)
-p.install()
-```
-
 ## Shell Interaction
 
 The [sh](http://amoffat.github.io/sh/) Python module is used to execute Packer.

packer.py

@@ -1,7 +1,9 @@
-import sh
-import os
+import copy
 import json
+import os
+import subprocess
 import zipfile
+from collections import namedtuple
 
 DEFAULT_PACKER_PATH = 'packer'
 
@@ -10,8 +12,14 @@ class Packer(object):
     """A packer client
     """
 
-    def __init__(self, packerfile, exc=None, only=None, vars=None,
-                 var_file=None, exec_path=DEFAULT_PACKER_PATH, out_iter=None,
+    def __init__(self,
+                 packerfile,
+                 exc=None,
+                 only=None,
+                 vars=None,
+                 var_file=None,
+                 exec_path=DEFAULT_PACKER_PATH,
+                 out_iter=None,
                  err_iter=None):
         """
         :param string packerfile: Path to Packer template file
@@ -24,8 +32,8 @@ def __init__(self, packerfile, exc=None, only=None, vars=None,
         self.packerfile = self._validate_argtype(packerfile, str)
         self.var_file = var_file
         if not os.path.isfile(self.packerfile):
-            raise OSError('packerfile not found at path: {0}'.format(
-                self.packerfile))
+            raise OSError(
+                'packerfile not found at path: {0}'.format(self.packerfile))
         self.exc = self._validate_argtype(exc or [], list)
         self.only = self._validate_argtype(only or [], list)
         self.vars = self._validate_argtype(vars or {}, dict)
@@ -38,10 +46,22 @@ def __init__(self, packerfile, exc=None, only=None, vars=None,
             kwargs["_err"] = err_iter
             kwargs["_out_bufsize"] = 1
 
-        self.packer = sh.Command(exec_path)
-        self.packer = self.packer.bake(**kwargs)
-
-    def build(self, parallel=True, debug=False, force=False,
+        command = []
+        command.append(exec_path)
+        command.extend(self.dict_to_command(kwargs))
+        self.packer = command
+
+    def dict_to_command(self, kwargs):
+        """Convert dict to '--key=value' command parameters"""
+        param = []
+        for parameter, value in kwargs.items():
+            param.append('--{}={}'.format(parameter, value))
+        return param
+
+    def build(self,
+              parallel=True,
+              debug=False,
+              force=False,
               machine_readable=False):
         """Executes a `packer build`
 
@@ -50,7 +70,9 @@ def build(self, parallel=True, debug=False, force=False,
         :param bool force: Force artifact output even if exists
         :param bool machine_readable: Make output machine-readable
         """
-        self.packer_cmd = self.packer.build
+        cmd = copy.copy(self.packer)
+        cmd.append('build')
+        self.packer_cmd = cmd
 
         self._add_opt('-parallel=true' if parallel else None)
         self._add_opt('-debug' if debug else None)
@@ -59,22 +81,24 @@ def build(self, parallel=True, debug=False, force=False,
         self._append_base_arguments()
         self._add_opt(self.packerfile)
 
-        return self.packer_cmd()
+        return self._run_command(self.packer_cmd)
 
     def fix(self, to_file=None):
         """Implements the `packer fix` function
 
         :param string to_file: File to output fixed template to
         """
-        self.packer_cmd = self.packer.fix
+        cmd = copy.copy(self.packer)
+        cmd.append('fix')
+        self.packer_cmd = cmd
 
         self._add_opt(self.packerfile)
 
-        result = self.packer_cmd()
+        result = self._run_command(self.packer_cmd)
         if to_file:
             with open(to_file, 'w') as f:
-                f.write(result.stdout.decode())
-        result.fixed = json.loads(result.stdout.decode())
+                f.write(result.stdout)
+        result = json.loads(result.stdout)
         return result
 
     def inspect(self, mrf=True):
@@ -107,31 +131,34 @@ def inspect(self, mrf=True):
 
         :param bool mrf: output in machine-readable form.
         """
-        self.packer_cmd = self.packer.inspect
+        cmd = copy.copy(self.packer)
+        cmd.append('inspect')
+        self.packer_cmd = cmd
 
         self._add_opt('-machine-readable' if mrf else None)
         self._add_opt(self.packerfile)
 
-        result = self.packer_cmd()
+        output = self._run_command(self.packer_cmd)
+        result = output.stdout
         if mrf:
-            result.parsed_output = self._parse_inspection_output(
-                                                        result.stdout.decode())
-        else:
-            result.parsed_output = None
+            result = self._parse_inspection_output(output.stdout)
         return result
 
     def push(self, create=True, token=False):
         """Implmenets the `packer push` function
 
         UNTESTED! Must be used alongside an Atlas account
         """
-        self.packer_cmd = self.packer.push
+        cmd = copy.copy(self.packer)
+        cmd.append('push')
+        self.packer_cmd = cmd
 
-        self._add_opt('-create=true' if create else None)
-        self._add_opt('-tokn={0}'.format(token) if token else None)
+        # self._add_opt('-create=true' if create else None)
+        self._add_opt('-token={0}'.format(token) if token else None)
         self._add_opt(self.packerfile)
 
-        return self.packer_cmd()
+        result = self._run_command(self.packer_cmd)
+        return result
 
     def validate(self, syntax_only=False):
         """Validates a Packer Template file (`packer validate`)
@@ -140,25 +167,18 @@ def validate(self, syntax_only=False):
         :param bool syntax_only: Whether to validate the syntax only
         without validating the configuration itself.
         """
-        self.packer_cmd = self.packer.validate
+        cmd = copy.copy(self.packer)
+        cmd.append('validate')
+        self.packer_cmd = cmd
 
         self._add_opt('-syntax-only' if syntax_only else None)
         self._append_base_arguments()
         self._add_opt(self.packerfile)
 
-        # as sh raises an exception rather than return a value when execution
-        # fails we create an object to return the exception and the validation
-        # state
-        try:
-            validation = self.packer_cmd()
-            validation.succeeded = validation.exit_code == 0
-            validation.error = None
-        except Exception as ex:
-            validation = ValidationObject()
-            validation.succeeded = False
-            validation.failed = True
-            validation.error = ex.message
-        return validation
+        result = self._run_command(self.packer_cmd)
+        if result.returncode:
+            raise PackerException(result.stdout)
+        return result
 
     def version(self):
         """Returns Packer's version number (`packer version`)
@@ -168,16 +188,20 @@ def version(self):
         the `packer v` prefix so that you don't have to parse the version
         yourself.
         """
-        return self.packer.version().split('v')[1].rstrip('\n')
+        cmd = copy.copy(self.packer)
+        cmd.append('version')
+        output = self._run_command(cmd)
+        version = output.stdout.split('\n')[0].split('v')[1]
+        return version
 
     def _add_opt(self, option):
         if option:
-            self.packer_cmd = self.packer_cmd.bake(option)
+            self.packer_cmd.append(option)
 
     def _validate_argtype(self, arg, argtype):
         if not isinstance(arg, argtype):
-            raise PackerException('{0} argument must be of type {1}'.format(
-                arg, argtype))
+            raise PackerException(
+                '{0} argument must be of type {1}'.format(arg, argtype))
         return arg
 
     def _append_base_arguments(self):
@@ -193,9 +217,11 @@ def _append_base_arguments(self):
             self._add_opt('-except={0}'.format(self._join_comma(self.exc)))
         elif self.only:
             self._add_opt('-only={0}'.format(self._join_comma(self.only)))
+
         for var, value in self.vars.items():
             self._add_opt("-var")
             self._add_opt("{0}={1}".format(var, value))
+
         if self.var_file:
             self._add_opt('-var-file={0}'.format(self.var_file))
 
@@ -212,45 +238,34 @@ def _parse_inspection_output(self, output):
         parts = {'variables': [], 'builders': [], 'provisioners': []}
         for line in output.splitlines():
             line = line.split(',')
-            if line[2].startswith('template'):
-                del line[0:2]
+
+            packer_type = line[2]
+            if packer_type.startswith('template'):
+                del line[0:2]  # Remove date
                 component = line[0]
+                name = line[1]
+
                 if component == 'template-variable':
-                    variable = {"name": line[1], "value": line[2]}
+                    variable = {"name": name, "value": line[2]}
                     parts['variables'].append(variable)
                 elif component == 'template-builder':
-                    builder = {"name": line[1], "type": line[2]}
+                    builder = {"name": name, "type": line[2]}
                     parts['builders'].append(builder)
                 elif component == 'template-provisioner':
-                    provisioner = {"type": line[1]}
+                    provisioner = {"type": name}
                     parts['provisioners'].append(provisioner)
         return parts
 
-
-class Installer(object):
-    def __init__(self, packer_path, installer_path):
-        self.packer_path = packer_path
-        self.installer_path = installer_path
-
-    def install(self):
-        with open(self.installer_path, 'rb') as f:
-            zip = zipfile.ZipFile(f)
-            for path in zip.namelist():
-                zip.extract(path, self.packer_path)
-        exec_path = os.path.join(self.packer_path, 'packer')
-        if not self._verify_packer_installed(exec_path):
-            raise PackerException('packer installation failed. '
-                                  'Executable could not be found under: '
-                                  '{0}'.format(exec_path))
-        else:
-            return exec_path
-
-    def _verify_packer_installed(self, packer_path):
-        return os.path.isfile(packer_path)
-
-
-class ValidationObject():
-    pass
+    def _run_command(self, command):
+        """Wrapper to execute command"""
+        PackerOutput = namedtuple('PackerOutput',
+                                  ['stdout', 'stderr', 'returncode'])
+        executed = subprocess.run(
+            command, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        packer_output = PackerOutput(executed.stdout.decode(),
+                                     executed.stderr.decode(),
+                                     executed.returncode)
+        return packer_output
 
 
 class PackerException(Exception):

tests/test_packer.py

@@ -3,7 +3,6 @@
 import testtools
 import os
 
-PACKER_PATH = '/usr/bin/packer'
 TEST_RESOURCES_DIR = 'tests/resources'
 TEST_PACKERFILE = os.path.join(TEST_RESOURCES_DIR, 'simple-test.json')
 TEST_BAD_PACKERFILE = os.path.join(TEST_RESOURCES_DIR, 'badpackerfile.json')

tests/test_utils.py

@@ -0,0 +1,61 @@
+import os
+
+import packer
+import testtools
+
+TEST_RESOURCES_DIR = 'tests/resources'
+TEST_PACKERFILE = os.path.join(TEST_RESOURCES_DIR, 'simple-test.json')
+TEST_BAD_PACKERFILE = os.path.join(TEST_RESOURCES_DIR, 'badpackerfile.json')
+
+PACKER = packer.Packer(TEST_PACKERFILE)
+
+
+def test_dict_to_command():
+    """Validate dict is converted to a command properly"""
+    kwargs = {'test': 'value'}
+    cmd = PACKER.dict_to_command(kwargs)
+    assert cmd == ['--test=value']
+
+
+def test_join_comma():
+    """Validate list concatination is correct"""
+    output = PACKER._join_comma(['hello', 'world'])
+    assert output == 'hello,world'
+
+
+def test_run_command():
+    """Check returned output from executed command"""
+    cmd = ['packer', 'version']
+    output = PACKER._run_command(cmd)
+
+    assert 'Packer v' in output.stdout
+    assert '' in output.stderr
+    assert 0 == output.returncode
+
+
+def test_parse_inspection_output():
+    """Check returned output from executed command"""
+    output = '''1508999535,,ui,say,Variables:
+    1508999535,,ui,say,  <No variables>
+    1508999535,,ui,say,
+    1508999535,,ui,say,Builders:
+    1508999535,,template-builder,docker,docker
+    1508999535,,ui,say,  docker
+    1508999535,,ui,say,
+    1508999535,,ui,say,Provisioners:
+    1508999535,,template-provisioner,shell
+    1508999535,,ui,say,  shell
+    1508999535,,ui,say,Note: If your build names contain user variables or template functions such as 'timestamp'%!(PACKER_COMMA) these are processed at build time%!(PACKER_COMMA) and therefore only show in their raw form here.'''
+
+    output = PACKER._parse_inspection_output(output)
+
+    assert {
+        'builders': [{
+            'name': 'docker',
+            'type': 'docker'
+        }],
+        'provisioners': [{
+            'type': 'shell'
+        }],
+        'variables': []
+    } == output