Fix root user authentication

[rohitthakur2590]

SUMMARY

ISSUE TYPE

• Bugfix Pull Request
• Docs Pull Request
• Feature Pull Request
• New Module Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.58%. Comparing base (a18d41a) to head (1116643).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #503   +/-   ##
=======================================
  Coverage   82.58%   82.58%           
=======================================
  Files         139      139           
  Lines       13128    13128           
=======================================
  Hits        10842    10842           
  Misses       2286     2286           

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/507335e796dd4619b736caa39dee1211

❌ ansible-test-network-integration-junos-vsrx-netconf-python39 RETRY_LIMIT in 8m 48s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-netconf-python36-stable211 SUCCESS in 1h 16m 48s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-netconf-python39-stable212 SUCCESS in 1h 04m 31s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-netconf-python36-stable29 FAILURE in 17m 11s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-network_cli-python39 RETRY_LIMIT in 9m 00s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-python39-stable212 SUCCESS in 14m 31s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-python36-stable211 SUCCESS in 16m 44s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-network_cli-python36-stable29 FAILURE in 12m 49s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python39 RETRY_LIMIT in 9m 09s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python39-stable212 SUCCESS in 14m 49s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python36-stable29 FAILURE in 14m 49s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python36-stable211 SUCCESS in 16m 32s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-netconf-python39-stable215 SUCCESS in 1h 09m 41s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-python39-stable215 SUCCESS in 15m 27s (non-voting)
❌ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python39-stable215 FAILURE in 24m 29s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-netconf-python39-stable214 SUCCESS in 1h 06m 00s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-python39-stable214 SUCCESS in 15m 46s (non-voting)
✔️ ansible-test-network-integration-junos-vsrx-network_cli-libssh-python39-stable214 SUCCESS in 15m 16s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 10s
✔️ ansible-tox-linters SUCCESS in 12m 54s
✔️ ansible-galaxy-importer SUCCESS in 4m 45s

[danny-burrows]

I'm very interested in seeing a fix for this merged if possible. I also think the addition of a test for this case is valuable.

I do think, however, that this fix can be simplified; you could set the auth variable once near the top of this block where you initially check for "root" special case, once this is handled you could treat this auth value as const (maybe even naming it AUTH as is the convention in Python) for the rest of the block and just use it as needed. Effectively you'd check once at the top of the function for "root", then remove all other special-case handling for "root" and simply use the initial auth.

Hope this helps, would love for this PR to get some more love!

[jrandall]

It looks like this was originally broken in ansible/ansible#62340 (released in ansible v2.8.5) which was fixing an issue with aggregate mode.

I believe the bug at the time was that the original code (introduced in ansible v2.8.0) in which setting encrypted_password for root was working was guarding the setting of auth with a conditional that checked if auth had already been set (if 'auth' not in locals():).

I think this will not be an issue if auth is simply set once as @danny-burrows suggests.

I would suggest a fix in which both arms of the condition at

junipernetworks.junos/plugins/modules/junos_user.py

Lines 271 to 279 in b445729

	if item["name"] != "root":
	user = SubElement(login, "user", {"operation": operation})
	SubElement(user, "name").text = item["name"]
	else:
	user = auth = SubElement(
	element,
	"root-authentication",
	{"operation": operation},
	)

set both user and auth (and then neither should be set later in the code).

IMHO replacing that block with something like this would be a clean fix:

        if item["name"] == "root":
            user = auth = SubElement(
                element,
                "root-authentication",
                {"operation": operation},
            )
        else:
            user = SubElement(login, "user", {"operation": operation})
            SubElement(user, "name").text = item["name"]
            auth = SubElement(user, "authentication")