ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
814 stars 1.49k forks source link

Cannot add multiple SRV records with nsupdate and subsequent invocations #3630

Open stephan2012 opened 2 years ago

stephan2012 commented 2 years ago

Summary

Cannot add multiple SRV records because latter updates remove all previous. Multiple values for a single invocation of nsupdate allows multiple values but this is not idiomatic for Ansible.

Here is how it looks like from the DNS server’s perspective:

Oct 29 14:31:50 n0211 named[708]: client @0x7f6678071970 192.168.100.66#38260/key ansible: signer "ansible" approved
Oct 29 14:31:50 n0211 named[708]: client @0x7f6680553ff0 192.168.100.66#38262/key ansible: updating zone 'lab.company.com/IN': update unsuccessful: _mysvc._tcp.lab.company.com/SRV: 'RRset exists (value dependent)' prerequisite not satisfied (NXRRSET)
Oct 29 14:31:50 n0211 named[708]: client @0x7f66800b84a0 192.168.100.66#38264/key ansible: signer "ansible" approved
Oct 29 14:31:50 n0211 named[708]: client @0x7f66800b84a0 192.168.100.66#38264/key ansible: updating zone 'lab.company.com/IN': deleting rrset at '_mysvc._tcp.lab.company.com' SRV
Oct 29 14:31:50 n0211 named[708]: client @0x7f66800b84a0 192.168.100.66#38264/key ansible: updating zone 'lab.company.com/IN': adding an RR at '_mysvc._tcp.lab.company.com' SRV 10 0 443 n0214.lab.company.com.
Oct 29 14:31:50 n0211 named[708]: zone lab.company.com/IN: sending notifies (serial 1618857170)
Oct 29 14:31:50 n0211 named[708]: client @0x7f66804d8560 192.168.8.52#41983 (lab.company.com): transfer of 'lab.company.com/IN': IXFR started (serial 1618857169 -> 1618857170)
Oct 29 14:31:50 n0211 named[708]: client @0x7f66804d8560 192.168.8.52#41983 (lab.company.com): transfer of 'lab.company.com/IN': IXFR ended
Oct 29 14:31:50 n0211 named[708]: client @0x7f6680528d90 192.168.8.53#45961 (lab.company.com): transfer of 'lab.company.com/IN': IXFR started (serial 1618857169 -> 1618857170)
Oct 29 14:31:50 n0211 named[708]: client @0x7f6680528d90 192.168.8.53#45961 (lab.company.com): transfer of 'lab.company.com/IN': IXFR ended
Oct 29 14:31:53 n0211 named[708]: client @0x7f66780512e0 192.168.100.66#38268/key ansible: signer "ansible" approved
Oct 29 14:31:53 n0211 named[708]: client @0x7f66804b7a30 192.168.100.66#38270/key ansible: updating zone 'lab.company.com/IN': update unsuccessful: _mysvc._tcp.lab.company.com/SRV: 'RRset exists (value dependent)' prerequisite not satisfied (NXRRSET)
Oct 29 14:31:53 n0211 named[708]: client @0x7f66804b8410 192.168.100.66#38272/key ansible: signer "ansible" approved
Oct 29 14:31:53 n0211 named[708]: client @0x7f66804b8410 192.168.100.66#38272/key ansible: updating zone 'lab.company.com/IN': deleting rrset at '_mysvc._tcp.lab.company.com' SRV
Oct 29 14:31:53 n0211 named[708]: client @0x7f66804b8410 192.168.100.66#38272/key ansible: updating zone 'lab.company.com/IN': adding an RR at '_mysvc._tcp.lab.company.com' SRV 10 0 443 n0216.lab.company.com.
Oct 29 14:31:55 n0211 named[708]: zone lab.company.com/IN: sending notifies (serial 1618857171)
Oct 29 14:31:55 n0211 named[708]: client @0x7f6680562610 192.168.8.52#52813 (lab.company.com): transfer of 'lab.company.com/IN': IXFR started (serial 1618857170 -> 1618857171)
Oct 29 14:31:55 n0211 named[708]: client @0x7f6680562610 192.168.8.52#52813 (lab.company.com): transfer of 'lab.company.com/IN': IXFR ended
Oct 29 14:31:55 n0211 named[708]: client @0x7f668051a770 192.168.8.53#45335 (lab.company.com): transfer of 'lab.company.com/IN': IXFR started (serial 1618857170 -> 1618857171)
Oct 29 14:31:55 n0211 named[708]: client @0x7f668051a770 192.168.8.53#45335 (lab.company.com): transfer of 'lab.company.com/IN': IXFR ended

nsupdate currently allows records to be either present or absent. Maybe a force option is missing that allows changing the behavior from replace to merge.

Issue Type

Bug Report

Component Name

nsupdate

Ansible Version

