From 2347e7d3b9979533f471874919e1a39747bf4597 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Fri, 6 Jul 2018 21:47:32 +0200
Subject: [PATCH] Fix race condition in get_locations_records()

The method IPASystemRecords.get_locations_records() has a race condition.
The IPASystemRecords object creates a mapping of server names to server
data. get_locations_records() uses server_find() again to get a list of
servers, but then operates on the cached dict of server names.

In parallel replication case, the second server_find() call in
get_locations_records() can return additional servers. Since the rest of
the code operates on the cached data, the method then fails with a KeyError.

server_data is now an OrderedDict to keep same sorting as with
server_find().

Fixes: https://pagure.io/freeipa/issue/7566
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Tibor Dudlak <tdudlak@redhat.com>
---
 ipaserver/dns_data_management.py | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/ipaserver/dns_data_management.py b/ipaserver/dns_data_management.py
index e5987b4bd7b43d3920e9da917258153e448206b7..c4f204c4941984d74e442c51ea206f11c05db2b9 100644
--- a/ipaserver/dns_data_management.py
+++ b/ipaserver/dns_data_management.py
@@ -6,7 +6,7 @@ from __future__ import absolute_import
 
 import six
 
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 from dns import (
     rdata,
     rdataclass,
@@ -68,7 +68,7 @@ class IPASystemRecords(object):
     def __init__(self, api_instance, all_servers=False):
         self.api_instance = api_instance
         self.domain_abs = DNSName(self.api_instance.env.domain).make_absolute()
-        self.servers_data = {}
+        self.servers_data = OrderedDict()
         self.__init_data(all_servers=all_servers)
 
     def reload_data(self):
@@ -90,7 +90,7 @@ class IPASystemRecords(object):
         return location + DNSName('_locations') + self.domain_abs
 
     def __init_data(self, all_servers=False):
-        self.servers_data = {}
+        self.servers_data.clear()
 
         kwargs = dict(no_members=False)
         if not all_servers:
@@ -325,7 +325,7 @@ class IPASystemRecords(object):
 
         zone_obj = zone.Zone(self.domain_abs, relativize=False)
         if servers is None:
-            servers = self.servers_data.keys()
+            servers = list(self.servers_data)
 
         for server in servers:
             self._add_base_dns_records_for_server(zone_obj, server,
@@ -348,11 +348,7 @@ class IPASystemRecords(object):
         """
         zone_obj = zone.Zone(self.domain_abs, relativize=False)
         if servers is None:
-            servers_result = self.api_instance.Command.server_find(
-                pkey_only=True,
-                servrole=u"IPA master",  # only fully installed masters
-            )['result']
-            servers = [s['cn'][0] for s in servers_result]
+            servers = list(self.servers_data)
 
         locations_result = self.api_instance.Command.location_find()['result']
         locations = [l['idnsname'][0] for l in locations_result]
-- 
2.17.1