New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC2136 fixes #6858

Merged
merged 6 commits into from Aug 21, 2018

Conversation

Projects
None yet
3 participants
@Habbie
Member

Habbie commented Aug 16, 2018

Short description

This fixes:

  • when updating a record in a child zone that also exists in the parent zone, we would incorrectly apply the update to the parent zone
  • two of the lookup calls would not always be followed by a full get consumption cycle
  • sort -V is a non-portable GNUism

This needs:

  • a test for the child/parent bug

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@Habbie Habbie changed the title from [WIP] RFC2136 fixes to RFC2136 fixes Aug 16, 2018

@Habbie Habbie added this to the auth-4.1.x milestone Aug 16, 2018

@Habbie Habbie changed the title from RFC2136 fixes to [WIP] RFC2136 fixes Aug 16, 2018

@Habbie

This comment has been minimized.

Show comment
Hide comment
@Habbie

Habbie Aug 16, 2018

Member

1dyndns-correct-zone also passes on master, so it's a bad test.

Member

Habbie commented Aug 16, 2018

1dyndns-correct-zone also passes on master, so it's a bad test.

@Habbie

This comment has been minimized.

Show comment
Hide comment
@Habbie

Habbie Aug 17, 2018

Member

1dyndns-correct-zone also passes on master, so it's a bad test.

Fixed the test and reordered the commits to make that easy to verify.

Member

Habbie commented Aug 17, 2018

1dyndns-correct-zone also passes on master, so it's a bad test.

Fixed the test and reordered the commits to make that easy to verify.

@pieterlexis pieterlexis self-requested a review Aug 17, 2018

@Habbie Habbie changed the title from [WIP] RFC2136 fixes to RFC2136 fixes Aug 17, 2018

@pieterlexis

This comment has been minimized.

Show comment
Hide comment
@pieterlexis

pieterlexis Aug 17, 2018

Member

Tests broken?

[2018-08-17 11:41:35] verify-dnssec-zone
[2018-08-17 11:41:35] --- ./tests/verify-dnssec-zone/expected_result	2018-08-17 11:19:26.567911034 +0000
[2018-08-17 11:41:35] +++ ./tests/verify-dnssec-zone/real_result	2018-08-17 11:41:35.407705482 +0000
[2018-08-17 11:41:35] @@ -41,7 +41,7 @@
[2018-08-17 11:41:35]  RETVAL: 0
[2018-08-17 11:41:35]  
[2018-08-17 11:41:35]  --- named-checkzone sub.test.dyndns
[2018-08-17 11:41:35] -zone sub.test.dyndns/IN: loaded serial 2018081702 (DNSSEC signed)
[2018-08-17 11:41:35] +zone sub.test.dyndns/IN: loaded serial 2012060701 (DNSSEC signed)
[2018-08-17 11:41:35]  OK
[2018-08-17 11:41:35]  RETVAL: 0
[2018-08-17 11:41:35] ****
Member

pieterlexis commented Aug 17, 2018

Tests broken?

[2018-08-17 11:41:35] verify-dnssec-zone
[2018-08-17 11:41:35] --- ./tests/verify-dnssec-zone/expected_result	2018-08-17 11:19:26.567911034 +0000
[2018-08-17 11:41:35] +++ ./tests/verify-dnssec-zone/real_result	2018-08-17 11:41:35.407705482 +0000
[2018-08-17 11:41:35] @@ -41,7 +41,7 @@
[2018-08-17 11:41:35]  RETVAL: 0
[2018-08-17 11:41:35]  
[2018-08-17 11:41:35]  --- named-checkzone sub.test.dyndns
[2018-08-17 11:41:35] -zone sub.test.dyndns/IN: loaded serial 2018081702 (DNSSEC signed)
[2018-08-17 11:41:35] +zone sub.test.dyndns/IN: loaded serial 2012060701 (DNSSEC signed)
[2018-08-17 11:41:35]  OK
[2018-08-17 11:41:35]  RETVAL: 0
[2018-08-17 11:41:35] ****

@Habbie Habbie changed the title from RFC2136 fixes to [WIP] RFC2136 fixes Aug 17, 2018

@Habbie Habbie changed the title from [WIP] RFC2136 fixes to RFC2136 fixes Aug 18, 2018

@Habbie

This comment has been minimized.

Show comment
Hide comment
@Habbie

Habbie Aug 18, 2018

Member

Tests broken?

Yes, fixed now!

Member

Habbie commented Aug 18, 2018

Tests broken?

Yes, fixed now!

@rgacogne

Looks good, very nice job with the tests!

Show outdated Hide outdated pdns/rfc2136handler.cc
@pieterlexis

Nice and easy fix :)

pieterlexis added a commit to pieterlexis/pdns that referenced this pull request Aug 20, 2018

@pieterlexis pieterlexis merged commit de9a4eb into PowerDNS:master Aug 21, 2018

4 checks passed

LGTM analysis: C/C++ No alert changes
Details
LGTM analysis: JavaScript No alert changes
Details
LGTM analysis: Python No alert changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

pieterlexis added a commit to pieterlexis/pdns that referenced this pull request Aug 21, 2018

@pieterlexis pieterlexis referenced this pull request Aug 21, 2018

Merged

Authoritative server 4.1.4 backports #6866

4 of 8 tasks complete

@Habbie Habbie deleted the Habbie:rfc2136-correct-zone branch Aug 22, 2018

@PowerDNS PowerDNS deleted a comment from savagegeek Aug 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment