Changed lib_openshift to use real temporary files.

6 files changed, 62 insertions, 45 deletions

diff --git a/roles/lib_openshift/src/class/oc_obj.py b/roles/lib_openshift/src/class/oc_obj.py
index 9d0b8e45b..2ec20e72c 100644
--- a/roles/lib_openshift/src/class/oc_obj.py
+++ b/roles/lib_openshift/src/class/oc_obj.py
@@ -49,7 +49,7 @@ class OCObject(OpenShiftCLI):
             return self._create(files[0])
 
         content['data'] = yaml.dump(content['data'])
-        content_file = Utils.create_files_from_contents(content)[0]
+        content_file = Utils.create_tmp_files_from_contents(content)[0]
 
         return self._create(content_file['path'])
 
diff --git a/roles/lib_openshift/src/class/oc_secret.py b/roles/lib_openshift/src/class/oc_secret.py
index 40b2aa4d1..e99999c37 100644
--- a/roles/lib_openshift/src/class/oc_secret.py
+++ b/roles/lib_openshift/src/class/oc_secret.py
@@ -48,7 +48,7 @@ class OCSecret(OpenShiftCLI):
     def create(self, files=None, contents=None):
         '''Create a secret '''
         if not files:
-            files = Utils.create_files_from_contents(contents)
+            files = Utils.create_tmp_files_from_contents(contents)
 
         secrets = ["%s=%s" % (sfile['name'], sfile['path']) for sfile in files]
         cmd = ['secrets', 'new', self.name]
