From c1d88c5dcab5673bfb77d81b877d0ea59a47f9cb Mon Sep 17 00:00:00 2001 From: Nima Eskandary Date: Fri, 26 Apr 2019 09:47:29 -0400 Subject: [PATCH] max zone size (#584) * max zone size rule * refactor IOs --- .../zone/ZoneViewLoaderIntegrationSpec.scala | 8 +++++ modules/api/src/main/resources/reference.conf | 2 ++ .../scala/vinyldns/api/VinylDNSConfig.scala | 2 ++ .../api/domain/zone/ZoneProtocol.scala | 11 ++++++ .../api/domain/zone/ZoneViewLoader.scala | 36 ++++++++++--------- .../docs/src/main/tut/operator/config-api.md | 5 +++ 6 files changed, 47 insertions(+), 17 deletions(-) diff --git a/modules/api/src/it/scala/vinyldns/api/domain/zone/ZoneViewLoaderIntegrationSpec.scala b/modules/api/src/it/scala/vinyldns/api/domain/zone/ZoneViewLoaderIntegrationSpec.scala index 1e2bd7e5b..63ff8d1b1 100644 --- a/modules/api/src/it/scala/vinyldns/api/domain/zone/ZoneViewLoaderIntegrationSpec.scala +++ b/modules/api/src/it/scala/vinyldns/api/domain/zone/ZoneViewLoaderIntegrationSpec.scala @@ -44,5 +44,13 @@ class ZoneViewLoaderIntegrationSpec extends WordSpec with Matchers { .load() .unsafeRunSync()) } + + "return a failure if the zone is larger than the max zone size" in { + assertThrows[ZoneTooLargeError]( + DnsZoneViewLoader(Zone("vinyldns.", "test@test.com"), DnsZoneViewLoader.dnsZoneTransfer, 1) + .load() + .unsafeRunSync() + ) + } } } diff --git a/modules/api/src/main/resources/reference.conf b/modules/api/src/main/resources/reference.conf index 9c09af48b..36fede9c3 100644 --- a/modules/api/src/main/resources/reference.conf +++ b/modules/api/src/main/resources/reference.conf @@ -8,6 +8,8 @@ vinyldns { # if we should start up polling for change requests, set this to false for the inactive cluster processing-disabled = false + max-zone-size = 60000 # number of records that can be in a zone + queue { class-name = "vinyldns.sqs.queue.SqsMessageQueueProvider" diff --git a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala index 8d610537e..d0a3c4780 100644 --- a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala +++ b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala @@ -112,4 +112,6 @@ object VinylDNSConfig { if (vinyldnsConfig.hasPath(key)) { vinyldnsConfig.getStringList(key).asScala.toList } else List() + + lazy val maxZoneSize: Int = vinyldnsConfig.as[Option[Int]]("max-zone-size").getOrElse(60000) } diff --git a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneProtocol.scala b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneProtocol.scala index 7b0fb4864..e8f72cd27 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneProtocol.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneProtocol.scala @@ -240,3 +240,14 @@ case class ConnectionFailed(zone: Zone, message: String) extends Throwable(messa case class ZoneValidationFailed(zone: Zone, errors: List[String], message: String) extends Throwable(message) + +case class ZoneTooLargeError(msg: String) extends Throwable(msg) + +object ZoneTooLargeError { + def apply(zone: Zone, zoneSize: Int, maxZoneSize: Int): ZoneTooLargeError = new ZoneTooLargeError( + s""" + |ZoneTooLargeError: Zone '${zone.name}' (id: '${zone.id}') contains $zoneSize records + |which exceeds the max of $maxZoneSize + """.stripMargin.replace("\n", " ") + ) +} diff --git a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneViewLoader.scala b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneViewLoader.scala index db2b880fe..18cd4667d 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneViewLoader.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/zone/ZoneViewLoader.scala @@ -27,12 +27,14 @@ import vinyldns.core.domain.zone.Zone import vinyldns.core.route.Monitored import scala.collection.JavaConverters._ +import vinyldns.core.domain.record.{RecordSetRepository, RecordType} trait ZoneViewLoader { def load: () => IO[ZoneView] } object DnsZoneViewLoader extends DnsConversions { + def dnsZoneTransfer(zone: Zone): ZoneTransferIn = { val conn = ZoneConnectionValidator @@ -54,7 +56,10 @@ object DnsZoneViewLoader extends DnsConversions { DnsZoneViewLoader(zone, dnsZoneTransfer) } -case class DnsZoneViewLoader(zone: Zone, zoneTransfer: Zone => ZoneTransferIn) +case class DnsZoneViewLoader( + zone: Zone, + zoneTransfer: Zone => ZoneTransferIn, + maxZoneSize: Int = VinylDNSConfig.maxZoneSize) extends ZoneViewLoader with DnsConversions with Monitored { @@ -62,23 +67,20 @@ case class DnsZoneViewLoader(zone: Zone, zoneTransfer: Zone => ZoneTransferIn) def load: () => IO[ZoneView] = () => monitor("dns.loadZoneView") { - IO { - val xfr = zoneTransfer(zone) - xfr.run() - - val rawDnsRecords: List[DNS.Record] = + for { + zoneXfr <- IO { + val xfr = zoneTransfer(zone) + xfr.run() xfr.getAXFR.asScala.map(_.asInstanceOf[DNS.Record]).toList.distinct - - // not accepting unknown record types - val supportedRecords = - rawDnsRecords.filter(record => fromDnsRecordType(record.getType) != RecordType.UNKNOWN) - - val dnsZoneName = zoneDnsName(zone.name) - - val recordSets = supportedRecords.map(toRecordSet(_, dnsZoneName, zone.id)) - - ZoneView(zone, recordSets) - } + } + rawDnsRecords = zoneXfr.filter(record => + fromDnsRecordType(record.getType) != RecordType.UNKNOWN) + _ <- if (rawDnsRecords.length > maxZoneSize) + IO.raiseError(ZoneTooLargeError(zone, rawDnsRecords.length, maxZoneSize)) + else IO.pure(Unit) + dnsZoneName <- IO(zoneDnsName(zone.name)) + recordSets <- IO(rawDnsRecords.map(toRecordSet(_, dnsZoneName, zone.id))) + } yield ZoneView(zone, recordSets) } } diff --git a/modules/docs/src/main/tut/operator/config-api.md b/modules/docs/src/main/tut/operator/config-api.md index ed4550259..4d7ebb7ee 100644 --- a/modules/docs/src/main/tut/operator/config-api.md +++ b/modules/docs/src/main/tut/operator/config-api.md @@ -463,6 +463,11 @@ vinyldns { host = "0.0.0.0" port = 9000 } + + # The maximum number of records VinylDNS will load when syncing a DNS Zone + # this is to prevent possible out of memory errors when loading a Zone + # this does not stop the zone from existing in DNS, but you will not be able to manage it in VinylDNS if the number of records exceeds the max + max-zone-size = 60000 # the delay between zone syncs so we are not syncing too often sync-delay = 10000