$ ansible --version
ansible 2.9.27
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/au/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.6/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.6.9 (default, Jan 26 2021, 15:33:00) [GCC 8.4.0]

Community.general Version

Tested with Ansible v2.9 only, but the nsupdate code does not have changed for this particular topic.

Configuration

$ ansible-config dump --only-changed
ANSIBLE_PIPELINING(/etc/ansible/ansible.cfg) = True
DEFAULT_ASK_VAULT_PASS(/etc/ansible/ansible.cfg) = False
DEFAULT_JINJA2_NATIVE(/etc/ansible/ansible.cfg) = True
DEFAULT_VAULT_PASSWORD_FILE(env: ANSIBLE_VAULT_PASSWORD_FILE) = /home/au/.ansible_vault_password.txt

OS / Environment

Ubuntu 18.04.6 LTS, Ansible installed via pip

Steps to Reproduce

- name: "Register SRV records"
  nsupdate:
    key_name: '{{ my_key_name }}.'
    key_secret: '{{ my_secret }}'
    key_algorithm: '{{ my_algorithm }}'
    server: 192.168.1.1
    state: present
    zone: lab.domain.com
    record: '_mysvc._tcp.lab.domain.com.'
    type: SRV
    value: '10 0 443 {{ ansible_fqdn }}.'
    ttl: 3600

Run this task in a playbook with two or more hosts.

Expected Results

nsupdate should delete resouce records only if requested or sensible (e.g., PTR records).

Actual Results

ok: [n0214 -> localhost] => (item={'record': '_mysvc._tcp', 'value': ['10 0 443 n0214.lab.company.com.']}) => {
  "ansible_loop_var": "item",
  "changed": false,
  "dns_rc": 0,
  "dns_rc_str": "NOERROR",
  "invocation": {
      "module_args": {
          "key_algorithm": "hmac-sha512",
          "key_name": "ansible.",
          "key_secret": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
          "port": 53,
          "protocol": "tcp",
          "record": "_mysvc._tcp.lab.company.com.",
          "server": "192.168.8.51",
          "state": "present",
          "ttl": 3600,
          "type": "SRV",
          "value": [
              "10 0 443 n0214.lab.company.com."
          ],
          "zone": "lab.company.com"
      }
  },
  "item": {
      "record": "_mysvc._tcp",
      "value": [
          "10 0 443 n0214.lab.company.com."
      ]
  },
  "record": {
      "record": "_mysvc._tcp.lab.company.com.",
      "ttl": 3600,
      "type": "SRV",
      "value": [
          "10 0 443 n0214.lab.company.com."
      ],
      "zone": "lab.company.com."
  }
}

Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 2 years ago

cc @nerzhul click here for bot help

felixfontein commented 2 years ago

I don't think this is a bug. This behavior is what's documented and what I would expect from using this module (disclaimer: I never used it). Allowing to only add entries (instead of replacing) would be a new feature.

The name force for such an option is a bad choice; force usually has different meanings for modules. I guess merge or update (boolean) would be better.

russoz commented 2 years ago

Agree, this is a feature request not a bug.

ansibullbot commented 1 year ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

Keeper-of-the-Keys commented 1 year ago

I was just looking at the code and tested a bit, the place the change would probably need to be made is around here.

Also from my testing the behavior of the module is actually bad/wrong because if multiple records already exist it will notice this and delete all leaving you with a single record.

Having multiple A/AAAA/MX/SRV/TXT records with different values is a fairly standard thing and though there are records for which the adage "there can only be one" applies this is not always the case.

Steps to reproduce:

  1. Generate multiple A records for the same host for instance with the following nsupdate script:
    server [dns server]
    zone [your zone]
    update add multirecordtest.[your domain]. A 10.0.0.1
    update add multirecordtest.[your domain]. A 10.0.0.2
    update add multirecordtest.[your domain]. A 10.0.0.3
    send
  2. Now run community.general.nsupdate and request an A record for 10.0.0.4 to also be created

Expected result - 4 A records 10.0.0.1-10.0.0.4 Actual results - only record 10.0.0.4

I realize that this does add some more complication to the nsupdate module but I do think it is a change that needs to happen.

Keeper-of-the-Keys commented 1 year ago

IMHO the state should be expanded to be {present|absent|update} where:

  1. present - creates a record if none exists or adds a second one if the existing record has different data
  2. absent - deletes a record if the data matches the provided data or all records if no data is provided
  3. update - the current behavior of present with a warning message that it will delete any additional records that may exist, or a failure if multiple records are detected.
Keeper-of-the-Keys commented 1 year ago

Based on some conversations on IRC with @flowerysong and zoredache I would suggest an alternative solution:

Add another parameter that controls whether the module treats the tuple(record+type) or the triple(record+type+data) as the unique key.

This would be close to the solution @felixfontein proposed also.