URL: https://github.com/SSSD/sssd/pull/66 Author: justin-stephenson Title: #66: Minor Dynamic DNS fixes Action: opened
PR body: """ To provide a bit more information, one of the fixes is to correct NULL being printed here(https://fedorahosted.org/sssd/ticket/3220):
[nsupdate_msg_create_common] (0x0200): Creating update message for realm [(null)].
For the other(https://bugzilla.redhat.com/show_bug.cgi?id=1386748), It is not uncommon for nsupdate to successfully update DNS records but report the error below which results in return(2) to be called inside nsupdate code
TSIG error with server: tsig verify failure
It is easy to reproduce with AD DNS changing Dynamic DNS to 'Nonsecure and secure' on the Zone Properties.
This patch allows PTR records to continue when this happens, however in this case our debug log messages still report failure and I think some improvement should be made here(not sure how exactly though)
[child_sig_handler] (0x1000): Waiting for child [3710]. [nsupdate_child_handler] (0x0040): Dynamic DNS child failed with status [512] [child_sig_handler] (0x0020): child [3710] failed with status [2]. [be_nsupdate_done] (0x0040): nsupdate child execution failed [1432158238]: Dynamic DNS update failed
It would be nice to correct this at the nsupdate level if this is not the expected behavior also. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/66/head:pr66 git checkout pr66
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-256044447
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-256044443
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
jhrozek commented: """ ok to test """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-256084328
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
jhrozek commented: """ Thank you for the patch. I wonder if you were actually able to reproduce the bug? If so, can you help me reproduce it locally? """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-257821917
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
justin-stephenson commented: """ Please try reproducing this on a SSSD - AD provider configured system using AD as a DNS server with the below steps:
1. In the AD Properties of a DNS Forward Zone, change `Dynamic Updates` drop-down to **Nonsecure and secure** 2. Remove A/AAAA and PTR records from Active Directory DNS for the SSSD system 3. Restart SSSD to trigger the nsupdate call 4. Check the reverse zone in AD for PTR records, they do not get created but the A/AAAA records do
With the patch, both forward and reverse records should be created, IIRC this was reproducible with Windows Server 2008 R2 and Server 2012. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-257863914
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
lslebodn commented: """ @jhrozek Do you pan to review the patch? """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-262520613
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
jhrozek commented: """ On Wed, Nov 23, 2016 at 06:05:10AM -0800, lslebodn wrote:
@jhrozek Do you pan to review the patch?
Feel free to take over the review, I'm currently busy with other work.
"""
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-262529286
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
lslebodn commented: """ I let fellow developers to review it. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-262816234
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
justin-stephenson commented: """ The commit `DYNDNS: Update PTR record after non-fatal error` has been successfully tested and confirmed fixed by a SSSD user with RHEL 6 and RHEL7 systems. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-279439152
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
fidencio commented: """ @justin-stephenson,
Firstly, sorry this patch has been waiting for so long. I'll try to reproduce the issue and review it Tomorrow. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-280395279
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
jhrozek commented: """ btw I'm fine with the patch, I was just waiting for Justin's confirmation that the patch indeed helps the customer.
But please, go ahead and review it again, I only scrolled through the patch so a second review wouldn't hurt. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-280404896
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
justin-stephenson commented: """ Thank you @fidencio @jhrozek 2 different customers confirmed this fix addressed their issue with the PTR record not being created. If you are not able to easily reproduce easily then perhaps this is sufficient enough of a test. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-280408727
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
fidencio commented: """ @justin-stephenson:
I've tried to reproduce the issue but, so far, I've failed (mainly due to a lack of knowledge). Let me drop a few questions in order to try to understand better how to setup my AD server here and how things are related to each other on server side.
1) How is the Reverse Lookup Zone created at the first place? 2) How is the PTR record created at the first place? 3) When you click in the A/AAAA SSSD entry, is the "Update associated pointer (PTR) record" checkbox enabled? 3.1) In case the checkbox is enabled, is the patch still needed? 4) Do we know, from SSSD side, what's the value of this in the AD server?
So far I'm afraid we may change on SSSD's side something that is responsibility of the server. In other words, that we may end up doing a workaround to solve a mis-configured server (and I really would like to avoid this situation). """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-281000617
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
justin-stephenson commented: """ @fidencio No problem, thanks for looking into this.
1. Once the DNS role is installed, create a Reverse Zone in the AD DNS MMC(right click Reverse-Lookup zones and click New Zone) 2. The idea is that SSSD will create the forward A and reverse zone PTR records when nsupdate gets called, the testing of this PR would be done with no existing PTR record for this SSSD system. 3. Yes it is checked, I believe this checkbox only tells AD the update the IP address of the PTR record when the forward A record address is manually updated but this checkbox should not be relevant when there is no existing PTR record 3.1) Yes, because the problem is nsupdate will still not create the Reverse PTR record when the TSIG error is encountered 4. Sorry, I don't quite follow this last question.
The root problem here is actually that the error `TSIG error with server: tsig verify failure` can happen within nsupdate code even when the A/AAAA record is successfully added. When this happens, the nsupdate child process returns non-zero which causes SSSD to consider it a failure and abort the PTR record update operation. I believe there are various reasons which can cause the error mentioned above but I was able to reproduce one of them with the reproducer steps mentioned here in the PR.
This fix will ignore the non-zero update and try to update the PTR record regardless of the nsupdate return code. I am planning on submitting a bug to nsupdate to address the root issue of why we have the TSIG error but in the meantime I think it would be useful to make SSSD more capable of handling updates even when there are errors which could happen with nsupdate.
Actually, the ticket **https://fedorahosted.org/sssd/ticket/3227** has a comment from pspacek `If the update failed, the records will be incorrect in any case. Personally I think we should try to update all record types and let admins to deal with potential inconsistencies. `
Sorry for the long comment, feel free to continue the discussion on IRC or if you have any questions let me know. Thank you @fidencio ! """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-281097299
URL: https://github.com/SSSD/sssd/pull/66 Author: justin-stephenson Title: #66: Minor Dynamic DNS fixes Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/66/head:pr66 git checkout pr66
URL: https://github.com/SSSD/sssd/pull/66 Author: justin-stephenson Title: #66: Minor Dynamic DNS fixes Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/66/head:pr66 git checkout pr66
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
fidencio commented: """ @justin-stephenson: Thanks for the patches and explanation. I was able to reproduce the behavior you mentioned and, IMO, the patches look fine.
I've just submitted them to our internal CI and as soon as we get the results I'll ACK them. """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-281193722
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
fidencio commented: """ CI: http://sssd-ci.duckdns.org/logs/job/63/03/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-281198976
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
jhrozek commented: """ * master: * d694d4fdcc81f24c2f9e3bb5a0dbe0a52498f196 * fccd8f9ab7a0ac9868c43ea0e8c3af142b2809fa """
See the full comment at https://github.com/SSSD/sssd/pull/66#issuecomment-281643200
URL: https://github.com/SSSD/sssd/pull/66 Title: #66: Minor Dynamic DNS fixes
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/66 Author: justin-stephenson Title: #66: Minor Dynamic DNS fixes Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/66/head:pr66 git checkout pr66
sssd-devel@lists.fedorahosted.org
-
fidencio -
jhrozek
-
justin-stephenson