From ac4b9b1de9b07c11be28e96dfc4779ba38e9c729 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 13:51:32 +0200 Subject: [PATCH 01/19] Import configparser directly from stdlib --- lib/ansible/modules/yum_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 88f8cd065bd336..f60652a13c92cf 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -419,10 +419,10 @@ sample: "present" ''' +import configparser import os from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.six.moves import configparser from ansible.module_utils.common.text.converters import to_native From 31832dd3d27795716558965ce4091ab94e0241a0 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 13:53:40 +0200 Subject: [PATCH 02/19] No need for class variables --- lib/ansible/modules/yum_repository.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index f60652a13c92cf..034ba8f65686cf 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -427,12 +427,6 @@ class YumRepo(object): - # Class global variables - module = None - params = None - section = None - repofile = configparser.RawConfigParser() - # List of parameters which will be allowed in the repo file output allowed_params = [ 'async', @@ -492,6 +486,7 @@ def __init__(self, module): self.params = self.module.params # Section is always the repoid self.section = self.params['repoid'] + self.repofile = configparser.RawConfigParser() # Check if repo directory exists repos_dir = self.params['reposdir'] From bd08a3bfedc9d0818a5d761c0291c753c809d92a Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 13:58:32 +0200 Subject: [PATCH 03/19] Utilize argspec for required arg check --- lib/ansible/modules/yum_repository.py | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 034ba8f65686cf..d797faf3cd4179 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -510,15 +510,6 @@ def add(self): # Add section self.repofile.add_section(self.section) - # Baseurl/mirrorlist is not required because for removal we need only - # the repo name. This is why we check if the baseurl/mirrorlist is - # defined. - req_params = (self.params['baseurl'], self.params['metalink'], self.params['mirrorlist']) - if req_params == (None, None, None): - self.module.fail_json( - msg="Parameter 'baseurl', 'metalink' or 'mirrorlist' is required for " - "adding a new repo.") - # Set options for key, value in sorted(self.params.items()): if key in self.list_params and isinstance(value, list): @@ -640,6 +631,10 @@ def main(): argument_spec['async'] = dict(type='bool') module = AnsibleModule( + required_if=[ + ["state", "present", ["baseurl", "mirrorlist", "metalink"], True], + ["state", "present", ["description"]], + ], argument_spec=argument_spec, add_file_common_args=True, supports_check_mode=True, @@ -648,18 +643,6 @@ def main(): name = module.params['name'] state = module.params['state'] - # Check if required parameters are present - if state == 'present': - if ( - module.params['baseurl'] is None and - module.params['metalink'] is None and - module.params['mirrorlist'] is None): - module.fail_json( - msg="Parameter 'baseurl', 'metalink' or 'mirrorlist' is required.") - if module.params['description'] is None: - module.fail_json( - msg="Parameter 'description' is required.") - # Rename "name" and "description" to ensure correct key sorting module.params['repoid'] = module.params['name'] module.params['name'] = module.params['description'] From 468ee0d2a20d696073c327004ad4fc41ffb9526c Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:11:16 +0200 Subject: [PATCH 04/19] Move arg processing to single place --- lib/ansible/modules/yum_repository.py | 41 +++++++++++++-------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index d797faf3cd4179..589e3c2f66ca45 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -476,9 +476,6 @@ class YumRepo(object): 'ui_repoid_vars', 'username'] - # List of parameters which can be a list - list_params = ['exclude', 'includepkgs'] - def __init__(self, module): # To be able to use fail_json self.module = module @@ -488,16 +485,6 @@ def __init__(self, module): self.section = self.params['repoid'] self.repofile = configparser.RawConfigParser() - # Check if repo directory exists - repos_dir = self.params['reposdir'] - if not os.path.isdir(repos_dir): - self.module.fail_json( - msg="Repo directory '%s' does not exist." % repos_dir) - - # Set dest; also used to set dest parameter for the FS attributes - self.params['dest'] = os.path.join( - repos_dir, "%s.repo" % self.params['file']) - # Read the repo file if it exists if os.path.isfile(self.params['dest']): self.repofile.read(self.params['dest']) @@ -512,10 +499,7 @@ def add(self): # Set options for key, value in sorted(self.params.items()): - if key in self.list_params and isinstance(value, list): - # Join items into one string for specific parameters - value = ' '.join(value) - elif isinstance(value, bool): + if isinstance(value, bool): # Convert boolean value to integer value = int(value) @@ -649,16 +633,29 @@ def main(): del module.params['description'] # Change list type to string for baseurl and gpgkey - for list_param in ['baseurl', 'gpgkey']: - if ( - list_param in module.params and - module.params[list_param] is not None): - module.params[list_param] = "\n".join(module.params[list_param]) + for list_param in {'baseurl', 'gpgkey'}: + v = module.params[list_param] + if v is not None: + module.params[list_param] = '\n'.join(v) + + for list_param in {'exclude', 'includepkgs'}: + v = module.params[list_param] + if v is not None: + module.params[list_param] = ' '.join(v) # Define repo file name if it doesn't exist if module.params['file'] is None: module.params['file'] = module.params['repoid'] + repos_dir = module.params['reposdir'] + if not os.path.isdir(repos_dir): + module.fail_json( + msg="Repo directory '%s' does not exist." % repos_dir + ) + + # Set dest; also used to set dest parameter for the FS attributes + module.params['dest'] = os.path.join(repos_dir, "%s.repo" % module.params['file']) + # Instantiate the YumRepo object yumrepo = YumRepo(module) From d708cdd2c8e43bcedeb6cfe32d47edb709ba5c98 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:14:11 +0200 Subject: [PATCH 05/19] Remove obvious comments --- lib/ansible/modules/yum_repository.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 589e3c2f66ca45..744ac4bd8f3e13 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -427,7 +427,6 @@ class YumRepo(object): - # List of parameters which will be allowed in the repo file output allowed_params = [ 'async', 'bandwidth', @@ -477,30 +476,22 @@ class YumRepo(object): 'username'] def __init__(self, module): - # To be able to use fail_json self.module = module - # Shortcut for the params self.params = self.module.params # Section is always the repoid self.section = self.params['repoid'] self.repofile = configparser.RawConfigParser() - # Read the repo file if it exists if os.path.isfile(self.params['dest']): self.repofile.read(self.params['dest']) def add(self): - # Remove already existing repo and create a new one if self.repofile.has_section(self.section): self.repofile.remove_section(self.section) - - # Add section self.repofile.add_section(self.section) - # Set options for key, value in sorted(self.params.items()): if isinstance(value, bool): - # Convert boolean value to integer value = int(value) # Set the value only if it was defined (default is None) @@ -514,7 +505,6 @@ def add(self): def save(self): if len(self.repofile.sections()): - # Write data into the file try: with open(self.params['dest'], 'w') as fd: self.repofile.write(fd) @@ -523,7 +513,6 @@ def save(self): msg="Problems handling file %s." % self.params['dest'], details=to_native(e)) else: - # Remove the file if there are not repos try: os.remove(self.params['dest']) except OSError as e: @@ -534,14 +523,12 @@ def save(self): details=to_native(e)) def remove(self): - # Remove section if exists if self.repofile.has_section(self.section): self.repofile.remove_section(self.section) def dump(self): repo_string = "" - # Compose the repo file for section in sorted(self.repofile.sections()): repo_string += "[%s]\n" % section @@ -554,7 +541,6 @@ def dump(self): def main(): - # Module settings argument_spec = dict( bandwidth=dict(), baseurl=dict(type='list', elements='str'), @@ -632,7 +618,6 @@ def main(): module.params['name'] = module.params['description'] del module.params['description'] - # Change list type to string for baseurl and gpgkey for list_param in {'baseurl', 'gpgkey'}: v = module.params[list_param] if v is not None: @@ -643,7 +628,6 @@ def main(): if v is not None: module.params[list_param] = ' '.join(v) - # Define repo file name if it doesn't exist if module.params['file'] is None: module.params['file'] = module.params['repoid'] @@ -656,10 +640,8 @@ def main(): # Set dest; also used to set dest parameter for the FS attributes module.params['dest'] = os.path.join(repos_dir, "%s.repo" % module.params['file']) - # Instantiate the YumRepo object yumrepo = YumRepo(module) - # Get repo status before change diff = { 'before_header': yumrepo.params['dest'], 'before': yumrepo.dump(), @@ -667,28 +649,22 @@ def main(): 'after': '' } - # Perform action depending on the state if state == 'present': yumrepo.add() elif state == 'absent': yumrepo.remove() - # Get repo status after change diff['after'] = yumrepo.dump() - # Compare repo states changed = diff['before'] != diff['after'] - # Save the file only if not in check mode and if there was a change if not module.check_mode and changed: yumrepo.save() - # Change file attributes if needed if os.path.isfile(module.params['dest']): file_args = module.load_file_common_arguments(module.params) changed = module.set_fs_attributes_if_different(file_args, changed) - # Print status of the change module.exit_json(changed=changed, repo=name, state=state, diff=diff) From b57137619ef84c25cc52d010f8a968c8eabf85b5 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:15:19 +0200 Subject: [PATCH 06/19] remove_section already check for existence --- lib/ansible/modules/yum_repository.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 744ac4bd8f3e13..dea7afbbc41c0b 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -486,8 +486,7 @@ def __init__(self, module): self.repofile.read(self.params['dest']) def add(self): - if self.repofile.has_section(self.section): - self.repofile.remove_section(self.section) + self.remove() self.repofile.add_section(self.section) for key, value in sorted(self.params.items()): @@ -504,7 +503,7 @@ def add(self): self.repofile.set(self.section, key, value) def save(self): - if len(self.repofile.sections()): + if self.repofile.sections(): try: with open(self.params['dest'], 'w') as fd: self.repofile.write(fd) @@ -523,8 +522,7 @@ def save(self): details=to_native(e)) def remove(self): - if self.repofile.has_section(self.section): - self.repofile.remove_section(self.section) + self.repofile.remove_section(self.section) def dump(self): repo_string = "" From 5a1532238a7e8dfa62a14a611b2a7feb6f51d78e Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:19:46 +0200 Subject: [PATCH 07/19] Easier to maintain I think --- lib/ansible/modules/yum_repository.py | 56 ++++----------------------- 1 file changed, 8 insertions(+), 48 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index dea7afbbc41c0b..55321ae8c09471 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -427,53 +427,13 @@ class YumRepo(object): - allowed_params = [ - 'async', - 'bandwidth', - 'baseurl', - 'cost', - 'countme', - 'deltarpm_metadata_percentage', - 'deltarpm_percentage', - 'enabled', - 'enablegroups', - 'exclude', - 'failovermethod', - 'gpgcakey', - 'gpgcheck', - 'gpgkey', - 'module_hotfixes', - 'http_caching', - 'include', - 'includepkgs', - 'ip_resolve', - 'keepalive', - 'keepcache', - 'metadata_expire', - 'metadata_expire_filter', - 'metalink', - 'mirrorlist', - 'mirrorlist_expire', - 'name', - 'password', - 'priority', - 'protect', - 'proxy', - 'proxy_password', - 'proxy_username', - 'repo_gpgcheck', - 'retries', - 's3_enabled', - 'skip_if_unavailable', - 'sslcacert', - 'ssl_check_cert_permissions', - 'sslclientcert', - 'sslclientkey', - 'sslverify', - 'throttle', - 'timeout', - 'ui_repoid_vars', - 'username'] + non_yum_args = frozenset(( + "dest", + "file", + "repoid", + "reposdir", + "state", + )) def __init__(self, module): self.module = module @@ -494,7 +454,7 @@ def add(self): value = int(value) # Set the value only if it was defined (default is None) - if value is not None and key in self.allowed_params: + if value is not None and key not in self.non_yum_args: if key == 'keepcache': self.module.deprecate( "'keepcache' parameter is deprecated.", From 5f5a22753013a9b361e37c6dbaca351a92b60680 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:22:58 +0200 Subject: [PATCH 08/19] comment --- lib/ansible/modules/yum_repository.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 55321ae8c09471..b312c0a9f23720 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -556,6 +556,7 @@ def main(): username=dict(), ) + # async is a Python keyword argument_spec['async'] = dict(type='bool') module = AnsibleModule( From 0c9a306b44274c943d56f9af998cee5dcc9575d6 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:25:54 +0200 Subject: [PATCH 09/19] set expects str --- lib/ansible/modules/yum_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index b312c0a9f23720..4d8ecfb903e253 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -451,7 +451,7 @@ def add(self): for key, value in sorted(self.params.items()): if isinstance(value, bool): - value = int(value) + value = str(int(value)) # Set the value only if it was defined (default is None) if value is not None and key not in self.non_yum_args: From d451d2de2dd31294a06c37c6e0686e1fc9a86985 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 16 Apr 2024 14:34:03 +0200 Subject: [PATCH 10/19] convert only what's needed --- lib/ansible/modules/yum_repository.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 4d8ecfb903e253..79fe34ad526bd0 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -450,17 +450,16 @@ def add(self): self.repofile.add_section(self.section) for key, value in sorted(self.params.items()): + if value is None or key in self.non_yum_args: + continue + if key == 'keepcache': + self.module.deprecate( + "'keepcache' parameter is deprecated.", + version='2.20' + ) if isinstance(value, bool): value = str(int(value)) - - # Set the value only if it was defined (default is None) - if value is not None and key not in self.non_yum_args: - if key == 'keepcache': - self.module.deprecate( - "'keepcache' parameter is deprecated.", - version='2.20' - ) - self.repofile.set(self.section, key, value) + self.repofile.set(self.section, key, value) def save(self): if self.repofile.sections(): From c8a0b25ac1670dd46639edee7f90ad9e3668450e Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 17 Apr 2024 08:30:41 +0200 Subject: [PATCH 11/19] deprecate all the things --- lib/ansible/modules/yum_repository.py | 40 +++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 79fe34ad526bd0..5c99df2d67cd8a 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -64,6 +64,8 @@ can give values over V(100), so V(200) means that the metadata is required to be half the size of the packages. Use V(0) to turn off this check, and always download metadata. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: str deltarpm_percentage: description: @@ -112,6 +114,8 @@ gpgcakey: description: - A URL pointing to the ASCII-armored CA key file for the repository. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: str gpgcheck: description: @@ -140,6 +144,8 @@ - V(packages) means that only RPM package downloads should be cached (but not repository metadata downloads). - V(none) means that no HTTP downloads should be cached. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. choices: [all, packages, none] type: str include: @@ -170,12 +176,15 @@ - This tells yum whether or not HTTP/1.1 keepalive should be used with this repository. This can improve transfer speeds by using one connection when downloading multiple files from a repository. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: bool keepcache: description: - Either V(1) or V(0). Determines whether or not yum keeps the cache of headers and packages after successful installation. - - This parameter is deprecated and will be removed in version 2.20. + - This parameter is deprecated as it is only valid in the main configuration + and will be removed in ansible-core 2.20. choices: ['0', '1'] type: str metadata_expire: @@ -201,6 +210,8 @@ other commands which will require the latest metadata. Eg. C(yum check-update). - Note that this option does not override "yum clean expire-cache". + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. choices: [never, 'read-only:past', 'read-only:present', 'read-only:future'] type: str metalink: @@ -222,6 +233,8 @@ - Time (in seconds) after which the mirrorlist locally cached will expire. - Default value is 6 hours. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: str name: description: @@ -243,6 +256,8 @@ protect: description: - Protect packages from updates from other repositories. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: bool proxy: description: @@ -291,6 +306,8 @@ O(skip_if_unavailable) to be V(true). This is most useful for non-root processes which use yum on repos that have client cert files which are readable only by root. + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: bool sslcacert: description: @@ -336,6 +353,8 @@ - When a repository id is displayed, append these yum variables to the string if they are used in the O(baseurl)/etc. Variables are appended in the order listed (and found). + - This parameter is deprecated as it has no affect with dnf as an underlying package manager + and will be removed in ansible-core 2.22. type: str username: description: @@ -454,9 +473,26 @@ def add(self): continue if key == 'keepcache': self.module.deprecate( - "'keepcache' parameter is deprecated.", + "'keepcache' parameter is deprecated as it is only valid in " + "the main configuration.", version='2.20' ) + elif key in { + "deltarpm_metadata_percentage", + "gpgcakey", + "http_caching", + "keepalive", + "metadata_expire_filter", + "mirrorlist_expire", + "protect", + "ssl_check_cert_permissions", + "ui_repoid_vars", + }: + self.module.deprecate( + f"'{key}' parameter is deprecated as it has no affect with dnf " + "as an underlying package manager.", + version='2.22' + ) if isinstance(value, bool): value = str(int(value)) self.repofile.set(self.section, key, value) From 639277fe6c8c13a2585670388f08a10601a88aa8 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 17 Apr 2024 15:12:30 +0200 Subject: [PATCH 12/19] excludepkgs is used in dnf, exclude still works too --- lib/ansible/modules/yum_repository.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 5c99df2d67cd8a..cb90e682f1cb74 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -95,8 +95,11 @@ space separated list. Shell globs using wildcards (for example V(*) and V(?)) are allowed. - The list can also be a regular YAML array. + - excludepkgs alias was added in ansible-core 2.18 type: list elements: str + aliases: + - excludepkgs failovermethod: choices: [roundrobin, priority] description: @@ -544,7 +547,7 @@ def main(): description=dict(), enabled=dict(type='bool'), enablegroups=dict(type='bool'), - exclude=dict(type='list', elements='str'), + exclude=dict(type='list', elements='str', aliases=['excludepkgs']), failovermethod=dict(choices=['roundrobin', 'priority']), file=dict(), gpgcakey=dict(no_log=False), From b7ddfc25f259ae3468ac05f9344f19688acf6c45 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 18 Apr 2024 12:59:49 +0200 Subject: [PATCH 13/19] deprecate async --- lib/ansible/modules/yum_repository.py | 29 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index cb90e682f1cb74..cd649d107786bf 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -23,9 +23,11 @@ - If set to V(true) Yum will download packages and metadata from this repo in parallel, if possible. - In ansible-core 2.11, 2.12, and 2.13 the default value is V(true). - - This option has been deprecated in RHEL 8. If you're using one of the + - This option has been removed in RHEL 8. If you're using one of the versions listed above, you can set this option to None to avoid passing an unknown configuration option. + - This parameter is deprecated as it has been removed on systems supported by ansible-core + and will be removed in ansible-core 2.22. type: bool bandwidth: description: @@ -64,7 +66,7 @@ can give values over V(100), so V(200) means that the metadata is required to be half the size of the packages. Use V(0) to turn off this check, and always download metadata. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: str deltarpm_percentage: @@ -117,7 +119,7 @@ gpgcakey: description: - A URL pointing to the ASCII-armored CA key file for the repository. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: str gpgcheck: @@ -147,7 +149,7 @@ - V(packages) means that only RPM package downloads should be cached (but not repository metadata downloads). - V(none) means that no HTTP downloads should be cached. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. choices: [all, packages, none] type: str @@ -179,7 +181,7 @@ - This tells yum whether or not HTTP/1.1 keepalive should be used with this repository. This can improve transfer speeds by using one connection when downloading multiple files from a repository. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: bool keepcache: @@ -213,7 +215,7 @@ other commands which will require the latest metadata. Eg. C(yum check-update). - Note that this option does not override "yum clean expire-cache". - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. choices: [never, 'read-only:past', 'read-only:present', 'read-only:future'] type: str @@ -236,7 +238,7 @@ - Time (in seconds) after which the mirrorlist locally cached will expire. - Default value is 6 hours. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: str name: @@ -259,7 +261,7 @@ protect: description: - Protect packages from updates from other repositories. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: bool proxy: @@ -309,7 +311,7 @@ O(skip_if_unavailable) to be V(true). This is most useful for non-root processes which use yum on repos that have client cert files which are readable only by root. - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: bool sslcacert: @@ -356,7 +358,7 @@ - When a repository id is displayed, append these yum variables to the string if they are used in the O(baseurl)/etc. Variables are appended in the order listed (and found). - - This parameter is deprecated as it has no affect with dnf as an underlying package manager + - This parameter is deprecated as it has no effect with dnf as an underlying package manager and will be removed in ansible-core 2.22. type: str username: @@ -480,6 +482,11 @@ def add(self): "the main configuration.", version='2.20' ) + elif key == 'async': + self.module.deprecate( + "'async' parameter is deprecated as it has been removed on systems supported by ansible-core", + version = '2.22', + ) elif key in { "deltarpm_metadata_percentage", "gpgcakey", @@ -492,7 +499,7 @@ def add(self): "ui_repoid_vars", }: self.module.deprecate( - f"'{key}' parameter is deprecated as it has no affect with dnf " + f"'{key}' parameter is deprecated as it has no effect with dnf " "as an underlying package manager.", version='2.22' ) From 8b2240f6bb6048d054e5273195dad3724adf182f Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 18 Apr 2024 16:00:14 +0200 Subject: [PATCH 14/19] wip --- lib/ansible/modules/yum_repository.py | 79 ++++++++++++--------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index cd649d107786bf..efbd8ec1e0eb0f 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -446,35 +446,26 @@ import configparser import os -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, FILE_COMMON_ARGUMENTS from ansible.module_utils.common.text.converters import to_native -class YumRepo(object): - non_yum_args = frozenset(( - "dest", - "file", - "repoid", - "reposdir", - "state", - )) - - def __init__(self, module): +class YumRepo: + def __init__(self, module, params, repoid, dest): self.module = module - self.params = self.module.params - # Section is always the repoid - self.section = self.params['repoid'] + self.params = params + self.section = repoid self.repofile = configparser.RawConfigParser() - - if os.path.isfile(self.params['dest']): - self.repofile.read(self.params['dest']) + self.dest = dest + if os.path.isfile(dest): + self.repofile.read(dest) def add(self): self.remove() self.repofile.add_section(self.section) for key, value in sorted(self.params.items()): - if value is None or key in self.non_yum_args: + if value is None: continue if key == 'keepcache': self.module.deprecate( @@ -510,20 +501,20 @@ def add(self): def save(self): if self.repofile.sections(): try: - with open(self.params['dest'], 'w') as fd: + with open(self.dest, 'w') as fd: self.repofile.write(fd) except IOError as e: self.module.fail_json( - msg="Problems handling file %s." % self.params['dest'], + msg="Problems handling file %s." % self.dest, details=to_native(e)) else: try: - os.remove(self.params['dest']) + os.remove(self.dest) except OSError as e: self.module.fail_json( msg=( "Cannot remove empty repo file %s." % - self.params['dest']), + self.dest), details=to_native(e)) def remove(self): @@ -613,43 +604,43 @@ def main(): add_file_common_args=True, supports_check_mode=True, ) + yum_repo_params = module.params.copy() + for alias in module.aliases: + yum_repo_params.pop(alias, None) + file_common_params = {} + for param in FILE_COMMON_ARGUMENTS: + file_common_params[param] = yum_repo_params.pop(param) + state = yum_repo_params.pop("state") - name = module.params['name'] - state = module.params['state'] - - # Rename "name" and "description" to ensure correct key sorting - module.params['repoid'] = module.params['name'] - module.params['name'] = module.params['description'] - del module.params['description'] + name = yum_repo_params['name'] + yum_repo_params['name'] = yum_repo_params.pop('description') for list_param in {'baseurl', 'gpgkey'}: - v = module.params[list_param] + v = yum_repo_params[list_param] if v is not None: - module.params[list_param] = '\n'.join(v) + yum_repo_params[list_param] = '\n'.join(v) for list_param in {'exclude', 'includepkgs'}: - v = module.params[list_param] + v = yum_repo_params[list_param] if v is not None: - module.params[list_param] = ' '.join(v) - - if module.params['file'] is None: - module.params['file'] = module.params['repoid'] + yum_repo_params[list_param] = ' '.join(v) - repos_dir = module.params['reposdir'] + repos_dir = yum_repo_params.pop("reposdir") if not os.path.isdir(repos_dir): module.fail_json( msg="Repo directory '%s' does not exist." % repos_dir ) - # Set dest; also used to set dest parameter for the FS attributes - module.params['dest'] = os.path.join(repos_dir, "%s.repo" % module.params['file']) + if (file := yum_repo_params.pop("file")) is None: + file = name + file_common_params["dest"] = os.path.join(repos_dir, f"{file}.repo") - yumrepo = YumRepo(module) + yumrepo = YumRepo(module, yum_repo_params, name, file_common_params["dest"]) diff = { - 'before_header': yumrepo.params['dest'], + 'before_header': file_common_params["dest"], 'before': yumrepo.dump(), - 'after_header': yumrepo.params['dest'], + 'after_header': file_common_params["dest"], 'after': '' } @@ -665,8 +656,8 @@ def main(): if not module.check_mode and changed: yumrepo.save() - if os.path.isfile(module.params['dest']): - file_args = module.load_file_common_arguments(module.params) + if os.path.isfile(file_common_params["dest"]): + file_args = module.load_file_common_arguments(file_common_params) changed = module.set_fs_attributes_if_different(file_args, changed) module.exit_json(changed=changed, repo=name, state=state, diff=diff) From 5c36582a98655b6adc96f07ab730d3db2c0b43c8 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 22 Apr 2024 11:43:15 +0200 Subject: [PATCH 15/19] change formatting --- lib/ansible/modules/yum_repository.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index efbd8ec1e0eb0f..179aad81c3f300 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -6,7 +6,6 @@ from __future__ import annotations - DOCUMENTATION = ''' --- module: yum_repository @@ -476,7 +475,7 @@ def add(self): elif key == 'async': self.module.deprecate( "'async' parameter is deprecated as it has been removed on systems supported by ansible-core", - version = '2.22', + version='2.22', ) elif key in { "deltarpm_metadata_percentage", @@ -505,17 +504,17 @@ def save(self): self.repofile.write(fd) except IOError as e: self.module.fail_json( - msg="Problems handling file %s." % self.dest, - details=to_native(e)) + msg=f"Problems handling file {self.dest}.", + details=to_native(e), + ) else: try: os.remove(self.dest) except OSError as e: self.module.fail_json( - msg=( - "Cannot remove empty repo file %s." % - self.dest), - details=to_native(e)) + msg=f"Cannot remove empty repo file {self.dest}.", + details=to_native(e), + ) def remove(self): self.repofile.remove_section(self.section) From 18622dc85ec2e8c3a1fe7050d0ee25fd5492e175 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 22 Apr 2024 11:50:04 +0200 Subject: [PATCH 16/19] sanity fix --- lib/ansible/modules/yum_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index 179aad81c3f300..dddb46b4ec56e9 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -614,12 +614,12 @@ def main(): name = yum_repo_params['name'] yum_repo_params['name'] = yum_repo_params.pop('description') - for list_param in {'baseurl', 'gpgkey'}: + for list_param in ('baseurl', 'gpgkey'): v = yum_repo_params[list_param] if v is not None: yum_repo_params[list_param] = '\n'.join(v) - for list_param in {'exclude', 'includepkgs'}: + for list_param in ('exclude', 'includepkgs'): v = yum_repo_params[list_param] if v is not None: yum_repo_params[list_param] = ' '.join(v) From d96b5a1cd110f08f39eee135cfad4705c2a1ccc6 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 22 Apr 2024 13:05:23 +0200 Subject: [PATCH 17/19] comment --- lib/ansible/modules/yum_repository.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ansible/modules/yum_repository.py b/lib/ansible/modules/yum_repository.py index dddb46b4ec56e9..c0c02c22f2ea66 100644 --- a/lib/ansible/modules/yum_repository.py +++ b/lib/ansible/modules/yum_repository.py @@ -603,14 +603,17 @@ def main(): add_file_common_args=True, supports_check_mode=True, ) + + # make copy of params as we need to split them into yum repo only and file params yum_repo_params = module.params.copy() for alias in module.aliases: yum_repo_params.pop(alias, None) + file_common_params = {} for param in FILE_COMMON_ARGUMENTS: file_common_params[param] = yum_repo_params.pop(param) - state = yum_repo_params.pop("state") + state = yum_repo_params.pop("state") name = yum_repo_params['name'] yum_repo_params['name'] = yum_repo_params.pop('description') From c42113359c3153db490ea6a8fc1de9a2faeae6c0 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 22 Apr 2024 13:10:39 +0200 Subject: [PATCH 18/19] Add changelog --- changelogs/fragments/yum_repository.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/fragments/yum_repository.yml diff --git a/changelogs/fragments/yum_repository.yml b/changelogs/fragments/yum_repository.yml new file mode 100644 index 00000000000000..661f695106a5a3 --- /dev/null +++ b/changelogs/fragments/yum_repository.yml @@ -0,0 +1,5 @@ +deprecated_features: + - yum_repository - the following options are deprecated: ``deltarpm_metadata_percentage``, ``gpgcakey``, ``http_caching``, ``keepalive``, ``metadata_expire_filter``, ``mirrorlist_expire``, ``protect``, ``ssl_check_cert_permissions``, ``ui_repoid_vars`` as they have no effect for dnf as an underlying package manager. The options will be removed in ansible-core 2.22. + - yum_repository - deprecate ``async`` option as it has been removed in RHEL 8 and will be removed in ansible-core 2.22. +minor_changes: + - yum_repository - add ``excludepkgs`` alias to the ``exclude`` option. From d55710412a3a9430a67b3be263a0a03fe57e8e82 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 22 Apr 2024 13:14:29 +0200 Subject: [PATCH 19/19] changelog fix --- changelogs/fragments/yum_repository.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/changelogs/fragments/yum_repository.yml b/changelogs/fragments/yum_repository.yml index 661f695106a5a3..508760614dc367 100644 --- a/changelogs/fragments/yum_repository.yml +++ b/changelogs/fragments/yum_repository.yml @@ -1,5 +1,9 @@ deprecated_features: - - yum_repository - the following options are deprecated: ``deltarpm_metadata_percentage``, ``gpgcakey``, ``http_caching``, ``keepalive``, ``metadata_expire_filter``, ``mirrorlist_expire``, ``protect``, ``ssl_check_cert_permissions``, ``ui_repoid_vars`` as they have no effect for dnf as an underlying package manager. The options will be removed in ansible-core 2.22. - yum_repository - deprecate ``async`` option as it has been removed in RHEL 8 and will be removed in ansible-core 2.22. + - >- + yum_repository - the following options are deprecated: ``deltarpm_metadata_percentage``, ``gpgcakey``, ``http_caching``, + ``keepalive``, ``metadata_expire_filter``, ``mirrorlist_expire``, ``protect``, ``ssl_check_cert_permissions``, + ``ui_repoid_vars`` as they have no effect for dnf as an underlying package manager. + The options will be removed in ansible-core 2.22. minor_changes: - yum_repository - add ``excludepkgs`` alias to the ``exclude`` option.