From 44f9bf545d2f2aaa2ecf80f4092c25e6a20d12ac Mon Sep 17 00:00:00 2001 From: Sebastian Guarino Date: Fri, 18 Feb 2022 20:58:38 +0100 Subject: [PATCH] Rework of gitlab_project_variable over gitlab_group_variable (#4086) * Rework of gitlab_project_variable over gitlab_group_variable * Linting and removed unused example * Fix test 2 * Sync from review of gitlab_project_variable #4038 * Linting, default protected True, value optional * Next version is 4.5.0 * Roll back protected default true, and value not required * Apply suggestions from code review Missing check_mode Co-authored-by: Markus Bergholz * Fix one unit test, comment test that requires premium gitlab * Add changelog * Update plugins/modules/source_control/gitlab/gitlab_group_variable.py Co-authored-by: Felix Fontein * Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml Co-authored-by: Felix Fontein * Added conditional gitlab_premium_tests variable when required * Allow delete without value * Fix variable name * Linting * Value should not be required in doc * Linting missing new-line * Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml Co-authored-by: Markus Bergholz Co-authored-by: Markus Bergholz Co-authored-by: Felix Fontein --- ...ct_variable_over_gitlab_group_variable.yml | 9 + .../gitlab/gitlab_group_variable.py | 340 +++++++++++++----- .../gitlab_group_variable/tasks/main.yml | 126 ++++++- 3 files changed, 378 insertions(+), 97 deletions(-) create mode 100644 changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml diff --git a/changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml b/changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml new file mode 100644 index 0000000000..0322b2d5fa --- /dev/null +++ b/changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml @@ -0,0 +1,9 @@ +bugfixes: + - > + gitlab_group_variable - allow to set same variable name under different environment scopes. + Due this change, the return value ``group_variable`` differs from previous version in check mode. + It was counting ``updated`` values, because it was accidentally overwriting environment scopes (https://github.com/ansible-collections/community.general/pull/4038). + - gitlab_group_variable - fix idempotent change behaviour for float and integer variables (https://github.com/ansible-collections/community.general/pull/4038). + - gitlab_group_variable - add missing documentation about GitLab versions that support ``environment_scope`` and ``variable_type`` (https://github.com/ansible-collections/community.general/pull/4038). +minor_changes: + - gitlab_group_variable - new ``variables`` parameter (https://github.com/ansible-collections/community.general/pull/4038 and https://github.com/ansible-collections/community.general/issues/4074). diff --git a/plugins/modules/source_control/gitlab/gitlab_group_variable.py b/plugins/modules/source_control/gitlab/gitlab_group_variable.py index 9e31562b54..9be3a3ab39 100644 --- a/plugins/modules/source_control/gitlab/gitlab_group_variable.py +++ b/plugins/modules/source_control/gitlab/gitlab_group_variable.py @@ -48,6 +48,8 @@ options: - When the list element is a simple key-value pair, set masked and protected to false. - When the list element is a dict with the keys I(value), I(masked) and I(protected), the user can have full control about whether a value should be masked, protected or both. + - Support for group variables requires GitLab >= 9.5. + - Support for environment_scope requires GitLab Premium >= 13.11. - Support for protected values requires GitLab >= 9.3. - Support for masked values requires GitLab >= 11.10. - A I(value) must be a string or a number. @@ -56,6 +58,46 @@ options: See GitLab documentation on acceptable values for a masked variable (U(https://docs.gitlab.com/ce/ci/variables/#masked-variables)). default: {} type: dict + variables: + version_added: 4.5.0 + description: + - A list of dictionaries that represents CI/CD variables. + - This modules works internal with this sructure, even if the older I(vars) parameter is used. + default: [] + type: list + elements: dict + suboptions: + name: + description: + - The name of the variable. + type: str + required: true + value: + description: + - The variable value. + - Required when I(state=present). + type: str + masked: + description: + - Wether variable value is masked or not. + type: bool + default: false + protected: + description: + - Wether variable value is protected or not. + type: bool + default: false + variable_type: + description: + - Wether a variable is an environment variable (C(env_var)) or a file (C(file)). + type: str + choices: [ "env_var", "file" ] + default: env_var + environment_scope: + description: + - The scope for the variable. + type: str + default: '*' notes: - Supports I(check_mode). ''' @@ -68,23 +110,15 @@ EXAMPLES = r''' api_token: secret_access_token group: scodeman/testgroup/ purge: false - vars: - ACCESS_KEY_ID: abc123 - SECRET_ACCESS_KEY: 321cba - -- name: Set or update some CI/CD variables - community.general.gitlab_group_variable: - api_url: https://gitlab.com - api_token: secret_access_token - group: scodeman/testgroup/ - purge: false - vars: - ACCESS_KEY_ID: abc123 - SECRET_ACCESS_KEY: + variables: + - name: ACCESS_KEY_ID + value: abc123 + - name: SECRET_ACCESS_KEY value: 3214cbad masked: true protected: true variable_type: env_var + environment_scope: production - name: Delete one variable community.general.gitlab_group_variable: @@ -125,13 +159,11 @@ group_variable: ''' import traceback - from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils.api import basic_auth_argument_spec from ansible.module_utils.six import string_types from ansible.module_utils.six import integer_types - GITLAB_IMP_ERR = None try: import gitlab @@ -143,6 +175,44 @@ except Exception: from ansible_collections.community.general.plugins.module_utils.gitlab import auth_argument_spec, gitlab_authentication +def vars_to_variables(vars, module): + # transform old vars to new variables structure + variables = list() + for item, value in vars.items(): + if (isinstance(value, string_types) or + isinstance(value, (integer_types, float))): + variables.append( + { + "name": item, + "value": str(value), + "masked": False, + "protected": False, + "variable_type": "env_var", + } + ) + + elif isinstance(value, dict): + new_item = {"name": item, "value": value.get('value')} + + new_item = { + "name": item, + "value": value.get('value'), + "masked": value.get('masked'), + "protected": value.get('protected'), + "variable_type": value.get('variable_type'), + } + + if value.get('environment_scope'): + new_item['environment_scope'] = value.get('environment_scope') + + variables.append(new_item) + + else: + module.fail_json(msg="value must be of type string, integer, float or dict") + + return variables + + class GitlabGroupVariables(object): def __init__(self, module, gitlab_instance): @@ -163,103 +233,150 @@ class GitlabGroupVariables(object): vars_page = self.group.variables.list(page=page_nb) return variables - def create_variable(self, key, value, masked, protected, variable_type): - if self._module.check_mode: - return - return self.group.variables.create({ - "key": key, - "value": value, - "masked": masked, - "protected": protected, - "variable_type": variable_type, - }) - - def update_variable(self, key, var, value, masked, protected, variable_type): - if var.value == value and var.protected == protected and var.masked == masked and var.variable_type == variable_type: - return False - + def create_variable(self, var_obj): if self._module.check_mode: return True + var = { + "key": var_obj.get('key'), + "value": var_obj.get('value'), + "masked": var_obj.get('masked'), + "protected": var_obj.get('protected'), + "variable_type": var_obj.get('variable_type'), + } + if var_obj.get('environment_scope') is not None: + var["environment_scope"] = var_obj.get('environment_scope') - if var.protected == protected and var.masked == masked and var.variable_type == variable_type: - var.value = value - var.save() - return True - - self.delete_variable(key) - self.create_variable(key, value, masked, protected, variable_type) + self.group.variables.create(var) return True - def delete_variable(self, key): + def update_variable(self, var_obj): if self._module.check_mode: - return - return self.group.variables.delete(key) + return True + self.delete_variable(var_obj) + self.create_variable(var_obj) + return True + + def delete_variable(self, var_obj): + if self._module.check_mode: + return True + self.group.variables.delete(var_obj.get('key'), filter={'environment_scope': var_obj.get('environment_scope')}) + return True -def native_python_main(this_gitlab, purge, var_list, state, module): +def compare(requested_variables, existing_variables, state): + # we need to do this, because it was determined in a previous version - more or less buggy + # basically it is not necessary and might results in more/other bugs! + # but it is required and only relevant for check mode!! + # logic represents state 'present' when not purge. all other can be derived from that + # untouched => equal in both + # updated => name and scope are equal + # added => name and scope does not exist + untouched = list() + updated = list() + added = list() + + if state == 'present': + existing_key_scope_vars = list() + for item in existing_variables: + existing_key_scope_vars.append({'key': item.get('key'), 'environment_scope': item.get('environment_scope')}) + + for var in requested_variables: + if var in existing_variables: + untouched.append(var) + else: + compare_item = {'key': var.get('name'), 'environment_scope': var.get('environment_scope')} + if compare_item in existing_key_scope_vars: + updated.append(var) + else: + added.append(var) + + return untouched, updated, added + + +def native_python_main(this_gitlab, purge, requested_variables, state, module): change = False return_value = dict(added=list(), updated=list(), removed=list(), untouched=list()) gitlab_keys = this_gitlab.list_all_group_variables() - existing_variables = [x.get_id() for x in gitlab_keys] + before = [x.attributes for x in gitlab_keys] - for key in var_list: - if not isinstance(var_list[key], (string_types, integer_types, float, dict)): - module.fail_json(msg="Value of %s variable must be of type string, integer, float or dict, passed %s" % (key, var_list[key].__class__.__name__)) + gitlab_keys = this_gitlab.list_all_group_variables() + existing_variables = [x.attributes for x in gitlab_keys] - for key in var_list: + # preprocessing:filter out and enrich before compare + for item in existing_variables: + item.pop('group_id') - if isinstance(var_list[key], (string_types, integer_types, float)): - value = var_list[key] - masked = False - protected = False - variable_type = 'env_var' - elif isinstance(var_list[key], dict): - value = var_list[key].get('value') - masked = var_list[key].get('masked', False) - protected = var_list[key].get('protected', False) - variable_type = var_list[key].get('variable_type', 'env_var') + for item in requested_variables: + item['key'] = item.pop('name') + item['value'] = str(item.get('value')) + if item.get('protected') is None: + item['protected'] = False + if item.get('masked') is None: + item['masked'] = False + if item.get('environment_scope') is None: + item['environment_scope'] = '*' + if item.get('variable_type') is None: + item['variable_type'] = 'env_var' - if key in existing_variables: - index = existing_variables.index(key) - existing_variables[index] = None + if module.check_mode: + untouched, updated, added = compare(requested_variables, existing_variables, state) - if state == 'present': - single_change = this_gitlab.update_variable( - key, - gitlab_keys[index], - value, - masked, - protected, - variable_type, - ) - change = single_change or change - if single_change: - return_value['updated'].append(key) - else: - return_value['untouched'].append(key) + if state == 'present': + add_or_update = [x for x in requested_variables if x not in existing_variables] + for item in add_or_update: + try: + if this_gitlab.create_variable(item): + return_value['added'].append(item) - elif state == 'absent': - this_gitlab.delete_variable(key) - change = True - return_value['removed'].append(key) + except Exception: + if this_gitlab.update_variable(item): + return_value['updated'].append(item) - elif key not in existing_variables and state == 'present': - this_gitlab.create_variable(key, value, masked, protected, variable_type) - change = True - return_value['added'].append(key) + if purge: + # refetch and filter + gitlab_keys = this_gitlab.list_all_group_variables() + existing_variables = [x.attributes for x in gitlab_keys] + for item in existing_variables: + item.pop('group_id') - existing_variables = list(filter(None, existing_variables)) - if purge: + remove = [x for x in existing_variables if x not in requested_variables] + for item in remove: + if this_gitlab.delete_variable(item): + return_value['removed'].append(item) + + elif state == 'absent': + # value does not matter on removing variables. + # key and environment scope are sufficient for item in existing_variables: - this_gitlab.delete_variable(item) - change = True - return_value['removed'].append(item) - else: - return_value['untouched'].extend(existing_variables) + item.pop('value') + item.pop('variable_type') + for item in requested_variables: + item.pop('value') + item.pop('variable_type') - return change, return_value + if not purge: + remove_requested = [x for x in requested_variables if x in existing_variables] + for item in remove_requested: + if this_gitlab.delete_variable(item): + return_value['removed'].append(item) + + else: + for item in existing_variables: + if this_gitlab.delete_variable(item): + return_value['removed'].append(item) + + if module.check_mode: + return_value = dict(added=added, updated=updated, removed=return_value['removed'], untouched=untouched) + + if len(return_value['added'] + return_value['removed'] + return_value['updated']) > 0: + change = True + + gitlab_keys = this_gitlab.list_all_group_variables() + after = [x.attributes for x in gitlab_keys] + + return change, return_value, before, after def main(): @@ -269,7 +386,15 @@ def main(): group=dict(type='str', required=True), purge=dict(type='bool', required=False, default=False), vars=dict(type='dict', required=False, default=dict(), no_log=True), - state=dict(type='str', default="present", choices=["absent", "present"]) + variables=dict(type='list', elements='dict', required=False, default=list(), options=dict( + name=dict(type='str', required=True), + value=dict(type='str', no_log=True), + masked=dict(type='bool', default=False), + protected=dict(type='bool', default=False), + environment_scope=dict(type='str', default='*'), + variable_type=dict(type='str', default='env_var', choices=["env_var", "file"]) + )), + state=dict(type='str', default="present", choices=["absent", "present"]), ) module = AnsibleModule( @@ -280,6 +405,7 @@ def main(): ['api_username', 'api_job_token'], ['api_token', 'api_oauth_token'], ['api_token', 'api_job_token'], + ['vars', 'variables'], ], required_together=[ ['api_username', 'api_password'], @@ -290,18 +416,46 @@ def main(): supports_check_mode=True ) + if not HAS_GITLAB_PACKAGE: + module.fail_json(msg=missing_required_lib("python-gitlab"), exception=GITLAB_IMP_ERR) + purge = module.params['purge'] var_list = module.params['vars'] state = module.params['state'] - if not HAS_GITLAB_PACKAGE: - module.fail_json(msg=missing_required_lib("python-gitlab"), exception=GITLAB_IMP_ERR) + if var_list: + variables = vars_to_variables(var_list, module) + else: + variables = module.params['variables'] + + if state == 'present': + if any(x['value'] is None for x in variables): + module.fail_json(msg='value parameter is required in state present') gitlab_instance = gitlab_authentication(module) this_gitlab = GitlabGroupVariables(module=module, gitlab_instance=gitlab_instance) - changed, return_value = native_python_main(this_gitlab, purge, var_list, state, module) + changed, raw_return_value, before, after = native_python_main(this_gitlab, purge, variables, state, module) + + # postprocessing + for item in after: + item.pop('group_id') + item['name'] = item.pop('key') + for item in before: + item.pop('group_id') + item['name'] = item.pop('key') + + untouched_key_name = 'key' + if not module.check_mode: + untouched_key_name = 'name' + raw_return_value['untouched'] = [x for x in before if x in after] + + added = [x.get('key') for x in raw_return_value['added']] + updated = [x.get('key') for x in raw_return_value['updated']] + removed = [x.get('key') for x in raw_return_value['removed']] + untouched = [x.get(untouched_key_name) for x in raw_return_value['untouched']] + return_value = dict(added=added, updated=updated, removed=removed, untouched=untouched) module.exit_json(changed=changed, group_variable=return_value) diff --git a/tests/integration/targets/gitlab_group_variable/tasks/main.yml b/tests/integration/targets/gitlab_group_variable/tasks/main.yml index d3b6eb4ce0..1bba0c9d1d 100644 --- a/tests/integration/targets/gitlab_group_variable/tasks/main.yml +++ b/tests/integration/targets/gitlab_group_variable/tasks/main.yml @@ -36,8 +36,9 @@ api_url: "{{ gitlab_host }}" api_token: "{{ gitlab_login_token }}" group: "{{ gitlab_group_name }}" - vars: - ACCESS_KEY_ID: checkmode + variables: + - name: ACCESS_KEY_ID + value: checkmode register: gitlab_group_variable_state - name: state must be changed @@ -219,6 +220,42 @@ that: - gitlab_group_variable_state is not changed +- name: change environment scope + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + vars: + ACCESS_KEY_ID: + environment_scope: testing + value: checkmode + register: gitlab_group_variable_state + when: gitlab_premium_tests is defined + +- name: state must be changed + assert: + that: + - gitlab_group_variable_state is changed + when: gitlab_premium_tests is defined + +- name: apply again the environment scope change + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + vars: + ACCESS_KEY_ID: + environment_scope: testing + value: checkmode + register: gitlab_group_variable_state + when: gitlab_premium_tests is defined + +- name: state must not be changed + assert: + that: + - gitlab_group_variable_state is not changed + when: gitlab_premium_tests is defined + - name: purge all variables at the beginning gitlab_group_variable: api_url: "{{ gitlab_host }}" @@ -426,8 +463,8 @@ api_url: "{{ gitlab_host }}" api_token: "{{ gitlab_login_token }}" group: "{{ gitlab_group_name }}" - vars: - my_test_var: + variables: + - name: my_test_var value: my_test_value variable_type: file purge: False @@ -583,3 +620,84 @@ - gitlab_group_variable_state.group_variable.untouched|length == 0 - gitlab_group_variable_state.group_variable.removed|length == 40 - gitlab_group_variable_state.group_variable.updated|length == 0 + +- name: same vars, diff scope + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + purge: True + variables: + - name: SECRET_ACCESS_KEY + value: 3214cbad + masked: true + protected: true + variable_type: env_var + environment_scope: production + - name: SECRET_ACCESS_KEY + value: hello_world + masked: true + protected: true + variable_type: env_var + environment_scope: development + register: gitlab_group_variable_state + when: gitlab_premium_tests is defined + +- name: verify two vars + assert: + that: + - gitlab_group_variable_state.changed + - gitlab_group_variable_state.group_variable.added|length == 2 + - gitlab_group_variable_state.group_variable.untouched|length == 0 + - gitlab_group_variable_state.group_variable.removed|length == 0 + - gitlab_group_variable_state.group_variable.updated|length == 0 + when: gitlab_premium_tests is defined + +- name: throw error when state is present but no value is given + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + variables: + - name: delete_me + register: gitlab_group_variable_state + ignore_errors: yes + +- name: verify fail + assert: + that: + - gitlab_group_variable_state.failed + - gitlab_group_variable_state is not changed + +- name: set a new variable to delete it later + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + purge: True + variables: + - name: delete_me + value: ansible + register: gitlab_group_variable_state + +- name: verify the change + assert: + that: + - gitlab_group_variable_state.changed + +- name: delete variable without referencing its value + gitlab_group_variable: + api_url: "{{ gitlab_host }}" + api_token: "{{ gitlab_login_token }}" + group: "{{ gitlab_group_name }}" + state: absent + variables: + - name: delete_me + register: gitlab_group_variable_state + +- name: verify deletion + assert: + that: + - gitlab_group_variable_state.changed + - gitlab_group_variable_state.group_variable.removed|length == 1 + \ No newline at end of file