community.general icon indicating copy to clipboard operation
community.general copied to clipboard

Fix nsupdate when updating NS record

Open lungj opened this issue 2 years ago • 6 comments

SUMMARY

Fixed a bug where one cannot update an existing NS record.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nsupdate

ADDITIONAL INFORMATION

In order to determine if any changes occurred, the nsupdate component queries the TTL of the resulting record.

When querying A, AAAA, SOA, or CNAME records, one gets something like this in the ANSWER section:

;; ANSWER SECTION:
github.com.		60	IN	A	140.82.113.3

Here, we can see a TTL of 60 seconds.

However, an NS record and its TTL appears in the AUTHORITY section:

;; AUTHORITY SECTION:
github.com.		900	IN	NS	dns1.p08.nsone.net.
github.com.		900	IN	NS	dns2.p08.nsone.net.
github.com.		900	IN	NS	dns3.p08.nsone.net.
github.com.		900	IN	NS	dns4.p08.nsone.net.

The nsupdate component only searches for TTL in the ANSWER section. Thus, when updating NS records, the nsupdate component reads the wrong section for a TTL. When no ANSWER section records exist, the nsupdate component outright fails due to an index out of bounds error.

This bug fix reads the AUTHORITY section when an NS request is made.

lungj avatar Aug 12 '22 06:08 lungj

cc @nerzhul click here for bot help

ansibullbot avatar Aug 12 '22 06:08 ansibullbot

Hmm, depending on the name server and the domain queried, it also shows up in the answer section. How about first looking in the answer section, and if it's not there, then use the authority section?

felixfontein avatar Aug 13 '22 09:08 felixfontein

Hmm, depending on the name server and the domain queried, it also shows up in the answer section. How about first looking in the answer section, and if it's not there, then use the authority section?

Thanks for the review again! The TTLs do not need to match between ANSWER and AUTHORITY. In fact, in many cases, A and NS records probably differ in TTL, as the actual (abridged) github.com records do. Using a fallback could lead to an incorrect return value from this function.

The nsupdate component compares the desired TTL with the TTL from the DNS query to see if they match; when trying to set an NS record, we should only be looking at the NS records' TTLs. Otherwise, whenever we call this module on an NS record, we are likely to have it report a change because the desired TTL in the AUTHORITY section will not match the TTL of anything in the ANSWER section. For example, if we have this:

;; ANSWER SECTION:
github.com.		60	IN	A	140.82.113.3
;; AUTHORITY SECTION:
github.com.		900	IN	NS	dns1.p08.nsone.net.

and we call this:

- community.general.nsupdate:
    key_name: "nsupdate"
    key_secret: "+bFQtBCta7j2vWkjPkAFtgA=="
    server: "127.0.0.1"
    zone: "github.com"
    record: "github.com."
    type: "NS"
    value:
      - "dns1.p08.nsone.net"
    ttl: 900

if we use the ANSWER value and then fall back to AUTHORITY, this component would say that 60 != 900 (line 431) and thus that there is a change -- when, in fact, no change is requested.

lungj avatar Aug 13 '22 22:08 lungj

Hmm, depending on the name server and the domain queried, it also shows up in the answer section. How about first looking in the answer section, and if it's not there, then use the authority section?

Thanks for the review again! The TTLs do not need to match between ANSWER and AUTHORITY. In fact, in many cases, A and NS records probably differ in TTL, as the actual (abridged) github.com records do. Using a fallback could lead to an incorrect return value from this function.

I was not talking about the TTL of A records in the answer, but of the NS records in the answer. If you query for a domain's NS records, you should get the correct TTL in the answer to that request.

For example dig -t NS github.com gives me

;; ANSWER SECTION:
github.com.		3600	IN	NS	dns3.p08.nsone.net.
github.com.		3600	IN	NS	ns-520.awsdns-01.net.
[... several more ...]

;; AUTHORITY SECTION:
github.com.		3600	IN	NS	ns-520.awsdns-01.net.
github.com.		3600	IN	NS	dns1.p08.nsone.net.
[... several more ...]

After all the code does query for records of the type it's interested in: https://github.com/ansible-collections/community.general/pull/5112/files#diff-b86a495028fd26a8d2fbf68b85320b9792958611523d19ad00b12b429371193fR413

felixfontein avatar Aug 14 '22 10:08 felixfontein

Hmm, depending on the name server and the domain queried, it also shows up in the answer section. How about first looking in the answer section, and if it's not there, then use the authority section?

