Addressing multiple jenkins_plugins module issue

[YoussefKhalidAli]

Fix jenkins_plugin issues. This PR addresses 3 different bugs in jenkins_plugin module.

1: #854
This fixes a bug where the jenkins_plugin module installs the latest version of a plugin, even if it was incompatible with the running Jenkins server version.
The fix ensures that when a specific plugin version is latest, the module now selects the latest plugin version that is compatible with the Jenkins server.
Compatibility is determined by comparing available plugin metadata against the current Jenkins version.

2: #4995
This is an issue with plugin dependencies not installing when version is set. This is technically not a bug since jenkins_plugin doesn't install dependencies if version is set.

with_dependencies:
description:
- Defines whether to install plugin dependencies.
- This option takes effect only if the O(version) is not defined.
type: bool
default: true

It was unintuitive for users expecting dependencies to be installed regardless.
This updates the module to install dependencies when with_dependencies=true, even if a specific version is set.

3: #4419
This is a bug with reaching the custom updates_url. This module uses url_username and url_password (intended for Jenkins), so when a request is made with fetch_url, it uses those credentials for that request, which causes an authentication error.
This PR introduces two new parameters: updates_url_username and updates_url_password, allowing users to provide separate credentials for the plugin update source. It also dynamically assigns force_basic_auth to avoid assigning url_* to every request based on target URL.

• Bugfix Pull Request

jenkins_plugin

[ansibullbot]

cc @jtyr
click here for bot help

[felixfontein]

Thanks for your contribution!

This PR introduces two new parameters: updates_url_username and updates_url_password, allowing users to provide separate credentials for the plugin update source. It also replaces fetch_url() with open_url() to update site requests to ensure that Jenkins server credentials are not unintentionally applied to those requests.

Right now url_username and url_password are used for both requests (I think), so changing that is a breaking change and won't get merged. You need a fallback to using url_username and url_password if no credentials are specified for the new options.

[felixfontein]

Note that changing from fetch_url to open_url can easily introducing breaking changes, since fetch_url does quite a few things implicitly (and what it does can also depend on the ansible-core version). Switching while still using the ansible.builtin.url docs fragment and the url_argument_spec() function is generally a very bad idea, since you then need to handle all changes to ansible-core in this collection as well. I would really stick with fetch_url. (You then have to modify module.params to pass over the right credentials, though...)

[YoussefKhalidAli]

I reverted the changes back to fetch_url. I want to avoid modifying the module.params unless absolutely necessary, like adding the updates_url_* parameters. To get around the credentials injection, I set force_basic_auth=False when making requests to non-Jenkins servers. I replicated the issue by setting up my own Nexus server and used it as updates_url. This method worked, but I’m not sure if using force_basic_auth=False like this is the preferred solution. Would it be better to keep force_basic_auth=True and instead refactor how credentials are handled or is this an acceptable approach?

[YoussefKhalidAli]

Or maybe set force_basic_auth=True and save the Jenkins and external credentials internally and switch between them by dynamically by setting url_username and url_password based on the target server.

[russoz]

hi @YoussefKhalidAli

Thanks for your continued submissions, those are much appreciated. Here a couple of initial comments - I glanced very quickly through the PR though, so possibly more to come later on.

[russoz]

This seems to be completely replacing the module's params with this customised dict. I would not recommend that.

[russoz]

Checking the file's mtime after it's been open makes no sense.

This would perhaps look better as:

Suggested change

	try: # Check if file is saved localy
	with open(cache_path, "r") as f:
	file_mtime = os.path.getmtime(cache_path)
	now = time.time()
	if now - file_mtime < 86400:
	plugin_data = json.load(f)
	else:
	raise FileNotFoundError("Cache file is outdated.")
	except Exception:
	response, info = fetch_url(self.module, "https://updates.jenkins.io/current/plugin-versions.json") # Get list of plugins and their dependencies
	if info['status'] != 200:
	self.module.fail_json(msg="Failed to fetch plugin-versions.json", details=info)
	try:
	plugin_data = json.loads(to_native(response.read()), object_pairs_hook=OrderedDict)
	# Save it to file for next time
	with open(cache_path, "w") as f:
	json.dump(plugin_data, f)
	try: # Check if file is saved localy
	file_mtime = os.path.getmtime(cache_path)
	now = time.time()
	if now - file_mtime >= 86400:
	response, info = fetch_url(self.module, "https://updates.jenkins.io/current/plugin-versions.json")
	if info['status'] != 200:
	self.module.fail_json(msg="Failed to fetch plugin-versions.json", details=info)
	plugin_data = json.loads(to_native(response.read()), object_pairs_hook=OrderedDict)
	# Save it to file for next time
	with open(cache_path, "w") as f:
	json.dump(plugin_data, f)
	with open(cache_path, "r") as f:
	plugin_data = json.load(f)
	except Exception:
	self.module.fail_json(...)

[russoz]

Hi @YoussefKhalidAli sorry I wasn't fast enough, I started this review earlier today but got sidetracked.

I spotted a typo and something else that could need a fix.

[russoz]

This can be simplified to:

Suggested change

	module.exit_json(changed=changed, plugin=name, state=state, dependencies=jp.dependencies_states if hasattr(jp, 'dependencies_states') else None)
	module.exit_json(changed=changed, plugin=name, state=state, dependencies=getattr(jp, 'dependencies_states', None))

[russoz]

I left a suggestion but is not a show stopper. LGTM