@@ -81,7 +81,7 @@ class OCSecret(OpenShiftCLI):
             This is accomplished by passing -ojson.  This will most likely change in the future
         '''
         if not files:
-            files = Utils.create_files_from_contents(contents)
+            files = Utils.create_tmp_files_from_contents(contents)
 
         secrets = ["%s=%s" % (sfile['name'], sfile['path']) for sfile in files]
         cmd = ['-ojson', 'secrets', 'new', self.name]
@@ -132,7 +132,7 @@ class OCSecret(OpenShiftCLI):
             if params['files']:
                 files = params['files']
             elif params['contents']:
-                files = Utils.create_files_from_contents(params['contents'])
+                files = Utils.create_tmp_files_from_contents(params['contents'])
             else:
                 return {'failed': True,
                         'msg': 'Either specify files or contents.'}
@@ -146,7 +146,7 @@ class OCSecret(OpenShiftCLI):
                     return {'changed': True,
                             'msg': 'Would have performed a create.'}
 
-                api_rval = ocsecret.create(params['files'], params['contents'])
+                api_rval = ocsecret.create(files, params['contents'])
 
                 # Remove files
                 if files and params['delete_after']:
diff --git a/roles/lib_openshift/src/lib/base.py b/roles/lib_openshift/src/lib/base.py
index 55f7d3146..d0843c03e 100644
--- a/roles/lib_openshift/src/lib/base.py
+++ b/roles/lib_openshift/src/lib/base.py
@@ -31,7 +31,8 @@ class OpenShiftCLI(object):
         if not res['results']:
             return res
 
-        fname = '/tmp/%s' % rname
+        fname = Utils.create_tmpfile(rname + '-')
+
         yed = Yedit(fname, res['results'][0], separator=sep)
         changes = []
         for key, value in content.items():
@@ -55,7 +56,7 @@ class OpenShiftCLI(object):
 
     def _create_from_content(self, rname, content):
         '''create a temporary file and then call oc create on it'''
-        fname = '/tmp/%s' % rname
+        fname = Utils.create_tmpfile(rname + '-')
         yed = Yedit(fname, content=content)
         yed.write()
 
@@ -98,7 +99,7 @@ class OpenShiftCLI(object):
         if results['returncode'] != 0 or not create:
             return results
 
-        fname = '/tmp/%s' % template_name
+        fname = Utils.create_tmpfile(template_name + '-')
         yed = Yedit(fname, results['results'])
         yed.write()
 
@@ -279,32 +280,50 @@ class OpenShiftCLI(object):
 
 class Utils(object):
     ''' utilities for openshiftcli modules '''
+
+    @staticmethod
+    def _write(filename, contents):
+        ''' Actually write the file contents to disk. This helps with mocking. '''
+
+        with open(filename, 'w') as sfd:
+            sfd.write(contents)
+
     @staticmethod
-    def create_file(rname, data, ftype='yaml'):
+    def create_tmp_file_from_contents(rname, data, ftype='yaml'):
         ''' create a file in tmp with name and contents'''
-        path = os.path.join('/tmp', rname)
-        with open(path, 'w') as fds:
-            if ftype == 'yaml':
-                fds.write(yaml.dump(data, Dumper=yaml.RoundTripDumper))
 
-            elif ftype == 'json':
-                fds.write(json.dumps(data))
-            else:
-                fds.write(data)
+        tmp = Utils.create_tmpfile(prefix=rname)
+
+        if ftype == 'yaml':
+            Utils._write(tmp, yaml.dump(data, Dumper=yaml.RoundTripDumper))
+        elif ftype == 'json':
+            Utils._write(tmp, json.dumps(data))
+        else:
+            Utils._write(tmp, data)
 
         # Register cleanup when module is done
-        atexit.register(Utils.cleanup, [path])
-        return path
+        atexit.register(Utils.cleanup, [tmp])
+        return tmp
+
+    @staticmethod
+    def create_tmpfile(prefix=None):
+        ''' Generates and returns a temporary file name '''
+
+        with tempfile.NamedTemporaryFile(prefix=prefix, delete=False) as tmp:
+            return tmp.name
 
     @staticmethod
-    def create_files_from_contents(content, content_type=None):
+    def create_tmp_files_from_contents(content, content_type=None):
         '''Turn an array of dict: filename, content into a files array'''
         if not isinstance(content, list):
             content = [content]
         files = []
         for item in content:
-            path = Utils.create_file(item['path'], item['data'], ftype=content_type)
-            files.append({'name': os.path.basename(path), 'path': path})
+            path = Utils.create_tmp_file_from_contents(item['path'] + '-',
+                                                       item['data'],
+                                                       ftype=content_type)
+            files.append({'name': os.path.basename(item['path']),
+                          'path': path})
         return files
 
     @staticmethod
diff --git a/roles/lib_openshift/src/lib/import.py b/roles/lib_openshift/src/lib/import.py
index c2b30e019..6344c1a54 100644
--- a/roles/lib_openshift/src/lib/import.py
+++ b/roles/lib_openshift/src/lib/import.py
@@ -12,6 +12,7 @@ import os
 import re
 import shutil
 import subprocess
+import tempfile
 # pylint: disable=import-error
 import ruamel.yaml as yaml
 from ansible.module_utils.basic import AnsibleModule
diff --git a/roles/lib_openshift/src/test/unit/oc_secret.py b/roles/lib_openshift/src/test/unit/oc_secret.py
index 835918b95..c81f0514b 100755
--- a/roles/lib_openshift/src/test/unit/oc_secret.py
+++ b/roles/lib_openshift/src/test/unit/oc_secret.py
@@ -35,8 +35,9 @@ class OCSecretTest(unittest.TestCase):
         ''' setup method will create a file and set to known configuration '''
         pass
 
-    @mock.patch('oc_secret.OCSecret.openshift_cmd')
-    def test_adding_a_secret(self, mock_openshift_cmd):
+    @mock.patch('oc_secret.Utils._write')
+    @mock.patch('oc_secret.OCSecret._run')
+    def test_adding_a_secret(self, mock_cmd, mock_write):
         ''' Testing adding a secret '''
 
         # Arrange
@@ -45,10 +46,10 @@ class OCSecretTest(unittest.TestCase):
         params = {
             'state': 'present',
             'namespace': 'default',
-            'name': 'secretname',
+            'name': 'testsecretname',
             'contents': [{
                 'path': "/tmp/somesecret.json",
-                'data': "{'one': 1, 'two': 2, 'three', 3}",
+                'data': "{'one': 1, 'two': 2, 'three': 3}",
             }],
             'decode': False,
             'kubeconfig': '/etc/origin/master/admin.kubeconfig',
@@ -58,17 +59,9 @@ class OCSecretTest(unittest.TestCase):
         }
 
         # Return values of our mocked function call. These get returned once per call.
-        mock_openshift_cmd.side_effect = [
-            {
-                "cmd": "/usr/bin/oc get secrets -o json secretname",
-                "results": "",
-                "returncode": 0,
-            },  # oc output for first call to openshift_cmd (oc secrets get)
-            {
-                "cmd": "/usr/bin/oc secrets new secretname somesecret.json=/tmp/somesecret.json",
-                "results": "",
-                "returncode": 0,
-            },  # oc output for second call to openshift_cmd (oc secrets new)
+        mock_cmd.side_effect = [
+            (1, '', 'Error from server: secrets "testsecretname" not found'),
+            (0, 'secret/testsecretname', ''),
         ]
 
         # Act
@@ -80,9 +73,13 @@ class OCSecretTest(unittest.TestCase):
         self.assertEqual(results['state'], 'present')
 
         # Making sure our mock was called as we expected
-        mock_openshift_cmd.assert_has_calls([
-            mock.call(['get', 'secrets', 'secretname', '-o', 'json'], output=True),
-            mock.call(['secrets', 'new', 'secretname', 'somesecret.json=/tmp/somesecret.json']),
+        mock_cmd.assert_has_calls([
+            mock.call(['oc', '-n', 'default', 'get', 'secrets', 'testsecretname', '-o', 'json'], None),
+            mock.call(['oc', '-n', 'default', 'secrets', 'new', 'testsecretname', mock.ANY], None),
+        ])
+
+        mock_write.assert_has_calls([
+            mock.call(mock.ANY, "{'one': 1, 'two': 2, 'three': 3}"),
         ])
 
     def tearDown(self):
diff --git a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py b/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py
index 342da961b..08fc9f6df 100755
--- a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py
+++ b/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py
@@ -149,18 +149,18 @@ metadata:
         mock_cmd.assert_has_calls([
             mock.call(['oc', '-n', 'default', 'get', 'sa', 'builder', '-o', 'json'], None),
             mock.call(['oc', '-n', 'default', 'get', 'sa', 'builder', '-o', 'json'], None),
-            mock.call(['oc', '-n', 'default', 'replace', '-f', '/tmp/builder'], None),
+            mock.call(['oc', '-n', 'default', 'replace', '-f', mock.ANY], None),
             mock.call(['oc', '-n', 'default', 'get', 'sa', 'builder', '-o', 'json'], None)
         ])
 
         mock_write.assert_has_calls([
-            mock.call('/tmp/builder', builder_yaml_file)
+            mock.call(mock.ANY, builder_yaml_file)
         ])
 
     @mock.patch('oc_serviceaccount_secret.Yedit._write')
     @mock.patch('oc_serviceaccount_secret.OCServiceAccountSecret._run')
     def test_removing_a_secret_to_a_serviceaccount(self, mock_cmd, mock_write):
-        ''' Testing adding a secret to a service account '''
+        ''' Testing removing a secret to a service account '''
 
         # Arrange
 
@@ -241,11 +241,11 @@ metadata:
         mock_cmd.assert_has_calls([
             mock.call(['oc', '-n', 'default', 'get', 'sa', 'builder', '-o', 'json'], None),
             mock.call(['oc', '-n', 'default', 'get', 'sa', 'builder', '-o', 'json'], None),
-            mock.call(['oc', '-n', 'default', 'replace', '-f', '/tmp/builder'], None),
+            mock.call(['oc', '-n', 'default', 'replace', '-f', mock.ANY], None),
         ])
 
         mock_write.assert_has_calls([
-            mock.call('/tmp/builder', builder_yaml_file)
+            mock.call(mock.ANY, builder_yaml_file)
         ])
 
     def tearDown(self):