[BUG] Split zones require `.` following the domain

[boltronics]

Description

When using boto_route53.present with private_zone and split_dns boolean values set to True, the value for zone must be set to something like example.com. instead of example.com (note the trailing period). Otherwise, an error will be logged:

[ERROR   ] Failed to retrieve zone example.com

However, the state itself will return Result: True.

----------
          ID: Add example.com A record to route53
    Function: boto_route53.present
        Name: example.com
      Result: True
     Comment: 
     Started: 19:34:37.652180
    Duration: 120.899 ms
     Changes:   

This behaviour seems to be unique to a split DNS setup. Zones with a more common non-split setup do not seem to have such issues.

Setup

The rendered state (as shown in state.show_sls output, only with the domain changed):

[INFO    ] Running state [example.com] at time 19:15:44.791942
[INFO    ] Executing state boto_route53.present for [example.com]
[ERROR   ] Failed to retrieve zone example.com
[INFO    ] No changes made for example.com
[INFO    ] Completed state [example.com] at time 19:15:44.900371 (duration_in_ms=108.429)

...

    Add example.com A record to route53:
        ----------
        boto_route53:
            |_
              ----------
              name:
                  example.com
            |_
              ----------
              value:
                  172.0.0.1
            |_
              ----------
              zone:
                  example.com
            |_
              ----------
              record_type:
                  A
            |_
              ----------
              ttl:
                  60
            |_
              ----------
              region:
                  us-west-2
            |_
              ----------
              split_dns:
                  True
            |_
              ----------
              private_zone:
                  True
            - present
            |_
              ----------
              order:
                  10081
        __sls__:
            route53
        __env__:
            base

Workaround

    Add example.com. A record to route53:
        ----------
        boto_route53:
            |_
              ----------
              name:
                  example.com.
            |_
              ----------
              value:
                  172.0.0.1
            |_
              ----------
              zone:
                  example.com.
            |_
              ----------
              record_type:
                  A
            |_
              ----------
              ttl:
                  60
            |_
              ----------
              region:
                  us-west-2
            |_
              ----------
              split_dns:
                  True
            |_
              ----------
              private_zone:
                  True
            - present
            |_
              ----------
              order:
                  10081
        __sls__:
            route53
        __env__:
            base

When running the above under test mode:

----------
          ID: Add example.com. A record to route53
    Function: boto_route53.present
        Name: example.com.
      Result: None
     Comment: Route53 record example.com. set to be added.
     Started: 19:59:07.858168
    Duration: 16073.838 ms
     Changes:   

It now appears that it will work as intended.

Please be as specific as possible and give set-up details.

• [X] VM running on a cloud service (AWS EC2 instance on ARM64, Debian 12)
• [X] onedir packaging
• [ ] used bootstrap to install (used a salt-cloud deploy script)

Steps to Reproduce the behavior

Create a split DNS system in route53. ie. One Public zone and one Private zone of the same name (example.com in this example). Try to add an A record for example.com within, as per the above example. I already have an A record for example.com in the public zone, but I don't expect this to matter.

In the above example, I also added "example.com." as the record name, which is technically correct but AWS specifically does not require. I did that because it was easier for me to render my pillar data that way (using a custom Python script), so that's the way I tested it. It probably does not matter.

Expected behavior

Output as per the above work-around when ran under test mode. ie. A successful A resource record "example.com." added to the zone "example.com".

Perhaps separately to this report, I also expect to see Result: False when something has went wrong and did not apply. That does not seem to be happening here.

Versions Report

          Salt: 3006.8
 
Python Version:
        Python: 3.10.14 (main, Apr  3 2024, 21:33:04) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.3
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.16.0
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 12 bookworm
        locale: utf-8
       machine: aarch64
       release: 6.1.0-23-cloud-arm64
        system: Linux
       version: Debian GNU/Linux 12 bookworm

Additional context It was mentioned in https://github.com/saltstack/salt/pull/24182:

Separately, I noticed this state reports that it returns successfully in many cases where it should not. This added functionality does not change that behavior -- the return behavior remains the same.

At first I thought that this may be one of the problems rfairburn was referring to, although after some debugging, it seems that this issue was likely introduced in his PR.

I identified a problematic if condition by logging the values for both _zone.name and zone, and noticed that they were never matching due to the additional period at the end of _zone.name which zone (passed to the function as an argument) did not have.

This issue can probably be resolved by simply changing the if condition in boto_route53.py from

if _zone.name == zone:

to

if _zone.name.rstrip(".") == zone.rstrip("."):

However, I have not tested this.