summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoel Diaz <jdiaz@redhat.com>2017-02-28 16:22:08 +0000
committerJoel Diaz <jdiaz@redhat.com>2017-03-21 17:33:27 -0400
commita11970d30c88d188392ec217c055b6b8169b3769 (patch)
tree8c32341fde26780aa24332f9ff6aff09db6110dc
parent45fbfdad1b80c50276a9da3841d6e4089b109e35 (diff)
downloadopenshift-a11970d30c88d188392ec217c055b6b8169b3769.tar.gz
openshift-a11970d30c88d188392ec217c055b6b8169b3769.tar.bz2
openshift-a11970d30c88d188392ec217c055b6b8169b3769.tar.xz
openshift-a11970d30c88d188392ec217c055b6b8169b3769.zip
clean up and clarify docs/comments
update unit tests
-rw-r--r--roles/lib_openshift/library/oc_user.py232
-rw-r--r--roles/lib_openshift/src/class/oc_user.py4
-rw-r--r--roles/lib_openshift/src/doc/user66
-rwxr-xr-xroles/lib_openshift/src/test/integration/oc_user.yml4
-rwxr-xr-xroles/lib_openshift/src/test/unit/test_oc_user.py (renamed from roles/lib_openshift/src/test/unit/oc_user.py)16
5 files changed, 278 insertions, 44 deletions
diff --git a/roles/lib_openshift/library/oc_user.py b/roles/lib_openshift/library/oc_user.py
index ded594ac6..d64290a2e 100644
--- a/roles/lib_openshift/library/oc_user.py
+++ b/roles/lib_openshift/library/oc_user.py
@@ -33,6 +33,7 @@
from __future__ import print_function
import atexit
+import copy
import json
import os
import re
@@ -40,7 +41,11 @@ import shutil
import subprocess
import tempfile
# pylint: disable=import-error
-import ruamel.yaml as yaml
+try:
+ import ruamel.yaml as yaml
+except ImportError:
+ import yaml
+
from ansible.module_utils.basic import AnsibleModule
# -*- -*- -*- End included fragment: lib/import.py -*- -*- -*-
@@ -83,13 +88,13 @@ options:
aliases: []
full_name:
description:
- - String with the full name/description of th user.
+ - String with the full name/description of the user.
required: false
default: None
aliases: []
groups:
description:
- - List of groups the user should be a member of.
+ - List of groups the user should be a member of. This does not add/update the legacy 'groups' field in the OpenShift user object, but makes user entries into the appropriate OpenShift group object for the given user.
required: false
default: []
aliases: []
@@ -104,16 +109,79 @@ EXAMPLES = '''
state: present
username: johndoe
full_name "John Doe"
+ groups:
+ - dedicated-admins
+ register: user_johndoe
+
+user_johndoe variable will have contents like:
+ok: [ded-int-aws-master-61034] => {
+ "user_johndoe": {
+ "changed": true,
+ "results": {
+ "cmd": "oc -n default get users johndoe -o json",
+ "results": [
+ {
+ "apiVersion": "v1",
+ "fullName": "John DOe",
+ "groups": null,
+ "identities": null,
+ "kind": "User",
+ "metadata": {
+ "creationTimestamp": "2017-02-28T15:09:21Z",
+ "name": "johndoe",
+ "resourceVersion": "848781",
+ "selfLink": "/oapi/v1/users/johndoe",
+ "uid": "e23d3300-fdc7-11e6-9e3e-12822d6b7656"
+ }
+ }
+ ],
+ "returncode": 0
+ },
+ "state": "present"
+ }
+}
+'groups' is empty because this field is the OpenShift user object's 'group' field.
- name: Ensure user does not exist
oc_user:
state: absent
username: johndoe
+
+- name: List user's info
+ oc_user:
+ state: list
+ username: johndoe
+ register: user_johndoe
+
+user_johndoe will have contents similar to:
+ok: [ded-int-aws-master-61034] => {
+ "user_johndoe": {
+ "changed": false,
+ "results": [
+ {
+ "apiVersion": "v1",
+ "fullName": "John Doe",
+ "groups": null,
+ "identities": null,
+ "kind": "User",
+ "metadata": {
+ "creationTimestamp": "2017-02-28T15:04:44Z",
+ "name": "johndoe",
+ "resourceVersion": "848280",
+ "selfLink": "/oapi/v1/users/johndoe",
+ "uid": "3d479ad2-fdc7-11e6-9e3e-12822d6b7656"
+ }
+ }
+ ],
+ "state": "list"
+ }
+}
'''
# -*- -*- -*- End included fragment: doc/user -*- -*- -*-
# -*- -*- -*- Begin included fragment: ../../lib_utils/src/class/yedit.py -*- -*- -*-
+# pylint: disable=undefined-variable,missing-docstring
# noqa: E301,E302
@@ -308,11 +376,17 @@ class Yedit(object):
if self.backup and self.file_exists():
shutil.copy(self.filename, self.filename + '.orig')
- # pylint: disable=no-member
- if hasattr(self.yaml_dict, 'fa'):
+ # Try to set format attributes if supported
+ try:
self.yaml_dict.fa.set_block_style()
+ except AttributeError:
+ pass
- Yedit._write(self.filename, yaml.dump(self.yaml_dict, Dumper=yaml.RoundTripDumper))
+ # Try to use RoundTripDumper if supported.
+ try:
+ Yedit._write(self.filename, yaml.dump(self.yaml_dict, Dumper=yaml.RoundTripDumper))
+ except AttributeError:
+ Yedit._write(self.filename, yaml.safe_dump(self.yaml_dict, default_flow_style=False))
return (True, self.yaml_dict)
@@ -352,10 +426,24 @@ class Yedit(object):
# check if it is yaml
try:
if content_type == 'yaml' and contents:
- self.yaml_dict = yaml.load(contents, yaml.RoundTripLoader)
- # pylint: disable=no-member
- if hasattr(self.yaml_dict, 'fa'):
+ # Try to set format attributes if supported
+ try:
+ self.yaml_dict.fa.set_block_style()
+ except AttributeError:
+ pass
+
+ # Try to use RoundTripLoader if supported.
+ try:
+ self.yaml_dict = yaml.safe_load(contents, yaml.RoundTripLoader)
+ except AttributeError:
+ self.yaml_dict = yaml.safe_load(contents)
+
+ # Try to set format attributes if supported
+ try:
self.yaml_dict.fa.set_block_style()
+ except AttributeError:
+ pass
+
elif content_type == 'json' and contents:
self.yaml_dict = json.loads(contents)
except yaml.YAMLError as err:
@@ -384,14 +472,16 @@ class Yedit(object):
return (False, self.yaml_dict)
if isinstance(entry, dict):
- # pylint: disable=no-member,maybe-no-member
+ # AUDIT:maybe-no-member makes sense due to fuzzy types
+ # pylint: disable=maybe-no-member
if key_or_item in entry:
entry.pop(key_or_item)
return (True, self.yaml_dict)
return (False, self.yaml_dict)
elif isinstance(entry, list):
- # pylint: disable=no-member,maybe-no-member
+ # AUDIT:maybe-no-member makes sense due to fuzzy types
+ # pylint: disable=maybe-no-member
ind = None
try:
ind = entry.index(key_or_item)
@@ -459,7 +549,9 @@ class Yedit(object):
if not isinstance(entry, list):
return (False, self.yaml_dict)
- # pylint: disable=no-member,maybe-no-member
+ # AUDIT:maybe-no-member makes sense due to loading data from
+ # a serialized format.
+ # pylint: disable=maybe-no-member
entry.append(value)
return (True, self.yaml_dict)
@@ -472,7 +564,8 @@ class Yedit(object):
entry = None
if isinstance(entry, dict):
- # pylint: disable=no-member,maybe-no-member
+ # AUDIT:maybe-no-member makes sense due to fuzzy types
+ # pylint: disable=maybe-no-member
if not isinstance(value, dict):
raise YeditException('Cannot replace key, value entry in ' +
'dict with non-dict type. value=[%s] [%s]' % (value, type(value))) # noqa: E501
@@ -481,7 +574,8 @@ class Yedit(object):
return (True, self.yaml_dict)
elif isinstance(entry, list):
- # pylint: disable=no-member,maybe-no-member
+ # AUDIT:maybe-no-member makes sense due to fuzzy types
+ # pylint: disable=maybe-no-member
ind = None
if curr_value:
try:
@@ -520,12 +614,20 @@ class Yedit(object):
return (False, self.yaml_dict)
# deepcopy didn't work
- tmp_copy = yaml.load(yaml.round_trip_dump(self.yaml_dict,
- default_flow_style=False),
- yaml.RoundTripLoader)
- # pylint: disable=no-member
- if hasattr(self.yaml_dict, 'fa'):
+ # Try to use ruamel.yaml and fallback to pyyaml
+ try:
+ tmp_copy = yaml.load(yaml.round_trip_dump(self.yaml_dict,
+ default_flow_style=False),
+ yaml.RoundTripLoader)
+ except AttributeError:
+ tmp_copy = copy.deepcopy(self.yaml_dict)
+
+ # set the format attributes if available
+ try:
tmp_copy.fa.set_block_style()
+ except AttributeError:
+ pass
+
result = Yedit.add_entry(tmp_copy, path, value, self.separator)
if not result:
return (False, self.yaml_dict)
@@ -538,11 +640,20 @@ class Yedit(object):
''' create a yaml file '''
if not self.file_exists():
# deepcopy didn't work
- tmp_copy = yaml.load(yaml.round_trip_dump(self.yaml_dict, default_flow_style=False), # noqa: E501
- yaml.RoundTripLoader)
- # pylint: disable=no-member
- if hasattr(self.yaml_dict, 'fa'):
+ # Try to use ruamel.yaml and fallback to pyyaml
+ try:
+ tmp_copy = yaml.load(yaml.round_trip_dump(self.yaml_dict,
+ default_flow_style=False),
+ yaml.RoundTripLoader)
+ except AttributeError:
+ tmp_copy = copy.deepcopy(self.yaml_dict)
+
+ # set the format attributes if available
+ try:
tmp_copy.fa.set_block_style()
+ except AttributeError:
+ pass
+
result = Yedit.add_entry(tmp_copy, path, value, self.separator)
if result:
self.yaml_dict = tmp_copy
@@ -698,6 +809,32 @@ class OpenShiftCLIError(Exception):
pass
+ADDITIONAL_PATH_LOOKUPS = ['/usr/local/bin', os.path.expanduser('~/bin')]
+
+
+def locate_oc_binary():
+ ''' Find and return oc binary file '''
+ # https://github.com/openshift/openshift-ansible/issues/3410
+ # oc can be in /usr/local/bin in some cases, but that may not
+ # be in $PATH due to ansible/sudo
+ paths = os.environ.get("PATH", os.defpath).split(os.pathsep) + ADDITIONAL_PATH_LOOKUPS
+
+ oc_binary = 'oc'
+
+ # Use shutil.which if it is available, otherwise fallback to a naive path search
+ try:
+ which_result = shutil.which(oc_binary, path=os.pathsep.join(paths))
+ if which_result is not None:
+ oc_binary = which_result
+ except AttributeError:
+ for path in paths:
+ if os.path.exists(os.path.join(path, oc_binary)):
+ oc_binary = os.path.join(path, oc_binary)
+ break
+
+ return oc_binary
+
+
# pylint: disable=too-few-public-methods
class OpenShiftCLI(object):
''' Class to wrap the command line tools '''
@@ -709,8 +846,9 @@ class OpenShiftCLI(object):
''' Constructor for OpenshiftCLI '''
self.namespace = namespace
self.verbose = verbose
- self.kubeconfig = kubeconfig
+ self.kubeconfig = Utils.create_tmpfile_copy(kubeconfig)
self.all_namespaces = all_namespaces
+ self.oc_binary = locate_oc_binary()
# Pylint allows only 5 arguments to be passed.
# pylint: disable=too-many-arguments
@@ -907,16 +1045,15 @@ class OpenShiftCLI(object):
stdout, stderr = proc.communicate(input_data)
- return proc.returncode, stdout, stderr
+ return proc.returncode, stdout.decode(), stderr.decode()
# pylint: disable=too-many-arguments,too-many-branches
def openshift_cmd(self, cmd, oadm=False, output=False, output_type='json', input_data=None):
'''Base command for oc '''
- cmds = []
+ cmds = [self.oc_binary]
+
if oadm:
- cmds = ['oadm']
- else:
- cmds = ['oc']
+ cmds.append('adm')
if self.all_namespaces:
cmds.extend(['--all-namespaces'])
@@ -932,7 +1069,10 @@ class OpenShiftCLI(object):
if self.verbose:
print(' '.join(cmds))
- returncode, stdout, stderr = self._run(cmds, input_data)
+ try:
+ returncode, stdout, stderr = self._run(cmds, input_data)
+ except OSError as ex:
+ returncode, stdout, stderr = 1, '', 'Failed to execute {}: {}'.format(subprocess.list2cmdline(cmds), ex)
rval = {"returncode": returncode,
"results": results,
@@ -984,7 +1124,13 @@ class Utils(object):
tmp = Utils.create_tmpfile(prefix=rname)
if ftype == 'yaml':
- Utils._write(tmp, yaml.dump(data, Dumper=yaml.RoundTripDumper))
+ # AUDIT:no-member makes sense here due to ruamel.YAML/PyYAML usage
+ # pylint: disable=no-member
+ if hasattr(yaml, 'RoundTripDumper'):
+ Utils._write(tmp, yaml.dump(data, Dumper=yaml.RoundTripDumper))
+ else:
+ Utils._write(tmp, yaml.safe_dump(data, default_flow_style=False))
+
elif ftype == 'json':
Utils._write(tmp, json.dumps(data))
else:
@@ -995,7 +1141,18 @@ class Utils(object):
return tmp
@staticmethod
- def create_tmpfile(prefix=None):
+ def create_tmpfile_copy(inc_file):
+ '''create a temporary copy of a file'''
+ tmpfile = Utils.create_tmpfile('lib_openshift-')
+ Utils._write(tmpfile, open(inc_file).read())
+
+ # Cleanup the tmpfile
+ atexit.register(Utils.cleanup, [tmpfile])
+
+ return tmpfile
+
+ @staticmethod
+ def create_tmpfile(prefix='tmp'):
''' Generates and returns a temporary file name '''
with tempfile.NamedTemporaryFile(prefix=prefix, delete=False) as tmp:
@@ -1055,7 +1212,12 @@ class Utils(object):
contents = sfd.read()
if sfile_type == 'yaml':
- contents = yaml.load(contents, yaml.RoundTripLoader)
+ # AUDIT:no-member makes sense here due to ruamel.YAML/PyYAML usage
+ # pylint: disable=no-member
+ if hasattr(yaml, 'RoundTripLoader'):
+ contents = yaml.load(contents, yaml.RoundTripLoader)
+ else:
+ contents = yaml.safe_load(contents)
elif sfile_type == 'json':
contents = json.loads(contents)
@@ -1298,14 +1460,14 @@ class OCUser(OpenShiftCLI):
@property
def user(self):
- ''' property function service'''
+ ''' property function user'''
if not self._user:
self.get()
return self._user
@user.setter
def user(self, data):
- ''' setter function for yedit var '''
+ ''' setter function for user '''
self._user = data
def exists(self):
diff --git a/roles/lib_openshift/src/class/oc_user.py b/roles/lib_openshift/src/class/oc_user.py
index 17b679289..d9e4eac13 100644
--- a/roles/lib_openshift/src/class/oc_user.py
+++ b/roles/lib_openshift/src/class/oc_user.py
@@ -19,14 +19,14 @@ class OCUser(OpenShiftCLI):
@property
def user(self):
- ''' property function service'''
+ ''' property function user'''
if not self._user:
self.get()
return self._user
@user.setter
def user(self, data):
- ''' setter function for yedit var '''
+ ''' setter function for user '''
self._user = data
def exists(self):
diff --git a/roles/lib_openshift/src/doc/user b/roles/lib_openshift/src/doc/user
index cae108415..65ee01eb7 100644
--- a/roles/lib_openshift/src/doc/user
+++ b/roles/lib_openshift/src/doc/user
@@ -37,13 +37,13 @@ options:
aliases: []
full_name:
description:
- - String with the full name/description of th user.
+ - String with the full name/description of the user.
required: false
default: None
aliases: []
groups:
description:
- - List of groups the user should be a member of.
+ - List of groups the user should be a member of. This does not add/update the legacy 'groups' field in the OpenShift user object, but makes user entries into the appropriate OpenShift group object for the given user.
required: false
default: []
aliases: []
@@ -58,9 +58,71 @@ EXAMPLES = '''
state: present
username: johndoe
full_name "John Doe"
+ groups:
+ - dedicated-admins
+ register: user_johndoe
+
+user_johndoe variable will have contents like:
+ok: [ded-int-aws-master-61034] => {
+ "user_johndoe": {
+ "changed": true,
+ "results": {
+ "cmd": "oc -n default get users johndoe -o json",
+ "results": [
+ {
+ "apiVersion": "v1",
+ "fullName": "John DOe",
+ "groups": null,
+ "identities": null,
+ "kind": "User",
+ "metadata": {
+ "creationTimestamp": "2017-02-28T15:09:21Z",
+ "name": "johndoe",
+ "resourceVersion": "848781",
+ "selfLink": "/oapi/v1/users/johndoe",
+ "uid": "e23d3300-fdc7-11e6-9e3e-12822d6b7656"
+ }
+ }
+ ],
+ "returncode": 0
+ },
+ "state": "present"
+ }
+}
+'groups' is empty because this field is the OpenShift user object's 'group' field.
- name: Ensure user does not exist
oc_user:
state: absent
username: johndoe
+
+- name: List user's info
+ oc_user:
+ state: list
+ username: johndoe
+ register: user_johndoe
+
+user_johndoe will have contents similar to:
+ok: [ded-int-aws-master-61034] => {
+ "user_johndoe": {
+ "changed": false,
+ "results": [
+ {
+ "apiVersion": "v1",
+ "fullName": "John Doe",
+ "groups": null,
+ "identities": null,
+ "kind": "User",
+ "metadata": {
+ "creationTimestamp": "2017-02-28T15:04:44Z",
+ "name": "johndoe",
+ "resourceVersion": "848280",
+ "selfLink": "/oapi/v1/users/johndoe",
+ "uid": "3d479ad2-fdc7-11e6-9e3e-12822d6b7656"
+ }
+ }
+ ],
+ "state": "list"
+ }
+}
'''
diff --git a/roles/lib_openshift/src/test/integration/oc_user.yml b/roles/lib_openshift/src/test/integration/oc_user.yml
index 7d6221e64..ad1f9d188 100755
--- a/roles/lib_openshift/src/test/integration/oc_user.yml
+++ b/roles/lib_openshift/src/test/integration/oc_user.yml
@@ -142,8 +142,8 @@
register: user_out
- name: assert test group created
assert:
- that: user_out['results'][0]['metadata']['name'] == "integration-test-group" and
- user_out['results'][0]['users'] is not defined
+ that: user_out['results']['results'][0]['metadata']['name'] == "integration-test-group"
+ msg: "{{ user_out }}"
- name: create user with group membership
oc_user:
diff --git a/roles/lib_openshift/src/test/unit/oc_user.py b/roles/lib_openshift/src/test/unit/test_oc_user.py
index 920b06d0e..933e96ae2 100755
--- a/roles/lib_openshift/src/test/unit/oc_user.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_user.py
@@ -24,7 +24,7 @@ import mock
# place class in our python path
module_path = os.path.join('/'.join(os.path.realpath(__file__).split('/')[:-4]), 'library') # noqa: E501
sys.path.insert(0, module_path)
-from oc_user import OCUser # noqa: E402
+from oc_user import OCUser, locate_oc_binary # noqa: E402
class OCUserTest(unittest.TestCase):
@@ -36,8 +36,9 @@ class OCUserTest(unittest.TestCase):
''' setup method will create a file and set to known configuration '''
pass
+ @mock.patch('oc_user.Utils.create_tmpfile_copy')
@mock.patch('oc_user.OCUser._run')
- def test_state_list(self, mock_cmd):
+ def test_state_list(self, mock_cmd, mock_tmpfile_copy):
''' Testing a user list '''
params = {'username': 'testuser@email.com',
'state': 'list',
@@ -65,13 +66,18 @@ class OCUserTest(unittest.TestCase):
(0, user, ''),
]
+ mock_tmpfile_copy.side_effect = [
+ '/tmp/mocked_kubeconfig',
+ ]
+
results = OCUser.run_ansible(params, False)
self.assertFalse(results['changed'])
self.assertTrue(results['results'][0]['metadata']['name'] == "testuser@email.com")
+ @mock.patch('oc_user.Utils.create_tmpfile_copy')
@mock.patch('oc_user.OCUser._run')
- def test_state_present(self, mock_cmd):
+ def test_state_present(self, mock_cmd, mock_tmpfile_copy):
''' Testing a user list '''
params = {'username': 'testuser@email.com',
'state': 'present',
@@ -102,6 +108,10 @@ class OCUserTest(unittest.TestCase):
(0, created_user, ''), # get
]
+ mock_tmpfile_copy.side_effect = [
+ '/tmp/mocked_kubeconfig',
+ ]
+
results = OCUser.run_ansible(params, False)
self.assertTrue(results['changed'])