cp_gaia_snmp contact/location responses contain escaped characters

[acbk2b]

Issue

Primary issue

The cp_gaia_snmp module wraps the /{show,set}-snmp API endpoint, which returns a basic JSON structure. However, a discrepancy exists between the contact/location fields for this API endpoint.
The API response for /show-snmp contains escaped : characters, while the request body for /set-snmp does not require the : to be escaped. Adding in escape character results in the API response returning double-escaped characters.

Example:

• Input: ::TA:test-contact-00000000
• API response: \\:\\:TA\\:test-contact-00000000

Using \\:\\:TA\\:test-contact-00000000 as the input then yields: \\\\\\:\\\\\\:TA\\\\\\:test-contact-00000000 (double escapes).
This discrepancy also causes the cp_gaia_snmp module to always run /set-snmp due to the module parameters and the API response to always be in conflict.

Secondary issue

The above discrepancy also uncovered a secondary issue with chkp_api_call, which computes whether the set operation actually caused a difference in the output. In the case of snmp, before/after will match (both contain escapes) which in turn causes the module to report changed=False despite /set-snmp being called.

This is incorrect behavior - a set operation does constitute a change. Debugging is also made more difficult by the fact that the module does not make it apparent that /set-snmp was actually called.

Example

• Input: ::TA:test-contact-00000000
• before API response: \\:\\:TA\\:test-contact-00000000
• /set-snmp called
• after API response: \\:\\:TA\\:test-contact-00000000
• before == after # True -> module returns change=False

Fix

Primary issue

Ideally, I think the /{set,show}-snmp API endpoints should be updated to be consistent - either both require escape characters or neither do.

However, in the interim a short-term fix can be applied using compare_params, like so:

def _escape_snmp_string(value):
    """Escape special characters the way the Gaia API stores them."""
    if value is None:
        return None
    return value.replace('\\', '\\\\').replace(':', '\\:')

# Bugfix: The CheckPoint Gaia API returns `contact` with the `:` characters escaped -> `\:`
#         This causes the module parameters to _never_ match, thus `compare_params` must be
#          utilized to escape to match the API output for `idempotency_check`

# Get clean checkpoint params
compare_params = {k: _clean_params(v) for k, v in module.params.items() if is_checkpoint_param(k) and v is not None}
# escape contact/location strings
contact = module.params.get('contact')
location = module.params.get('location')
if contact is not None:
    compare_params['contact'] = _escape_snmp_string(contact)
if location is not None:
    compare_params['location'] = _escape_snmp_string(location)

res = chkp_api_call(module, api_call_object, False, compare_params=compare_params)

This escapes the input params for contact/location and passes them to compare_params for chkp_api_call to compare against the API response from /show-snmp

Secondary

checkpoint.py can simply be updated to return changed=True by default - this branch is only reachable after /set-* or /add-* has been called.

Current logic:

after = _strip_ignore(res, ignore)
changed = False if before == after else True

return {
    api_call_object.replace('-', '_'): res,
    "changed": changed
}

Updated

return {
    api_call_object.replace('-', '_'): res,
    "changed": True
}

---

I am happy to help contribute a fix for this issue via pull-request.
Thanks!