Add symbol support to zos_apf (#1493)

* modified DatasetCreatedError message

* Added gdg functions

* Created unit test for validating gds relative name

* Updated to fail when future gen

* Update arg parser

* Add escaping function for data set names

* Add unit tests for name escaping

* Remove calls to escape_data_set_name

* renamed tests

* Added MVSDataset class

* Updated escaped symbols

* Updated tests

* Added utils

* Add changelog

* Uncommented test

* Updated exception

* Updated mvsdataset class

* Updated class

* initial branch save with escaping added, changelog fragment rough-in
Also changed dataset to have a bool indicating if symbols can be included
  in get_tmp_ds_name

* corrected changelog fragment to include PR number in link
added import to zos_apf to get staticmethod loaded.

* added output to a failing test

* fixing pprint statement

* added escape function to get_tmp_dataset

* corrected staticmethod declaration

* change in volume finder to add all volume lists to the base value

* changing volume lister to handle potentially blank sub-lists

* adding protection in get_volumes for a short line of input

* added object definition to get_temp_ds_name

* added detail into test that is failing (finally back to test)

* removing potential import loop

* pulled from dev, then removed unneeded arg value.

* tweaking the get_volumes logic to handle partial lists.

* putting dataset reference back into tests/helpers

* re-commenting to see if that impacts loading of backup module

* added detail to make sure (looks like some commands actually break with escaping)

* sorting out the error details.  Will need to table what can't shouldn't be escaped:
something@something works, something \@something works, something\\@something fails

* temporarily disabled escaping in main zos_apf call

* fixed missing declaration issue

* cleanup of comments and trace notes

* changed fragment to minor_changes

* corrected get_tmp_ds for symbols=false
removed several print statements from test case.

* removed extra line between import statments - unneeded change.

* updated symbols generation for better distribution
updated credits for both developers involved.

* changed test of batch_add_del to not use symbols

* remove first ds name (library) symbol option in test_batch_add_del

* added prettyprint into failing assertion, re-activating symbols on first get_ds

* included output for test print

* removed escape of dsn in zos_apf/554

* removed print/pprint from testing
re-enabled symbol usage in second test wave

* removed a previously commented out line (removing escaping from batch call)

---------

Co-authored-by: Fernando Flores <[email protected]>
Co-authored-by: Ivan Moreno <[email protected]>

Author: 3 people

changelogs/fragments/1380-enhancement-add-sybols-zos_apf.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - zos_apf - Change input to auto-escape 'library' names containing symbols
+    (https://github.com/ansible-collections/ibm_zos_core/pull/1493).

plugins/modules/zos_apf.py

@@ -22,6 +22,8 @@
 version_added: '1.3.0'
 author:
   - "Behnam (@balkajbaf)"
+  - "Rich Parker (@richp405)"
+  - "Fernando Flores (@fernandofloresg))"
 short_description: Add or remove libraries to Authorized Program Facility (APF)
 description:
   - Adds or removes libraries to Authorized Program Facility (APF).
@@ -508,7 +510,8 @@ def main():
     except ValueError as err:
         module.fail_json(msg="Parameter verification failed", stderr=str(err))
 
-    library = parsed_args.get('library')
+    library = parsed_args.get("library")
+
     state = parsed_args.get('state')
     force_dynamic = parsed_args.get('force_dynamic')
     volume = parsed_args.get('volume')

tests/functional/modules/test_zos_apf_func.py

@@ -14,8 +14,7 @@
 from __future__ import absolute_import, division, print_function
 from ibm_zos_core.tests.helpers.dataset import get_tmp_ds_name
 from ibm_zos_core.tests.helpers.volumes import Volume_Handler
-from shellescape import quote
-from pprint import pprint
+from shlex import quote
 
 __metaclass__ = type
 
@@ -56,7 +55,7 @@ def test_add_del(ansible_zos_module, volumes_with_vvds):
         VolumeHandler = Volume_Handler(volumes_with_vvds)
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", state="present", force_dynamic=True)
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -93,7 +92,7 @@ def test_add_del_with_tmp_hlq_option(ansible_zos_module, volumes_with_vvds):
         tmphlq = "TMPHLQ"
         test_info = dict(library="", state="present", force_dynamic=True, tmp_hlq="", persistent=dict(data_set_name="", backup=True))
         test_info['tmp_hlq'] = tmphlq
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -129,7 +128,7 @@ def test_add_del_volume(ansible_zos_module, volumes_with_vvds):
         VolumeHandler = Volume_Handler(volumes_with_vvds)
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", volume="", state="present", force_dynamic=True)
-        ds = get_tmp_ds_name(1,1)
+        ds = get_tmp_ds_name(1,1,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -148,6 +147,7 @@ def test_add_del_volume(ansible_zos_module, volumes_with_vvds):
             hosts.all.shell(cmd=cmdStr)
             test_info['persistent']['data_set_name'] = prstds
         results = hosts.all.zos_apf(**test_info)
+
         for result in results.contacted.values():
             assert result.get("rc") == 0
         test_info['state'] = 'absent'
@@ -192,7 +192,7 @@ def test_add_del_volume_persist(ansible_zos_module, volumes_with_vvds):
         VolumeHandler = Volume_Handler(volumes_with_vvds)
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", volume="", persistent=dict(data_set_name="", marker="/* {mark} BLOCK */"), state="present", force_dynamic=True)
-        ds = get_tmp_ds_name(1,1)
+        ds = get_tmp_ds_name(1,1,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -251,20 +251,19 @@ def test_batch_add_del(ansible_zos_module, volumes_with_vvds):
             persistent=dict(data_set_name="", marker="/* {mark} BLOCK */"), state="present", force_dynamic=True
         )
         for item in test_info['batch']:
-            ds = get_tmp_ds_name(1,1)
+            ds = get_tmp_ds_name(1,1,True)
             hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
             item['library'] = ds
             cmdStr = "dls -l " + ds + " | awk '{print $5}' "
             results = hosts.all.shell(cmd=cmdStr)
             for result in results.contacted.values():
                 vol = result.get("stdout")
             item['volume'] = vol
-        prstds = get_tmp_ds_name(5,5)
+        prstds = get_tmp_ds_name(5,5,True)
         cmdStr = "dtouch -tseq {0}".format(prstds)
         hosts.all.shell(cmd=cmdStr)
         test_info['persistent']['data_set_name'] = prstds
         results = hosts.all.zos_apf(**test_info)
-        pprint(vars(results))
         for result in results.contacted.values():
             assert result.get("rc") == 0
         add_exptd = add_batch_expected.format(test_info['batch'][0]['library'], test_info['batch'][0]['volume'],
@@ -279,7 +278,6 @@ def test_batch_add_del(ansible_zos_module, volumes_with_vvds):
         assert actual == add_exptd
         test_info['state'] = 'absent'
         results = hosts.all.zos_apf(**test_info)
-        pprint(vars(results))
         for result in results.contacted.values():
             assert result.get("rc") == 0
         del_exptd = del_expected.replace(" ", "")
@@ -315,7 +313,7 @@ def test_operation_list_with_filter(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -357,7 +355,7 @@ def test_add_already_present(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -394,7 +392,7 @@ def test_del_not_present(ansible_zos_module, volumes_with_vvds):
         VolumeHandler = Volume_Handler(volumes_with_vvds)
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", state="present", force_dynamic=True)
-        ds = get_tmp_ds_name(1,1)
+        ds = get_tmp_ds_name(1,1,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -438,7 +436,7 @@ def test_add_with_wrong_volume(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", volume="", state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -472,7 +470,7 @@ def test_persist_invalid_ds_format(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", persistent=dict(data_set_name="", marker="/* {mark} BLOCK */"), state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -506,7 +504,7 @@ def test_persist_invalid_marker(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", persistent=dict(data_set_name="", marker="/* {mark} BLOCK */"), state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:
@@ -539,7 +537,7 @@ def test_persist_invalid_marker_len(ansible_zos_module, volumes_with_vvds):
         volume = VolumeHandler.get_available_vol()
         test_info = dict(library="", persistent=dict(data_set_name="", marker="/* {mark} BLOCK */"), state="present", force_dynamic=True)
         test_info['state'] = 'present'
-        ds = get_tmp_ds_name(3,2)
+        ds = get_tmp_ds_name(3,2,True)
         hosts.all.shell(cmd=f"dtouch -tseq -V{volume} {ds} ")
         test_info['library'] = ds
         if test_info.get('volume') is not None:

tests/helpers/dataset.py

@@ -75,4 +75,4 @@ def get_random_qs(size=7):
         random_char = random_q[random.choice(range(0, size))]
         random_q = random_q.replace(random_char, random.choice(special_chars))
         count += 1
-    return random_q
+    return random_q

tests/helpers/volumes.py

@@ -77,6 +77,7 @@ def get_volumes(ansible_zos_module, path):
     # is a instance of a class to manage the use.
     hosts = ansible_zos_module
     list_volumes = []
+    all_volumes_list = []
     storage_online = []
     flag = False
     iteration = 5
@@ -88,16 +89,19 @@ def get_volumes(ansible_zos_module, path):
         time.sleep(1)
         if all_volumes is not None:
             for volume in all_volumes.contacted.values():
-                all_volumes = volume.get('content')
-            flag = True if len(all_volumes) > 5 else False
+                temp = volume.get('content')
+                if temp is not None:
+                    all_volumes_list += temp
+            flag = True if len(all_volumes_list) > 5 else False
         iteration -= 1
     # Check if the volume is of storage and is active on prefer but also online as a correct option
-    for info in all_volumes:
+    for info in all_volumes_list:
         if "ACTIVATED" in info or "-D U," in info or "UNIT" in info:
             continue
         vol_w_info = info.split()
-        if vol_w_info[2] == 'O' and vol_w_info[4] == "STRG/RSDNT":
-            storage_online.append(vol_w_info[3])
+        if len(vol_w_info)>3:
+            if vol_w_info[2] == 'O' and vol_w_info[4] == "STRG/RSDNT":
+                storage_online.append(vol_w_info[3])
     # Insert a volumes for the class ls_Volumes to give flag of in_use and correct manage
     for vol in storage_online:
         list_volumes.append(vol)