From d09203de75444e54f90ea5bf9b83443e29726281 Mon Sep 17 00:00:00 2001 From: Hari Date: Sun, 29 May 2016 14:07:41 +0530 Subject: [PATCH] Fix accessing deleted vdns obect. When virtual-dns is deleted, all records configured under the vdns have their vdns pointer reset. In this case, it was not reset as the record wasnt added in the list under vdns, although the record was pointing to vdns. This resulted in accessing deleted vdns entry. In addition, make sure record is made valid when the appropriate vdns for it is updated. Change-Id: Id32f029014a46e8fe3efe35380940706708e7708 closes-bug: #1584174 --- src/dns/bind/named_config.h | 1 + src/dns/cmn/dns.sandesh | 2 ++ src/dns/mgr/dns_mgr.h | 1 + src/dns/mgr/dns_oper.cc | 50 +++++++++++++++++++------------- src/dns/mgr/dns_oper.h | 2 +- src/dns/test/SConscript | 6 ++-- src/dns/test/dns_mgr_test.cc | 55 ++++++++++++++++++++++++++++++------ 7 files changed, 86 insertions(+), 31 deletions(-) diff --git a/src/dns/bind/named_config.h b/src/dns/bind/named_config.h index 12ec3662271..071e3f1a5e5 100644 --- a/src/dns/bind/named_config.h +++ b/src/dns/bind/named_config.h @@ -28,6 +28,7 @@ class BindStatus { private: friend class DnsBindTest; + friend class DnsManagerTest; bool IsBindPid(uint32_t pid); bool CheckBindStatus(); diff --git a/src/dns/cmn/dns.sandesh b/src/dns/cmn/dns.sandesh index 267d041684e..80d30e873b3 100644 --- a/src/dns/cmn/dns.sandesh +++ b/src/dns/cmn/dns.sandesh @@ -13,6 +13,7 @@ struct VirtualDnsTraceData { 8: string floating_ip_record; 9: string external_visible; 10: string reverse_resolution; + 11: i32 flags; } struct VirtualDnsRecordTraceData { @@ -24,6 +25,7 @@ struct VirtualDnsRecordTraceData { 6: i32 rec_ttl; 7: string source; 8: string installed; + 9: i32 flags; } trace sandesh IpamTrace { diff --git a/src/dns/mgr/dns_mgr.h b/src/dns/mgr/dns_mgr.h index 65df95f2554..e96ea195bac 100644 --- a/src/dns/mgr/dns_mgr.h +++ b/src/dns/mgr/dns_mgr.h @@ -76,6 +76,7 @@ class DnsManager { private: friend class DnsBindTest; + friend class DnsManagerTest; bool SendRecordUpdate(BindUtil::Operation op, const VirtualDnsRecordConfig *config); diff --git a/src/dns/mgr/dns_oper.cc b/src/dns/mgr/dns_oper.cc index eb20b41a99a..bec9d94e9e0 100644 --- a/src/dns/mgr/dns_oper.cc +++ b/src/dns/mgr/dns_oper.cc @@ -440,6 +440,7 @@ void VirtualDnsConfig::VirtualDnsTrace(VirtualDnsTraceData &rec) { rec.floating_ip_record = rec_.floating_ip_record; rec.external_visible = (rec_.external_visible ? "yes" : "no"); rec.reverse_resolution = (rec_.reverse_resolution ? "yes" : "no"); + rec.flags = flags_; } void VirtualDnsConfig::Trace(const std::string &ev) { @@ -503,7 +504,9 @@ VirtualDnsRecordConfig::~VirtualDnsRecordConfig() { void VirtualDnsRecordConfig::OnAdd(IFMapNode *node) { MarkValid(); if (!virt_dns_) { - UpdateVdns(node); + if (!UpdateVdns(node)) { + ClearValid(); + } } else if (!virt_dns_->IsValid()) { virt_dns_->AddRecord(this); } else { @@ -528,10 +531,15 @@ void VirtualDnsRecordConfig::OnDelete() { void VirtualDnsRecordConfig::OnChange(IFMapNode *node) { DnsItem new_rec; - if (!GetObject(node, new_rec) || !HasChanged(new_rec)) + if (!GetObject(node, new_rec) || + (IsValid() && !HasChanged(new_rec))) { return; + } if (!virt_dns_) { - UpdateVdns(node); + MarkValid(); + if (!UpdateVdns(node)) { + ClearValid(); + } } // For records, notify a change with a delete of the old record // followed by addition of new record; If only TTL has changed, @@ -551,26 +559,29 @@ void VirtualDnsRecordConfig::OnChange(const DnsItem &new_rec) { Trace("Change"); } -void VirtualDnsRecordConfig::UpdateVdns(IFMapNode *node) { - if (node) { - IFMapNode *vdns_node = Dns::GetDnsConfigManager()->FindTarget(node, - "virtual-DNS-virtual-DNS-record"); - if (vdns_node == NULL) { - DNS_TRACE(DnsError, "Virtual DNS Record <" + GetName() + - "> does not have virtual DNS link"); - return; - } - virt_dns_ = VirtualDnsConfig::Find(vdns_node->name()); - if (!virt_dns_) { - virt_dns_ = new VirtualDnsConfig(vdns_node); - virt_dns_->AddRecord(this); - } - virtual_dns_name_ = virt_dns_->GetName(); +bool VirtualDnsRecordConfig::UpdateVdns(IFMapNode *node) { + if (!node) { + return false; + } + + IFMapNode *vdns_node = Dns::GetDnsConfigManager()->FindTarget(node, + "virtual-DNS-virtual-DNS-record"); + if (vdns_node == NULL) { + DNS_TRACE(DnsError, "Virtual DNS Record <" + GetName() + + "> does not have virtual DNS link"); + return false; } + virt_dns_ = VirtualDnsConfig::Find(vdns_node->name()); + if (!virt_dns_) { + virt_dns_ = new VirtualDnsConfig(vdns_node); + } + virt_dns_->AddRecord(this); + virtual_dns_name_ = virt_dns_->GetName(); + return true; } bool VirtualDnsRecordConfig::CanNotify() { - if (!virt_dns_ || !virt_dns_->IsValid()) + if (!IsValid() || !virt_dns_ || !virt_dns_->IsValid()) return false; if (rec_.eclass == DNS_CLASS_IN && rec_.type == DNS_PTR_RECORD) { @@ -663,6 +674,7 @@ void VirtualDnsRecordConfig::VirtualDnsRecordTrace(VirtualDnsRecordTraceData &re rec.installed = "true"; else rec.installed = "false"; + rec.flags = flags_; } void VirtualDnsRecordConfig::Trace(const std::string &ev) { diff --git a/src/dns/mgr/dns_oper.h b/src/dns/mgr/dns_oper.h index 759432dae25..c0b47a47b73 100644 --- a/src/dns/mgr/dns_oper.h +++ b/src/dns/mgr/dns_oper.h @@ -207,7 +207,7 @@ struct VirtualDnsRecordConfig : public DnsConfig { void OnDelete(); void OnChange(IFMapNode *node); void OnChange(const DnsItem &new_rec); - void UpdateVdns(IFMapNode *node); + bool UpdateVdns(IFMapNode *node); bool CanNotify(); bool HasChanged(DnsItem &rhs); diff --git a/src/dns/test/SConscript b/src/dns/test/SConscript index 89e722cbde0..4738e64c4a4 100644 --- a/src/dns/test/SConscript +++ b/src/dns/test/SConscript @@ -52,19 +52,19 @@ env.Alias('src/dns:dns_bind_test', dns_bind_test) dns_options_test = env.UnitTest('dns_options_test', ['dns_options_test.cc']) env.Alias('src/dns:dns_options_test', dns_options_test) -#dns_mgr_test = env.UnitTest('dns_mgr_test', ['dns_mgr_test.cc']) -#env.Alias('src/dns:dns_mgr_test', dns_mgr_test) +dns_mgr_test = env.UnitTest('dns_mgr_test', ['dns_mgr_test.cc']) +env.Alias('src/dns:dns_mgr_test', dns_mgr_test) test_suite = [ dns_bind_test, dns_options_test, -# dns_mgr_test, ] test = env.TestSuite('dns-test', test_suite) env.Alias('controller/src/dns:test', test) env.Alias('controller/src/dns:flaky-test', [ dns_config_test, + dns_mgr_test, ]) Return('test_suite') diff --git a/src/dns/test/dns_mgr_test.cc b/src/dns/test/dns_mgr_test.cc index f271ad6e7a5..f40759fb570 100644 --- a/src/dns/test/dns_mgr_test.cc +++ b/src/dns/test/dns_mgr_test.cc @@ -1,7 +1,6 @@ /* * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved. */ -#include "cfg/dns_config.h" #include #include @@ -21,14 +20,52 @@ #include "io/event_manager.h" #include "schema/vnc_cfg_types.h" #include "cmn/dns.h" +#include "bind/bind_util.h" +#include "cfg/dns_config.h" #include "cfg/dns_config.h" #include "cfg/dns_config_parser.h" -#include "bind/named_config.h" -#include "mgr/dns_mgr.h" #include "testing/gunit.h" +#include "mgr/dns_mgr.h" +#include "bind/named_config.h" using namespace std; +class NamedConfigTest : public NamedConfig { +public: + NamedConfigTest(const std::string &conf_dir, const std::string &conf_file) : + NamedConfig(conf_dir, conf_file, "/var/log/named/bind.log", + "rndc.conf", "xvysmOR8lnUQRBcunkC6vg==", "100M") {} + static void Init() { + assert(singleton_ == NULL); + singleton_ = new NamedConfigTest(".", "named.conf"); + singleton_->Reset(); + } + static void Shutdown() { + delete singleton_; + singleton_ = NULL; + remove("./named.conf"); + remove("./rndc.conf"); + } + virtual void UpdateNamedConf(const VirtualDnsConfig *updated_vdns) { + CreateNamedConf(updated_vdns); + } + std::string GetZoneFileName(const std::string &vdns, + const std::string &name) { + if (name.size() && name.at(name.size() - 1) == '.') + return (name + "zone"); + else + return (name + ".zone"); + } + std::string GetZoneFilePath(const std::string &vdns, + const string &name) { + return (named_config_dir_ + GetZoneFileName("", name)); + } + std::string GetZoneFilePath(const string &name) { + return GetZoneFilePath("", name); + } + std::string GetResolveFile() { return ""; } +}; + static string FileRead(const string &filename) { ifstream file(filename.c_str()); string content((istreambuf_iterator(file)), @@ -38,24 +75,26 @@ static string FileRead(const string &filename) { class DnsManagerTest : public ::testing::Test { protected: + DnsManagerTest() : parser_(&db_) { - NamedConfig::Init(); } virtual void SetUp() { IFMapLinkTable_Init(&db_, &db_graph_); vnc_cfg_Server_ModuleInit(&db_, &db_graph_); - + NamedConfigTest::Init(); Dns::SetDnsManager(&dns_manager_); - dns_manager_.GetConfigManager().Initialize(&db_, &db_graph_, - "local"); + dns_manager_.config_mgr_.Initialize(&db_, &db_graph_); + dns_manager_.bind_status_.named_pid_ = 0; } virtual void TearDown() { task_util::WaitForIdle(); + dns_manager_.bind_status_.named_pid_ = -1; dns_manager_.Shutdown(); + NamedConfigTest::Shutdown(); + // dns_manager_.GetConfigManager().Terminate(); task_util::WaitForIdle(); db_util::Clear(&db_); } - DB db_; DBGraph db_graph_; DnsManager dns_manager_;