fix(netconf_config.py): type conversion issue for confirm_timeout

[jclaerhout]

SUMMARY

This pull request fixes a type conversion issue in the netconf_config module of the ansible.netcommon collection.

When using the confirm parameter, Ansible throws an error due to a type mismatch:

Argument must be bytes or unicode, got 'int'

This occurs because confirm_timeout, which is expected to be an integer, is not properly converted before being passed to the NETCONF commit operation.

The fix ensures confirm_timeout is explicitly converted to a string using to_text() with errors='surrogate_or_strict' before passing it to the commit operation.

Fixes #551

---

ISSUE TYPE

• Bugfix Pull Request

---

COMPONENT NAME

ansible.netcommon.netconf_config

---

ADDITIONAL INFORMATION

The issue occurs when attempting to use confirm in ansible.netcommon.netconf_config, leading to a failure due to incorrect type handling.

Code Change (Line 669 of netconf_config.py):

- confirmed=confirmed_commit, timeout=confirm_timeout
+ confirmed=confirmed_commit, timeout=to_text(confirm_timeout, errors='surrogate_or_strict')

This fix ensures proper type handling and allows users to correctly perform confirmed commits with rollback timers.

References:

• Related issue: [ansible.netcommon#551](netconf_config false argument when using confirm_commit #551)
• Previous attempt to fix the issue: [ansible.netcommon#670](Bugfix: Fix commit timeout type failure issue #670)

Let me know if any further modifications are required! 🚀

[Dalle55]

Hi,
We've been testing it in a dedicated virtualenv on Cisco IOS XE devices, and would like to report a few important issues that surfaced — along with a working patch and reproduction steps.

Problems observed with the current PR implementation

1. to_text() called with unsupported keyword error

This line is problematic:

confirm = to_text(module.params["confirm"], error="surrogate_or_strict")

• to_text() from ansible.module_utils._text does not support the error keyword.
• It causes immediate runtime failure:

TypeError: to_text() got an unexpected keyword argument 'error'

Fix used in our local patch:

confirm = to_text(module.params["confirm"])

2. Confirm value not properly cast

If confirm is passed as an integer from the playbook (as in confirm: 10), Ansible will fail with:

Argument must be bytes or unicode, got 'int'

Fix applied:

We explicitly convert confirm to string before using it in any NETCONF RPC or XML operation:

confirm = str(to_text(module.params["confirm"]))

3. Confirmed commit not functioning properly on Cisco IOS XE

• Even after fixing type handling, the module logic assumes commit and confirm can be bundled into a single RPC.
• However, Cisco IOS XE does not support this behavior reliably.
• Sending <commit confirmed="true" timeout="10"/> does not work. Cisco expects a full <commit><confirmed/><confirm-timeout>10</confirm-timeout></commit> RPC sent separately.

Temporary workaround:

We split the commit logic into two steps using netconf_rpc manually after pushing config via netconf_config.

[Dalle55]

Correction regarding commit confirmed in Ansible's netconf_config module

RFC 6241, Section 8.4 (Confirmed Commit)

According to RFC 6241, section 8.4, the correct format for a confirmed commit operation in NETCONF is to use child elements, not attributes. Example:

<commit>
  <confirmed/>
  <confirm-timeout>10</confirm-timeout>
</commit>

However, the netconf_config module currently generates the following structure:

<commit confirmed="true" timeout="10"/>

This is not compliant with RFC 6241 and leads to errors on compliant NETCONF servers.

Real-World Observation

Cisco IOS XE correctly implements the confirmed commit functionality using the expected structure from the RFC. It rejects the attribute-based form because it is not valid NETCONF syntax.

This means:

• Cisco is acting correctly.
• The Ansible module is generating invalid XML and must be fixed.

Recommendation

Update the module to generate proper RFC-compliant commit RPCs using child elements for confirmed and confirm-timeout.

[tchadelicard]

Hello,

After a thorough review of the issue, I'd like to provide some context and clarification regarding the behavior of the commit operation for Cisco IOS-XE devices when using ncclient.

Background: IOS-XE and NETCONF commit support

To the best of my knowledge, Cisco IOS-XE does not support the standard NETCONF commit operation as defined by the :candidate and :confirmed-commit capabilities. This means that while the base NETCONF operations are available, full candidate configuration workflows (including confirmed commits) are not natively supported on IOS-XE in the same way as on other platforms like IOS-XR.

Analysis of ncclient Implementation

Despite this limitation, ncclient still attempts to perform a standard commit RPC when requested. The IosxeDeviceHandler in ncclient does not define a custom commit operation, and therefore falls back to the default Commit class implementation defined in the operations/edit.py module.

Here is the relevant part of the Commit class:

class Commit(RPC):
    DEPENDS = [':candidate']

    def request(self, confirmed=False, timeout=None, persist=None, persist_id=None):
        node = new_ele("commit")
        if confirmed:
            self._assert(":confirmed-commit")
            sub_ele(node, "confirmed")
            if timeout is not None:
                sub_ele(node, "confirm-timeout").text = timeout
        # ... other fields
        return self._request(node)

This generates a well-formed, RFC-compliant XML payload such as:

<commit>
  <confirmed />
  <confirm-timeout>120</confirm-timeout>
</commit>

The XML structure itself is correct - the issue does not lie in ncclient's implementation of the commit operation.

Root cause of the current bug

The actual problem occurs in Ansible’s netconf_config module, where the confirm_timeout parameter is passed as an integer, but the NETCONF RPC construction expects it to be a string.

Since ElementTree does not automatically convert non-string types to text content, assigning an integer directly (e.g., elem.text = 120) raises a TypeError:

TypeError: cannot serialize 120 (type int)

This causes the task to fail before the RPC is even sent - regardless of whether the device supports the operation.

Solution and testing

The fix proposed by @jclaerhout addresses this by explicitly converting confirm_timeout to a string before assignment. This is a minimal but necessary change to ensure type safety in XML construction.

I have tested this change on a device that does support candidate configurations and confirmed commit (e.g., SROS), and it works as expected - the correct XML is generated and the operation succeeds.

Conclusion

While IOS-XE may not support confirmed commits due to lack of :candidate capability, that is a platform limitation, not a bug in the code. However, the current failure in netconf_config occurs even on platforms that do support the feature, due to the type mismatch.

Therefore, I recommend merging the proposed change, as it:

• Fixes a real type-handling bug in the module
• Enables correct behavior on platforms that do support confirmed commits
• Does not introduce any regressions
• Maintains compatibility across devices

Without this fix, users are unable to use the commit confirm functionality even on supported platforms.

Could we please move forward with this merge request?

Thank you!