From 72517c7b316f7cba93a3b2cdc6e2c69dad63a8b1 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 673397ef2b2252f431eec1f3e1f71dc45ff87511..90c4d8536478599e5a35462c7f00ea4c958ff63a 100644
--- a/ipaserver/dns_data_management.py
+++ b/ipaserver/dns_data_management.py
@@ -8,7 +8,7 @@ import logging
 
 import six
 
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 from dns import (
     rdata,
     rdataclass,
@@ -71,7 +71,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):
@@ -93,7 +93,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:
@@ -328,7 +328,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,
@@ -351,11 +351,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