Thanks for the review again! The TTLs do not need to match between ANSWER and AUTHORITY. In fact, in many cases, A and NS records probably differ in TTL, as the actual (abridged) github.com records do. Using a fallback could lead to an incorrect return value from this function.

I was not talking about the TTL of A records in the answer, but of the NS records in the answer. If you query for a domain's NS records, you should get the correct TTL in the answer to that request.

For example dig -t NS github.com gives me

;; ANSWER SECTION:
github.com.		3600	IN	NS	dns3.p08.nsone.net.
github.com.		3600	IN	NS	ns-520.awsdns-01.net.
[... several more ...]

;; AUTHORITY SECTION:
github.com.		3600	IN	NS	ns-520.awsdns-01.net.
github.com.		3600	IN	NS	dns1.p08.nsone.net.
[... several more ...]

After all the code does query for records of the type it's interested in: https://github.com/ansible-collections/community.general/pull/5112/files#diff-b86a495028fd26a8d2fbf68b85320b9792958611523d19ad00b12b429371193fR413

Thanks for clarifying! I've made the patch sinceI'm not versed enough in all the differences in behaviour of different DNS servers. One of the other use cases for the nsupdate module is for updating a zone that contains subdomains so we're interested in results of dig -t NS github.com @l.gtld-servers.net. (assuming we're the administrators of the .com TLD), but the fallback to AUTHORITY you suggested works in this situation since there is no ANSWER section given.

lungj avatar Aug 14 '22 18:08 lungj

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: syntax-error: EOL while scanning string literal (<unknown>, line 430)

The test botmeta failed with 1 error:

plugins/modules/net_tools/nsupdate.py:0:0: Cannot load DOCUMENTATION: EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:0:0: python-syntax-error: Python SyntaxError while parsing module

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module community.general.nsupdate" returned exit status 1.
>>> Standard Error
ERROR! module community.general.nsupdate missing documentation (or could not parse documentation): EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: python-syntax-error: unterminated string literal (detected at line 430) (<unknown>, line 430)

The test ansible-test sanity --test import --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: traceback: SyntaxError: unterminated string literal

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test compile --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: python-syntax-error: EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test import --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: traceback: SyntaxError: unterminated string literal

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test compile --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: python-syntax-error: unterminated string literal (detected at line 430) (<unknown>, line 430)

The test ansible-test sanity --test import --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.11 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: traceback: SyntaxError: unterminated string literal

The test ansible-test sanity --test import --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: traceback: SyntaxError: unterminated string literal

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test compile --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.11 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.10 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: syntax-error: unterminated string literal (detected at line 430) (<unknown>, line 430)

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: python-syntax-error: EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test import --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: traceback: SyntaxError: EOL while scanning string literal

The test ansible-test sanity --test compile --python 3.9 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.8 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 3.5 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.7 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:45: SyntaxError: if self.module.params['type'] == 'NS:

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:0:0: python-syntax-error: Python SyntaxError while parsing module

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module community.general.nsupdate" returned exit status 1.
>>> Standard Error
ERROR! module community.general.nsupdate missing documentation (or could not parse documentation): unterminated string literal (detected at line 430) (<unknown>, line 430)

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:0:0: python-syntax-error: Python SyntaxError while parsing module

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:0:0: python-syntax-error: Python SyntaxError while parsing module

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Command "ansible-doc -t module community.general.nsupdate" returned exit status 1.
>>> Standard Error
ERROR! module community.general.nsupdate missing documentation (or could not parse documentation): EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:46: syntax-error: EOL while scanning string literal (<unknown>, line 430)

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/net_tools/nsupdate.py:430:42: syntax-error: unterminated string literal (detected at line 430) (<unknown>, line 430)

click here for bot help

ansibullbot avatar Aug 14 '22 19:08 ansibullbot

I'll merge this weekend if nobody complains.

felixfontein avatar Aug 18 '22 11:08 felixfontein

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/ad8965218d820f842e84b856d4de5ae7111645e3/pr-5112

Backported as https://github.com/ansible-collections/community.general/pull/5131

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

patchback[bot] avatar Aug 20 '22 11:08 patchback[bot]

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/ad8965218d820f842e84b856d4de5ae7111645e3/pr-5112

Backported as https://github.com/ansible-collections/community.general/pull/5132

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

patchback[bot] avatar Aug 20 '22 11:08 patchback[bot]

@lungj thanks for fixing this! @russoz thanks for reviewing!

felixfontein avatar Aug 20 '22 11:08 felixfontein

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

github-actions[bot] avatar Aug 20 '22 11:08 github-actions[bot]