From e6abc5a5b18da036cbcc6848032ac1682af76e7f Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Fri, 7 Apr 2017 11:32:07 -0700 Subject: [PATCH 1/4] fix #149: don't add umapi column unless needed. Turns out counting the umapis on the way out is harder than counting them on the way in. :) --- user_sync/rules.py | 49 +++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/user_sync/rules.py b/user_sync/rules.py index 18a566e50..64c06624b 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -900,32 +900,32 @@ def read_stray_key_map(self, file_path, delimiter = None): id_type_column_name = 'type' user_column_name = 'username' domain_column_name = 'domain' - org_name_column_name = 'umapi' + ummapi_name_column_name = 'umapi' rows = user_sync.helper.iter_csv_rows(file_path, delimiter = delimiter, recognized_column_names = [ id_type_column_name, user_column_name, domain_column_name, - org_name_column_name, + ummapi_name_column_name, ], logger = self.logger) for row in rows: - org_name = row.get(org_name_column_name) or PRIMARY_UMAPI_NAME + umapi_name = row.get(ummapi_name_column_name) or PRIMARY_UMAPI_NAME id_type = row.get(id_type_column_name) user = row.get(user_column_name) domain = row.get(domain_column_name) user_key = self.get_user_key(id_type, user, domain) if user_key: - self.add_stray(org_name, None) - self.add_stray(org_name, user_key) + self.add_stray(umapi_name, None) + self.add_stray(umapi_name, user_key) else: self.logger.error("Invalid input line, ignored: %s", row) user_count = len(self.get_stray_keys()) user_plural = "" if user_count == 1 else "s" - org_count = len(self.stray_key_map) - 1 - org_plural = "" if org_count == 1 else "s" - if org_count > 0: + secondary_count = len(self.stray_key_map) - 1 + if secondary_count > 0: + umapi_plural = "" if secondary_count == 1 else "s" self.logger.info('Read %d Adobe-only user%s for primary umapi, with %d secondary umapi%s', - user_count, user_plural, org_count, org_plural) + user_count, user_plural, secondary_count, umapi_plural) else: self.logger.info('Read %d Adobe-only user%s.', user_count, user_plural) @@ -933,26 +933,35 @@ def write_stray_key_map(self): file_path = self.stray_list_output_path logger = self.logger logger.info('Writing Adobe-only users to: %s', file_path) + # figure out if we should include a umapi column + secondary_count = 0 + fieldnames = ['type', 'username', 'domain'] + for umapi_name in self.stray_key_map: + if umapi_name != PRIMARY_UMAPI_NAME and self.get_stray_keys(umapi_name): + if not secondary_count: + fieldnames.append('umapi') + secondary_count += 1 with open(file_path, 'wb') as output_file: delimiter = user_sync.helper.guess_delimiter_from_filename(file_path) - writer = csv.DictWriter(output_file, - fieldnames = ['type', 'username', 'domain', 'umapi'], - delimiter = delimiter) + writer = csv.DictWriter(output_file, fieldnames=fieldnames, delimiter=delimiter) writer.writeheader() # None sorts before strings, so sorting the keys in the map # puts the primary umapi first in the output, which is handy - for org_name in sorted(self.stray_key_map.keys()): - for user_key in self.get_stray_keys(org_name): + for umapi_name in sorted(self.stray_key_map.keys()): + for user_key in self.get_stray_keys(umapi_name): id_type, username, domain = self.parse_user_key(user_key) - umapi = org_name if org_name else "" - writer.writerow({'type': id_type, 'username': username, 'domain': domain, 'umapi': umapi}) + umapi = umapi_name if umapi_name else "" + if secondary_count: + row_dict = {'type': id_type, 'username': username, 'domain': domain, 'umapi': umapi} + else: + row_dict = {'type': id_type, 'username': username, 'domain': domain} + writer.writerow(row_dict) user_count = len(self.stray_key_map.get(PRIMARY_UMAPI_NAME, [])) user_plural = "" if user_count == 1 else "s" - org_count = len(self.stray_key_map) - 1 - org_plural = "" if org_count == 1 else "s" - if org_count > 0: + if secondary_count > 0: + umapi_plural = "" if secondary_count == 1 else "s" logger.info('Wrote %d Adobe-only user%s for primary umapi, with %d secondary umapi%s', - user_count, user_plural, org_count, org_plural) + user_count, user_plural, secondary_count, umapi_plural) else: logger.info('Wrote %d Adobe-only user%s.', user_count, user_plural) From 731977f12688486a654ec946e569db6f68c8847b Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Fri, 7 Apr 2017 11:50:22 -0700 Subject: [PATCH 2/4] fix #151: don't report managing Adobe-only users when we haven't. Needed to distinguish writing output file from management. --- user_sync/rules.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/user_sync/rules.py b/user_sync/rules.py index 64c06624b..7c06311ff 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -99,12 +99,11 @@ def __init__(self, caller_options): self.read_stray_key_map(options['stray_list_input_path']) self.stray_list_output_path = options['stray_list_output_path'] - # determine whether we need to process strays at all - self.will_process_strays = (not options['exclude_strays']) and (options['manage_groups'] or - options['stray_list_output_path'] or - options['disentitle_strays'] or - options['remove_strays'] or - options['delete_strays']) + # determine what processing is needed on strays + self.will_manage_strays = (options['manage_groups'] or options['disentitle_strays'] or + options['remove_strays'] or options['delete_strays']) + self.will_process_strays = (not options['exclude_strays']) and (options['stray_list_output_path'] or + self.will_manage_strays) # in/out variables for per-user after-mapping-hook code self.after_mapping_hook_scope = { @@ -193,7 +192,7 @@ def log_action_summary(self, umapi_connectors): ['adobe_users_excluded', 'Number of Adobe users excluded from updates'], ['adobe_users_unchanged', 'Number of non-excluded Adobe users with no changes'], ['adobe_users_created', 'Number of new Adobe users added'], - ['adobe_users_updated', 'Number of existing Adobe users updated'], + ['adobe_users_updated', 'Number of matching Adobe users updated'], ] if self.will_process_strays: if self.options['delete_strays']: @@ -204,7 +203,7 @@ def log_action_summary(self, umapi_connectors): action = 'removed from all groups' else: action = 'with groups processed' - action_summary_description.append(['adobe_strays_processed', 'Number of Adobe-only users ' + action + ':']) + action_summary_description.append(['adobe_strays_processed', 'Number of Adobe-only users ' + action]) # prepare the network summary umapi_summary_format = 'Number of%s%s UMAPI actions sent (total, success, error)' @@ -436,15 +435,16 @@ def process_strays(self, umapi_connectors): stray_count = len(self.get_stray_keys()) if self.stray_list_output_path: self.write_stray_key_map() - max_missing = self.options['max_adobe_only_users'] - if stray_count > max_missing: - self.logger.critical('Unable to process Adobe-only users, as their count (%s) is larger ' - 'than the max_adobe_only_users setting (%d)', stray_count, max_missing) - self.action_summary['adobe_strays_processed'] = 0 - return - self.action_summary['adobe_strays_processed'] = stray_count - self.logger.debug("Processing Adobe-only users...") - self.manage_strays(umapi_connectors) + if self.will_manage_strays: + max_missing = self.options['max_adobe_only_users'] + if stray_count > max_missing: + self.logger.critical('Unable to process Adobe-only users, as their count (%s) is larger ' + 'than the max_adobe_only_users setting (%d)', stray_count, max_missing) + self.action_summary['adobe_strays_processed'] = 0 + return + self.action_summary['adobe_strays_processed'] = stray_count + self.logger.debug("Processing Adobe-only users...") + self.manage_strays(umapi_connectors) def manage_strays(self, umapi_connectors): ''' From c8691d4c1985dd5452ef944ff79b4dcebee2385f Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Fri, 7 Apr 2017 12:07:22 -0700 Subject: [PATCH 3/4] fix #150: correct count of updated matching Adobe users. Needed to count user keys updated in any of multiple umapis. --- user_sync/rules.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/user_sync/rules.py b/user_sync/rules.py index 7c06311ff..7b81b662c 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -89,9 +89,12 @@ def __init__(self, caller_options): # of primary-umapi users, who are presumed to be in primary-umapi domains. # So instead of keeping track of excluded users in the primary umapi, # we keep track of included users, so we can match them against users - # in the secondary umapis (and exclude all that don't match). + # in the secondary umapis (and exclude all that don't match). Finally, + # we keep track of user keys (in any umapi) that we have updated, so + # we can correctly report their count. self.included_user_keys = set() self.excluded_user_count = 0 + self.updated_user_keys = set() # stray key input path comes in, stray_list_output_path goes out self.stray_key_map = self.make_stray_key_map() @@ -170,6 +173,7 @@ def log_action_summary(self, umapi_connectors): # find the total number of adobe users and excluded users self.action_summary['adobe_users_read'] = len(self.included_user_keys) + self.excluded_user_count self.action_summary['adobe_users_excluded'] = self.excluded_user_count + self.action_summary['adobe_users_updated'] = len(self.updated_user_keys) # find out the number of users that have no changes; this depends on whether # we actually read the directory or read an input file. So there are two cases: if self.action_summary['adobe_users_read'] == 0: @@ -627,7 +631,7 @@ def update_umapi_user(self, umapi_info, user_key, umapi_connector, ''' is_primary_org = umapi_info.get_name() == PRIMARY_UMAPI_NAME if attributes_to_update or groups_to_add or groups_to_remove: - self.action_summary['adobe_users_updated'] += 1 if is_primary_org else 0 + self.updated_user_keys.add(user_key) if attributes_to_update: self.logger.info('Updating info for user key: %s changes: %s', user_key, attributes_to_update) if groups_to_add or groups_to_remove: From e706921bcd3f28249729e08813e6750865dfbfa5 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Fri, 7 Apr 2017 12:20:21 -0700 Subject: [PATCH 4/4] fix #153: indicate in Action Summary when Test Mode is in use. This required passing the test-mode flag to the Rule Processor options. --- user_sync/config.py | 1 + user_sync/rules.py | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/user_sync/config.py b/user_sync/config.py index f563bc617..a9d290078 100644 --- a/user_sync/config.py +++ b/user_sync/config.py @@ -348,6 +348,7 @@ def get_rule_options(self): 'remove_strays': options['remove_strays'], 'stray_list_input_path': options['stray_list_input_path'], 'stray_list_output_path': options['stray_list_output_path'], + 'test_mode': options['test_mode'], 'update_user_info': options['update_user_info'], 'username_filter_regex': options['username_filter_regex'], } diff --git a/user_sync/rules.py b/user_sync/rules.py index 7b81b662c..2fafa450c 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -54,6 +54,7 @@ def __init__(self, caller_options): 'remove_strays': False, 'stray_list_input_path': None, 'stray_list_output_path': None, + 'test_mode': False, 'update_user_info': True, 'username_filter_regex': None, } @@ -185,7 +186,11 @@ def log_action_summary(self, umapi_connectors): self.action_summary['adobe_users_updated'] - self.action_summary['adobe_strays_processed'] ) - logger.info('---------------------------------- Action Summary ----------------------------------') + if self.options['test_mode']: + header = '- Action Summary (TEST MODE) -' + else: + header = '------- Action Summary -------' + logger.info('---------------------------' + header + '---------------------------') # English text description for action summary log. # The action summary will be shown the same order as they are defined in this list