From 8c735ef9c0acb6f0da1f3828394ab1c713449100 Mon Sep 17 00:00:00 2001 From: Alan King Date: Thu, 16 Mar 2023 14:21:32 -0400 Subject: [PATCH] [#7000] Fix tests for running in topology Test_AllRules: Use zonereport to detect database Test_AllRules skips one test based on the database type. This is derived from the local server's server_config.json file. Catalog consumers do not have the database configuration in their server_config.json files, so this fails when running topology tests on catalog consumers. This change uses izonereport to find the database type in the catalog provider entry of the zonereport. This should work no matter which server in the zone is running the tests. --- msiExec PEP test should use local resource test_rulebase.Test_Rulebase.test_msiExecCmd_closeAllL1Desc__issue_6623 implements a resource PEP but the data object being opened only has a replica which is on a resource tied to a different computer, so the PEP fires over there (and is, of course, not implemented anywhere but the local machine). This change makes a local resource and puts the data object's only replica there so that the resource PEP will fire. --- Avoid log test in test_acPreProcForExecCmd__3867 Use metadata on a temporary resource instead because the logs are unreliable. --- Skip msiDeleteUnusedAVUs for consumer topology tests msiDeleteUnusedAVUs does not redirect to the catalog provider, so this microservice will always fail when not run from the catalog provider. --- Fix hostname in downgrade service account admin test The test for downgrading the service account rodsadmin account needs to determine the expected hostname by fetching it from the zone report just like rsGeneralAdmin. Apparently, the order of the entries is not consistent across test runs but is consistent in a given server's lifetime. --- Fix recursive ireg tests expecting success Two recursive ireg tests were expecting success when registering files to a resource on a machine on which those files do not exist. A local resource is now created and targeted for registration. --- Skip test_pam_password_authentication in topology Configuring SSL is part of the test in order to ensure that the test is run regularly for normal testing. This cannot be done for other servers in the topology from inside the test (must be done by the testing environment before running the test). Skip the test if run in topology since the topology cannot be configured properly to run it. --- Change expected error in msiRemoveUserFromGroup test If the test is being run from a catalog consumer, the privilege level of the authenticated user is checked before redirecting to the catalog provider and returns an error. When run on the catalog provider, the database operation itself will return the error. --- scripts/irods/lib.py | 8 +++ scripts/irods/test/test_all_rules.py | 17 +++-- scripts/irods/test/test_iadmin.py | 10 ++- scripts/irods/test/test_ireg.py | 58 +++++++++++------ .../test/test_pam_password_authentication.py | 1 + scripts/irods/test/test_rulebase.py | 64 +++++++++++++------ 6 files changed, 110 insertions(+), 48 deletions(-) diff --git a/scripts/irods/lib.py b/scripts/irods/lib.py index f24f1e7da8..1e01be7a33 100644 --- a/scripts/irods/lib.py +++ b/scripts/irods/lib.py @@ -774,3 +774,11 @@ def get_resource_parent_name(session, resource_name): return session.run_icommand(['iquest', '%s', "select RESC_NAME where RESC_ID = '{}'" .format(parent_id)])[0].strip() + +def get_database_instance_name(session): + servers = json.loads(session.assert_icommand(['izonereport'], 'STDOUT')[1])['zones'][0]['servers'] + + for server in servers: + server_config = server['server_config'] + if server_config['catalog_service_role'] == 'provider': + return next(iter(server_config['plugin_configuration']['database'])) diff --git a/scripts/irods/test/test_all_rules.py b/scripts/irods/test/test_all_rules.py index a2a8cb154b..c66361483f 100644 --- a/scripts/irods/test/test_all_rules.py +++ b/scripts/irods/test/test_all_rules.py @@ -23,6 +23,7 @@ from . import resource_suite from . import session + @six.add_metaclass(metaclass_unittest_test_case_generator.MetaclassUnittestTestCaseGenerator) class Test_AllRules(resource_suite.ResourceBase, unittest.TestCase): @@ -31,11 +32,10 @@ class Test_AllRules(resource_suite.ResourceBase, unittest.TestCase): global plugin_name plugin_name = IrodsConfig().default_rule_engine_plugin + # Get the database instance name from the zone report. Test_AllRules does not and shall not run in federation, so + # this should be good enough for single machine tests and topology tests for both provider and consumer. global database_instance_name - if test.settings.TOPOLOGY_FROM_RESOURCE_SERVER: - database_instance_name = 'none' - else: - database_instance_name = next(iter(IrodsConfig().server_config['plugin_configuration']['database'])) + database_instance_name = lib.get_database_instance_name(session.make_session_for_existing_admin()) global rulesdir currentdir = os.path.dirname(os.path.realpath(__file__)) @@ -1624,7 +1624,14 @@ def issue_7165(_, callback, _): # Show the MSI returns an error when the user is not a rodsadmin. rule = rule_map[plugin_name].format(group, self.user0.username, self.user0.zone_name) - self.user0.assert_icommand(['irule', '-r', rep_instance, rule, 'null', 'ruleExecOut'], 'STDERR', ['-830000 CAT_INSUFFICIENT_PRIVILEGE_LEVEL']) + # The returned error string changes depending on where the the rule is running. The API being invoked by the + # microservice does not perform the permission check until after it has redirected to the appropriate server + # so the API privilege level will trip for the general admin API before getting to the database plugin. + if test.settings.TOPOLOGY_FROM_RESOURCE_SERVER: + expected_error_str = '-13000 SYS_NO_API_PRIV' + else: + expected_error_str = '-830000 CAT_INSUFFICIENT_PRIVILEGE_LEVEL' + self.user0.assert_icommand(['irule', '-r', rep_instance, rule, 'null', 'ruleExecOut'], 'STDERR', [expected_error_str]) # Show the MSI returns an error when the group does not exist. rule = rule_map[plugin_name].format('does_not_exist', self.user0.username, self.user0.zone_name) diff --git a/scripts/irods/test/test_iadmin.py b/scripts/irods/test/test_iadmin.py index 90b8a0b6d7..59d9725763 100644 --- a/scripts/irods/test/test_iadmin.py +++ b/scripts/irods/test/test_iadmin.py @@ -44,7 +44,7 @@ def tearDown(self): # iadmin ################### - @unittest.skipIf(test.settings.TOPOLOGY_FROM_RESOURCE_SERVER, "msiDeleteUnusedAVUs does not redirect and remote() does not return errors") + @unittest.skipIf(test.settings.TOPOLOGY_FROM_RESOURCE_SERVER, 'msiDeleteUnusedAVUs does not redirect appropriately. See #6989.') @unittest.skipIf(plugin_name == 'irods_rule_engine_plugin-python', 'Applies to the NREP only') def test_non_admins_are_not_allowed_to_delete_unused_metadata__issue_6183(self): attr_name = 'issue_6183_attr' @@ -2197,9 +2197,15 @@ def test_moduser_type_invalid_type(self): def test_downgrade_of_service_account_user_is_not_allowed__issue_6127(self): """Test downgrading of service account user's type from rodsadmin to other supported types is not allowed.""" + def get_first_hostname_from_zone_report(): + """Get the hostname of the first "servers" entry of the zone report.""" + zr = json.loads(self.admin.assert_icommand(['izonereport'], 'STDOUT')[1].strip()) + return zr['zones'][0]['servers'][0]['service_account_environment']['irods_host'] + # rodsadmin -> rodsuser self.assertEqual('rodsadmin', lib.get_user_type(self.admin, 'rods')) - error_msg = f'Cannot downgrade another rodsadmin [rods] running another server [{lib.get_hostname()}] in this zone.' + hostname = get_first_hostname_from_zone_report() + error_msg = f'Cannot downgrade another rodsadmin [rods] running another server [{hostname}] in this zone.' out, err, ec = self.admin.run_icommand(['iadmin', 'moduser', 'rods', 'type', 'rodsuser']) self.assertNotEqual(ec, 0) self.assertIn(error_msg, out) diff --git a/scripts/irods/test/test_ireg.py b/scripts/irods/test/test_ireg.py index 9bc365438a..88568f58a4 100644 --- a/scripts/irods/test/test_ireg.py +++ b/scripts/irods/test/test_ireg.py @@ -559,42 +559,58 @@ def test_ireg_directory_with_no_option(self): self.admin.assert_icommand_fail(['ils', '-l', coll], 'STDOUT', coll) - def test_ireg_recursive_C__issue_2912(self): + def test_ireg_recursive_C__issue_2919(self): reg_path = os.path.join(self.admin.local_session_dir, 'regme') coll = os.path.join(self.admin.session_collection, 'reghere') + local_resource = 'test_ireg_recursive_C__issue_2919_resc' - self.admin.assert_icommand_fail(['ils', '-l', coll], 'STDOUT', coll) + try: + lib.create_ufs_resource(self.admin, local_resource) - lib.make_deep_local_tmp_dir(reg_path) + self.admin.assert_icommand_fail(['ils', '-l', coll], 'STDOUT', coll) - out,err,rc = self.admin.run_icommand(['ireg', '-C', reg_path, coll]) - self.assertEqual(rc, 0) - self.assertEqual(len(err), 0) - self.assertIn('-C option is deprecated. Use -r instead.', out) + lib.make_deep_local_tmp_dir(reg_path) - _,out,_ = self.admin.assert_icommand(['ils', '-lr', coll], 'STDOUT', coll) + out,err,rc = self.admin.run_icommand(['ireg', '-R', local_resource, '-C', reg_path, coll]) + self.assertEqual(rc, 0) + self.assertEqual(len(err), 0) + self.assertIn('-C option is deprecated. Use -r instead.', out) - for f in lib.files_in_dir(reg_path): - self.assertIn(f, out) + _,out,_ = self.admin.assert_icommand(['ils', '-lr', coll], 'STDOUT', coll) - for d in lib.dirs_in_dir(reg_path): - self.assertIn(d, out) + for f in lib.files_in_dir(reg_path): + self.assertIn(f, out) + + for d in lib.dirs_in_dir(reg_path): + self.assertIn(d, out) + + finally: + self.admin.run_icommand(['iunreg', '-r', coll]) + lib.remove_resource(self.admin, local_resource) - def test_ireg_recursive_r__issue_2912(self): + def test_ireg_recursive_r__issue_2919(self): reg_path = os.path.join(self.admin.local_session_dir, 'regme') coll = os.path.join(self.admin.session_collection, 'reghere') + local_resource = 'test_ireg_recursive_C__issue_2919_resc' - self.admin.assert_icommand_fail(['ils', '-l', coll], 'STDOUT', coll) + try: + lib.create_ufs_resource(self.admin, local_resource) - lib.make_deep_local_tmp_dir(reg_path) + self.admin.assert_icommand_fail(['ils', '-l', coll], 'STDOUT', coll) - self.admin.assert_icommand(['ireg', '-r', reg_path, coll]) + lib.make_deep_local_tmp_dir(reg_path) - _,out,_ = self.admin.assert_icommand(['ils', '-lr', coll], 'STDOUT', coll) + self.admin.assert_icommand(['ireg', '-R', local_resource, '-r', reg_path, coll]) - for f in lib.files_in_dir(reg_path): - self.assertIn(f, out) + _,out,_ = self.admin.assert_icommand(['ils', '-lr', coll], 'STDOUT', coll) - for d in lib.dirs_in_dir(reg_path): - self.assertIn(d, out) + for f in lib.files_in_dir(reg_path): + self.assertIn(f, out) + + for d in lib.dirs_in_dir(reg_path): + self.assertIn(d, out) + + finally: + self.admin.run_icommand(['iunreg', '-r', coll]) + lib.remove_resource(self.admin, local_resource) diff --git a/scripts/irods/test/test_pam_password_authentication.py b/scripts/irods/test/test_pam_password_authentication.py index 4ccd6740db..9fd37064e0 100644 --- a/scripts/irods/test/test_pam_password_authentication.py +++ b/scripts/irods/test/test_pam_password_authentication.py @@ -13,6 +13,7 @@ @unittest.skipIf(test.settings.USE_SSL, 'SSL is set up in these tests, so just skip if SSL is enabled already.') +@unittest.skipIf(test.settings.RUN_IN_TOPOLOGY, 'SSL configuration cannot be applied to all servers from the tests.') class test_configurations(unittest.TestCase): plugin_name = IrodsConfig().default_rule_engine_plugin diff --git a/scripts/irods/test/test_rulebase.py b/scripts/irods/test/test_rulebase.py index 7ffde36068..269c7f1b0b 100644 --- a/scripts/irods/test/test_rulebase.py +++ b/scripts/irods/test/test_rulebase.py @@ -513,29 +513,50 @@ def test_rulebase_update_sixty_four_chars__3560(self): @unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'only applicable for irods_rule_language REP') def test_acPreProcForExecCmd__3867(self): - with temporary_core_file() as core: - core.add_rule('acPreProcForExecCmd(*cmd, *args, *addr, *hint){ writeLine("serverLog", "TEST_MARKER_test_acPreProcForExecCmd__3867")}') + pep_map = { + 'irods_rule_engine_plugin-irods_rule_language': textwrap.dedent( + ''' + acPreProcForExecCmd(*cmd, *args, *addr, *hint) {{ + msiAddKeyVal(*key_val_pair,"{}","{}"); + msiAssociateKeyValuePairsToObj(*key_val_pair,"{}","-R"); + }}''') + } - rule_file = 'test_acPreProcForExecCmd__3867.r' - rule_string = ''' -test_acPreProcForExecCmd__3867_rule { - msiExecCmd('hello', '', '', '', '', *out) -} + rule_map = { + 'irods_rule_engine_plugin-irods_rule_language': textwrap.dedent( + ''' + test_acPreProcForExecCmd__3867_rule {{ + msiExecCmd('hello', '', '', '', '', *out) + }} -INPUT null -OUTPUT ruleExecOut -''' - with open(rule_file, 'w') as f: - f.write(rule_string) + INPUT null + OUTPUT ruleExecOut''') + } - initial_log_size = lib.get_file_size_by_path(paths.server_log_path()) - self.admin.assert_icommand('irule -F ' + rule_file) - os.unlink(rule_file) + temporary_resource = 'test_acPreProcForExecCmd__3867_resc' + metadata_attr = 'TEST_MARKER_test_acPreProcForExecCmd__3867' + metadata_value = 'wow, static PEPs still work' + + rule_file = os.path.join(self.admin.local_session_dir, 'test_acPreProcForExecCmd__3867.r') + rule_string = rule_map[self.plugin_name] + + try: + lib.create_passthru_resource(self.admin, temporary_resource) + + with temporary_core_file() as core: + core.add_rule(pep_map[self.plugin_name].format(metadata_attr, metadata_value, temporary_resource)) + + with open(rule_file, 'w') as f: + f.write(rule_string) + + self.admin.assert_icommand(['irule', '-F', rule_file]) + + lib.delayAssert(lambda: lib.metadata_attr_with_value_exists(self.admin, metadata_attr, metadata_value)) + + finally: + lib.remove_resource(self.admin, temporary_resource) + self.admin.run_icommand(['iadmin', 'rum']) - lib.delayAssert( - lambda: lib.log_message_occurrences_equals_count( - msg='TEST_MARKER_test_acPreProcForExecCmd__3867', - start_index=initial_log_size)) @unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'only run for native rule language') def test_create_close__issue_5018(self): @@ -576,6 +597,7 @@ def main(rule_args, callback, rei): @unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'only applicable for irods_rule_language REP') def test_msiExecCmd_closeAllL1Desc__issue_6623(self): + local_resource = 'localresc' logical_path = os.path.join(self.admin.session_collection, 'foo') attr = 'test_msiExecCmd_closeAllL1Desc__issue_6623' @@ -605,8 +627,9 @@ def test_msiExecCmd_closeAllL1Desc__issue_6623(self): self.assertFalse(lib.replica_exists(self.admin, logical_path, 0)) try: + lib.create_ufs_resource(self.admin, local_resource) with temporary_core_file() as core: - self.admin.assert_icommand(['itouch', logical_path]) + self.admin.assert_icommand(['itouch', '-R', local_resource, logical_path]) self.assertTrue(lib.replica_exists(self.admin, logical_path, 0)) # Add a PEP which fires after the "close" resource operation. The PEP adds an AVU to the object at logical_path. @@ -629,6 +652,7 @@ def test_msiExecCmd_closeAllL1Desc__issue_6623(self): finally: self.admin.run_icommand(['irm', '-f', logical_path]) self.admin.run_icommand(['iadmin', 'rum']) + lib.remove_resource(self.admin, local_resource) @unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'Only implemented for NREP.')