jenkins_plugin: fix sanity checks (#5565) (#5575)

* jenkins_plugin: fix sanity checks

* update BOTMETA

* add changelog fragment

* fix copyright

* Update plugins/module_utils/jenkins.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/module_utils/jenkins.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 8ad43fd774)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

.github/BOTMETA.yml

@@ -284,6 +284,9 @@ files:
   $module_utils/ipa.py:
     labels: ipa
     maintainers: $team_ipa
+  $module_utils/jenkins.py:
+    labels: jenkins
+    maintainers: russoz
   $module_utils/manageiq.py:
     labels: manageiq
     maintainers: $team_manageiq

changelogs/fragments/5565-jenkins-plugin-sanity.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - jenkins_plugin - refactor code to module util to fix sanity check (https://github.com/ansible-collections/community.general/pull/5565).

plugins/module_utils/jenkins.py

@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+
+# Copyright (c) 2022, Alexei Znamensky <russoz@gmail.com>
+#
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+import os
+import time
+
+
+def download_updates_file(updates_expiration):
+    updates_filename = 'jenkins-plugin-cache.json'
+    updates_dir = os.path.expanduser('~/.ansible/tmp')
+    updates_file = os.path.join(updates_dir, updates_filename)
+    download_updates = True
+
+    # Make sure the destination directory exists
+    if not os.path.isdir(updates_dir):
+        os.makedirs(updates_dir, 0o700)
+
+    # Check if we need to download new updates file
+    if os.path.isfile(updates_file):
+        # Get timestamp when the file was changed last time
+        ts_file = os.stat(updates_file).st_mtime
+        ts_now = time.time()
+
+        if ts_now - ts_file < updates_expiration:
+            download_updates = False
+
+    return updates_file, download_updates

plugins/modules/jenkins_plugin.py

@@ -290,12 +290,6 @@ state:
     sample: "present"
 '''
 
-from ansible.module_utils.basic import AnsibleModule, to_bytes
-from ansible.module_utils.six.moves import http_cookiejar as cookiejar
-from ansible.module_utils.six.moves.urllib.parse import urlencode
-from ansible.module_utils.urls import fetch_url, url_argument_spec
-from ansible.module_utils.six import text_type, binary_type
-from ansible.module_utils.common.text.converters import to_native
 import hashlib
 import io
 import json
@@ -303,6 +297,15 @@ import os
 import tempfile
 import time
 
+from ansible.module_utils.basic import AnsibleModule, to_bytes
+from ansible.module_utils.six.moves import http_cookiejar as cookiejar
+from ansible.module_utils.six.moves.urllib.parse import urlencode
+from ansible.module_utils.urls import fetch_url, url_argument_spec
+from ansible.module_utils.six import text_type, binary_type
+from ansible.module_utils.common.text.converters import to_native
+
+from ansible_collections.community.general.plugins.module_utils.jenkins import download_updates_file
+
 
 class FailedInstallingWithPluginManager(Exception):
     pass
@@ -605,21 +608,12 @@ class JenkinsPlugin(object):
         return urls
 
     def _download_updates(self):
-        updates_filename = 'jenkins-plugin-cache.json'
+        try:
-        updates_dir = os.path.expanduser('~/.ansible/tmp')
+            updates_file, download_updates = download_updates_file(self.params['updates_expiration'])
-        updates_file = "%s/%s" % (updates_dir, updates_filename)
+        except OSError as e:
-        download_updates = True
+            self.module.fail_json(
-
+                msg="Cannot create temporal directory.",
-        # Check if we need to download new updates file
+                details=to_native(e))
-        if os.path.isfile(updates_file):
-            # Get timestamp when the file was changed last time
-            ts_file = os.stat(updates_file).st_mtime
-            ts_now = time.time()
-
-            if ts_now - ts_file < self.params['updates_expiration']:
-                download_updates = False
-
-        updates_file_orig = updates_file
 
         # Download the updates file if needed
         if download_updates:
@@ -632,56 +626,39 @@ class JenkinsPlugin(object):
                 msg_exception="Updates download failed.")
 
             # Write the updates file
-            update_fd, updates_file = tempfile.mkstemp()
+            tmp_update_fd, tmp_updates_file = tempfile.mkstemp()
-            os.write(update_fd, r.read())
+            os.write(tmp_update_fd, r.read())
 
             try:
-                os.close(update_fd)
+                os.close(tmp_update_fd)
             except IOError as e:
                 self.module.fail_json(
-                    msg="Cannot close the tmp updates file %s." % updates_file,
+                    msg="Cannot close the tmp updates file %s." % tmp_updates_file,
                     details=to_native(e))
 
         # Open the updates file
         try:
-            f = io.open(updates_file, encoding='utf-8')
+            f = io.open(tmp_updates_file, encoding='utf-8')
+
+            # Read only the second line
+            dummy = f.readline()
+            data = json.loads(f.readline())
         except IOError as e:
             self.module.fail_json(
                 msg="Cannot open temporal updates file.",
                 details=to_native(e))
-
-        i = 0
-        for line in f:
-            # Read only the second line
-            if i == 1:
-                try:
-                    data = json.loads(line)
         except Exception as e:
             self.module.fail_json(
                 msg="Cannot load JSON data from the tmp updates file.",
                 details=to_native(e))
 
-                break
-
-            i += 1
-
         # Move the updates file to the right place if we could read it
         if download_updates:
-            # Make sure the destination directory exists
+            self.module.atomic_move(tmp_updates_file, updates_file)
-            if not os.path.isdir(updates_dir):
-                try:
-                    os.makedirs(updates_dir, int('0700', 8))
-                except OSError as e:
-                    self.module.fail_json(
-                        msg="Cannot create temporal directory.",
-                        details=to_native(e))
-
-            self.module.atomic_move(updates_file, updates_file_orig)
 
         # Check if we have the plugin data available
-        if 'plugins' not in data or self.params['name'] not in data['plugins']:
+        if not data.get('plugins', {}).get(self.params['name']):
-            self.module.fail_json(
+            self.module.fail_json(msg="Cannot find plugin data in the updates file.")
-                msg="Cannot find plugin data in the updates file.")
 
         return data['plugins'][self.params['name']]
 

tests/sanity/ignore-2.11.txt

@@ -9,7 +9,6 @@ plugins/modules/consul.py validate-modules:undocumented-parameter
 plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choice
 plugins/modules/gconftool2.py validate-modules:parameter-state-invalid-choice         # state=get - removed in 8.0.0
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter
-plugins/modules/jenkins_plugin.py use-argspec-type-path
 plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/manageiq_policies.py validate-modules:parameter-state-invalid-choice
 plugins/modules/manageiq_provider.py validate-modules:doc-choices-do-not-match-spec   # missing docs on suboptions

tests/sanity/ignore-2.12.txt

@@ -4,7 +4,6 @@ plugins/modules/consul.py validate-modules:undocumented-parameter
 plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choice
 plugins/modules/gconftool2.py validate-modules:parameter-state-invalid-choice         # state=get - removed in 8.0.0
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter
-plugins/modules/jenkins_plugin.py use-argspec-type-path
 plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/manageiq_policies.py validate-modules:parameter-state-invalid-choice
 plugins/modules/manageiq_provider.py validate-modules:doc-choices-do-not-match-spec   # missing docs on suboptions

tests/sanity/ignore-2.13.txt

@@ -4,7 +4,6 @@ plugins/modules/consul.py validate-modules:undocumented-parameter
 plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choice
 plugins/modules/gconftool2.py validate-modules:parameter-state-invalid-choice         # state=get - removed in 8.0.0
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter
-plugins/modules/jenkins_plugin.py use-argspec-type-path
 plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/manageiq_policies.py validate-modules:parameter-state-invalid-choice
 plugins/modules/manageiq_provider.py validate-modules:doc-choices-do-not-match-spec   # missing docs on suboptions

tests/sanity/ignore-2.14.txt

@@ -5,7 +5,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/gconftool2.py validate-modules:parameter-state-invalid-choice         # state=get - removed in 8.0.0
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter
-plugins/modules/jenkins_plugin.py use-argspec-type-path
 plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/manageiq_policies.py validate-modules:parameter-state-invalid-choice
 plugins/modules/manageiq_provider.py validate-modules:doc-choices-do-not-match-spec   # missing docs on suboptions

tests/sanity/ignore-2.15.txt

@@ -5,7 +5,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/gconftool2.py validate-modules:parameter-state-invalid-choice         # state=get - removed in 8.0.0
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter
-plugins/modules/jenkins_plugin.py use-argspec-type-path
 plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/manageiq_policies.py validate-modules:parameter-state-invalid-choice
 plugins/modules/manageiq_provider.py validate-modules:doc-choices-do-not-match-spec   # missing docs on suboptions