mir/vinyldns
2
0
Fork 0
mirror of https://github.com/VinylDNS/vinyldns synced 2025-09-03 15:55:42 +00:00
Code Issues Releases Wiki Activity

Remove multi-record feature flag from API and portal (#880)

Browse Source 
* Remove multi-record configuration from API.
* Remove multi-record config from portal.
* Remove deprecated tests and update functional test script.
This commit is contained in:
Michael Ly
2019-10-28 16:36:50 -04:00
committed by GitHub
parent 279384c37b
commit e81dba4525
10 changed files with 57 additions and 436 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
docker/functest/run.sh
View File

@@ -54,28 +54,23 @@ find . -name "__pycache__" -delete
result=0 result=0
# If PROD_ENV is not true, we are in a local docker environment so do not skip anything # If PROD_ENV is not true, we are in a local docker environment so do not skip anything
if [ "${PROD_ENV}" = "true" ]; then if [ "${PROD_ENV}" = "true" ]; then
# -m plays havoc with -k, using variables is a headache, so doing this by hand PARALLEL_TEST_MARKER="not skip_production and not serial"
# run parallel tests first (not serial) SERIAL_TEST_MARKER="not skip_production and serial"
echo "./run-tests.py live_tests -n${PAR_CPU} -v -m \"not skip_production and not serial and not multi_record_enabled\" -v --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False"
./run-tests.py live_tests -n${PAR_CPU} -v -m "not skip_production and not serial and not multi_record_enabled" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False
result=$?
if [ $result -eq 0 ]; then
# run serial tests second (serial marker)
echo "./run-tests.py live_tests -n0 -v -m \"not skip_production and serial and not multi_record_enabled\" -v --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True"
./run-tests.py live_tests -n0 -v -m "not skip_production and serial and not multi_record_enabled" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True
result=$?
fi
else else
# run parallel tests first (not serial) PARALLEL_TEST_MARKER="not serial"
echo "./run-tests.py live_tests -n${PAR_CPU} -v -m \"not serial and not multi_record_disabled\" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False" SERIAL_TEST_MARKER="serial"
./run-tests.py live_tests -n${PAR_CPU} -v -m "not serial and not multi_record_disabled" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False fi
# -m plays havoc with -k, using variables is a headache, so doing this by hand
# run parallel tests first (not serial)
echo "./run-tests.py live_tests -n${PAR_CPU} -v -m \"${PARALLEL_TEST_MARKER}\" -v --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False"
./run-tests.py live_tests -n${PAR_CPU} -v -m ${PARALLEL_TEST_MARKER} --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=False
result=$?
if [ $? -eq 0 ]; then
# run serial tests second (serial marker)
echo "./run-tests.py live_tests -n0 -v -m \"${SERIAL_TEST_MARKER}\" -v --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True"
./run-tests.py live_tests -n0 -v -m ${SERIAL_TEST_MARKER} --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True
result=$? result=$?
if [ $result -eq 0 ]; then
# run serial tests second (serial marker)
echo "./run-tests.py live_tests -n0 -v -m \"serial and not multi_record_disabled\" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True"
./run-tests.py live_tests -n0 -v -m "serial and not multi_record_disabled" --url=${VINYLDNS_URL} --dns-ip=${DNS_IP} ${TEST_PATTERN} --teardown=True
result=$?
fi
fi fi
exit $result exit $result

237
modules/api/functional_test/live_tests/batch/create_batch_change_test.py
View File

@@ -3806,48 +3806,6 @@ def test_create_batch_with_irrelevant_global_acl_rule_applied_fails(shared_zone_
shared_client.wait_until_recordset_change_status(delete_a_rs, 'Complete') shared_client.wait_until_recordset_change_status(delete_a_rs, 'Complete')
@pytest.mark.multi_record_disabled
@pytest.mark.serial
@pytest.mark.skip_production
def test_create_batch_duplicates_add_check(shared_zone_test_context):
"""
Test recordsets with multiple records cannot be added in batch (relies on config, skip-prod)
"""
client = shared_zone_test_context.ok_vinyldns_client
batch_change_input = {
"comments": "this is optional",
"changes": [
get_change_A_AAAA_json("multi.ok.", address="1.2.3.4"),
get_change_A_AAAA_json("multi.ok.", address="4.5.6.7"),
get_change_PTR_json("192.0.2.44", ptrdname="multi.test"),
get_change_PTR_json("192.0.2.44", ptrdname="multi2.test"),
get_change_TXT_json("multi-txt.ok.", text="some-multi-text"),
get_change_TXT_json("multi-txt.ok.", text="more-multi-text"),
get_change_MX_json("multi-mx.ok.", preference=0),
get_change_MX_json("multi-mx.ok.", preference=1000, exchange="bar.foo."),
# adding an HVD so this will fail if accidentally run against wrong config
get_change_A_AAAA_json("high-value-domain")
]
}
response = client.create_batch_change(batch_change_input, status=400)
def err(name, type):
return 'Multi-record recordsets are not enabled for this instance of VinylDNS. ' \
'Cannot create a new record set with multiple records for inputName {} and type {}.'.format(name, type)
assert_error(response[0], error_messages=[err("multi.ok.", "A")])
assert_error(response[1], error_messages=[err("multi.ok.", "A")])
assert_error(response[2], error_messages=[err("192.0.2.44", "PTR")])
assert_error(response[3], error_messages=[err("192.0.2.44", "PTR")])
assert_error(response[4], error_messages=[err("multi-txt.ok.", "TXT")])
assert_error(response[5], error_messages=[err("multi-txt.ok.", "TXT")])
assert_error(response[6], error_messages=[err("multi-mx.ok.", "MX")])
assert_error(response[7], error_messages=[err("multi-mx.ok.", "MX")])
@pytest.mark.manual_batch_review @pytest.mark.manual_batch_review
def test_create_batch_with_zone_name_requiring_manual_review(shared_zone_test_context): def test_create_batch_with_zone_name_requiring_manual_review(shared_zone_test_context):
""" """
@@ -3915,197 +3873,7 @@ def test_create_batch_delete_record_for_invalid_record_data_fails(shared_zone_te
finally: finally:
clear_recordset_list(to_delete, client) clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_disabled
@pytest.mark.skip_production
def test_create_batch_multi_record_update_fails(shared_zone_test_context):
"""
Test recordsets with multiple records cannot be edited in batch (relies on config, skip-prod)
"""
client = shared_zone_test_context.ok_vinyldns_client
ok_zone = shared_zone_test_context.ok_zone
# record sets to setup
a_update_name = generate_record_name()
a_update_fqdn = a_update_name + ".ok."
a_update = get_recordset_json(ok_zone, a_update_name, "A", [{"address": "1.1.1.1"}, {"address": "1.1.1.2"}], 200)
txt_update_name = generate_record_name()
txt_update_fqdn = txt_update_name + ".ok."
txt_update = get_recordset_json(ok_zone, txt_update_name, "TXT", [{"text": "hello"}, {"text": "again"}], 200)
a_delete_name = generate_record_name()
a_delete_fqdn = a_delete_name + ".ok."
a_delete = get_recordset_json(ok_zone, a_delete_name, "A", [{"address": "1.1.1.1"}, {"address": "1.1.1.2"}], 200)
txt_delete_name = generate_record_name()
txt_delete_fqdn = txt_delete_name + ".ok."
txt_delete = get_recordset_json(ok_zone, txt_delete_name, "TXT", [{"text": "hello"}, {"text": "again"}], 200)
batch_change_input = {
"comments": "this is optional",
"changes": [
get_change_A_AAAA_json(a_update_fqdn, change_type="DeleteRecordSet"),
get_change_A_AAAA_json(a_update_fqdn, address="1.2.3.4"),
get_change_A_AAAA_json(a_update_fqdn, address="4.5.6.7"),
get_change_TXT_json(txt_update_fqdn, change_type="DeleteRecordSet"),
get_change_TXT_json(txt_update_fqdn, text="some-multi-text"),
get_change_TXT_json(txt_update_fqdn, text="more-multi-text"),
get_change_A_AAAA_json(a_delete_fqdn, change_type="DeleteRecordSet"),
get_change_TXT_json(txt_delete_fqdn, change_type="DeleteRecordSet"),
# adding an HVD so this will fail if accidentally run against wrong config
get_change_A_AAAA_json("high-value-domain")
]
}
to_delete = []
try:
for rs in [a_update, txt_update, a_delete, txt_delete]:
create_rs = client.create_recordset(rs, status=202)
to_delete.append(client.wait_until_recordset_change_status(create_rs, 'Complete'))
response = client.create_batch_change(batch_change_input, status=400)
def existing_err(name, type):
return 'RecordSet with name {} and type {} cannot be updated in a single '.format(name, type) + \
'Batch Change because it contains multiple DNS records (2).'
def new_err(name, type):
return 'Multi-record recordsets are not enabled for this instance of VinylDNS. ' \
'Cannot create a new record set with multiple records for inputName {} and type {}.'.format(name,
type)
assert_error(response[0], error_messages=[existing_err(a_update_fqdn, "A")])
assert_error(response[1], error_messages=[existing_err(a_update_fqdn, "A"), new_err(a_update_fqdn, "A")])
assert_error(response[2], error_messages=[existing_err(a_update_fqdn, "A"), new_err(a_update_fqdn, "A")])
assert_error(response[3], error_messages=[existing_err(txt_update_fqdn, "TXT")])
assert_error(response[4],
error_messages=[existing_err(txt_update_fqdn, "TXT"), new_err(txt_update_fqdn, "TXT")])
assert_error(response[5],
error_messages=[existing_err(txt_update_fqdn, "TXT"), new_err(txt_update_fqdn, "TXT")])
assert_error(response[6], error_messages=[existing_err(a_delete_fqdn, "A")])
assert_error(response[7], error_messages=[existing_err(txt_delete_fqdn, "TXT")])
finally:
clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_disabled
def test_create_batch_deletes_fails(shared_zone_test_context):
"""
Test creating batch change with DeleteRecordSets when multi-record support is disabled
"""
client = shared_zone_test_context.ok_vinyldns_client
ok_zone = shared_zone_test_context.ok_zone
ok_group = shared_zone_test_context.ok_group
rs_name = generate_record_name()
rs_name_2 = generate_record_name()
multi_rs_name = generate_record_name()
multi_rs_name_2 = generate_record_name()
rs_fqdn = rs_name + ".ok."
rs_fqdn_2 = rs_name + ".ok."
multi_rs_fqdn = multi_rs_name + ".ok."
multi_rs_fqdn_2 = multi_rs_name_2 + ".ok."
rs_to_create = get_recordset_json(ok_zone, rs_name, "A", [{"address": "1.2.3.4"}], 200, ok_group['id'])
rs_to_create_2 = get_recordset_json(ok_zone, rs_name_2, "A", [{"address": "1.2.3.4"}], 200, ok_group['id'])
multi_record_rs_to_create = get_recordset_json(ok_zone, multi_rs_name, "A", [{"address": "1.2.3.4"}, {"address": "1.1.1.1"}], 200, ok_group['id'])
multi_record_rs_to_create_2 = get_recordset_json(ok_zone, multi_rs_name_2, "A", [{"address": "1.2.3.4"}, {"address": "1.1.1.1"}], 200, ok_group['id'])
batch_change_input = {
"comments": "this is optional",
"changes": [
get_change_A_AAAA_json(rs_fqdn, address="1.2.3.4", change_type="DeleteRecordSet"),
get_change_A_AAAA_json(rs_fqdn_2, change_type="DeleteRecordSet"),
get_change_A_AAAA_json(multi_rs_fqdn, address="1.2.3.4", change_type="DeleteRecordSet"),
get_change_A_AAAA_json(multi_rs_fqdn_2, change_type="DeleteRecordSet")
]
}
to_delete = []
try:
create_rs = client.create_recordset(rs_to_create, status=202)
create_rs_2 = client.create_recordset(rs_to_create_2, status=202)
create_multi_rs = client.create_recordset(multi_record_rs_to_create, status=202)
create_multi_rs_2 = client.create_recordset(multi_record_rs_to_create_2, status=202)
to_delete.append(client.wait_until_recordset_change_status(create_rs, 'Complete'))
to_delete.append(client.wait_until_recordset_change_status(create_rs_2, 'Complete'))
to_delete.append(client.wait_until_recordset_change_status(create_multi_rs, 'Complete'))
to_delete.append(client.wait_until_recordset_change_status(create_multi_rs_2, 'Complete'))
response = client.create_batch_change(batch_change_input, status=400)
assert_successful_change_in_error_response(response[0], input_name=rs_fqdn, record_type="A", record_data="1.2.3.4", change_type="DeleteRecordSet")
assert_successful_change_in_error_response(response[1], input_name=rs_fqdn_2, record_type="A", change_type="DeleteRecordSet")
assert_failed_change_in_error_response(response[2], input_name=multi_rs_fqdn, record_type="A", record_data="1.2.3.4", change_type="DeleteRecordSet",
error_messages=['RecordSet with name ' + multi_rs_fqdn + ' and type A cannot be updated in a single Batch Change because it contains multiple DNS records (2).'])
assert_failed_change_in_error_response(response[3], input_name=multi_rs_fqdn_2, record_type="A", change_type="DeleteRecordSet",
error_messages=['RecordSet with name ' + multi_rs_fqdn_2 + ' and type A cannot be updated in a single Batch Change because it contains multiple DNS records (2).'])
finally:
clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_disabled
@pytest.mark.serial
@pytest.mark.skip_production
def test_create_batch_change_with_multi_record_adds_without_multi_record_support(shared_zone_test_context):
"""
Test new recordsets with multiple records cannot be added in batch, and existing recordsets cannot be added to
"""
client = shared_zone_test_context.ok_vinyldns_client
ok_zone = shared_zone_test_context.ok_zone
ok_group = shared_zone_test_context.ok_group
to_delete = []
rs_name = generate_record_name()
rs_fqdn = rs_name + ".ok."
rs_to_create = get_recordset_json(ok_zone, rs_name, "A", [{"address": "1.2.3.4"}], 200, ok_group['id'])
batch_change_input = {
"comments": "this is optional",
"changes": [
get_change_A_AAAA_json("multi.ok.", address="1.2.3.4"),
get_change_A_AAAA_json("multi.ok.", address="4.5.6.7"),
get_change_PTR_json("192.0.2.44", ptrdname="multi.test"),
get_change_PTR_json("192.0.2.44", ptrdname="multi2.test"),
get_change_TXT_json("multi-txt.ok.", text="some-multi-text"),
get_change_TXT_json("multi-txt.ok.", text="more-multi-text"),
get_change_MX_json("multi-mx.ok.", preference=0),
get_change_MX_json("multi-mx.ok.", preference=1000, exchange="bar.foo."),
get_change_A_AAAA_json(rs_fqdn, address="1.1.1.1")
]
}
try:
create_rs = client.create_recordset(rs_to_create, status=202)
to_delete.append(client.wait_until_recordset_change_status(create_rs, 'Complete'))
response = client.create_batch_change(batch_change_input, status=400)
def err(name, type):
return 'Multi-record recordsets are not enabled for this instance of VinylDNS. ' \
'Cannot create a new record set with multiple records for inputName {} and type {}.'.format(name,
type)
assert_error(response[0], error_messages=[err("multi.ok.", "A")])
assert_error(response[1], error_messages=[err("multi.ok.", "A")])
assert_error(response[2], error_messages=[err("192.0.2.44", "PTR")])
assert_error(response[3], error_messages=[err("192.0.2.44", "PTR")])
assert_error(response[4], error_messages=[err("multi-txt.ok.", "TXT")])
assert_error(response[5], error_messages=[err("multi-txt.ok.", "TXT")])
assert_error(response[6], error_messages=[err("multi-mx.ok.", "MX")])
assert_error(response[7], error_messages=[err("multi-mx.ok.", "MX")])
assert_failed_change_in_error_response(response[8], input_name=rs_fqdn, record_data="1.1.1.1",
error_messages=['Record "' + rs_fqdn + '" Already Exists: cannot add an existing record; to update it, issue a DeleteRecordSet then an Add.'])
finally:
clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_enabled
@pytest.mark.serial @pytest.mark.serial
def test_create_batch_delete_record_access_checks(shared_zone_test_context): def test_create_batch_delete_record_access_checks(shared_zone_test_context):
""" """
@@ -4169,8 +3937,6 @@ def test_create_batch_delete_record_access_checks(shared_zone_test_context):
clear_ok_acl_rules(shared_zone_test_context) clear_ok_acl_rules(shared_zone_test_context)
clear_recordset_list(to_delete, ok_client) clear_recordset_list(to_delete, ok_client)
@pytest.mark.multi_record_enabled
@pytest.mark.skip_production @pytest.mark.skip_production
def test_create_batch_multi_record_update_succeeds(shared_zone_test_context): def test_create_batch_multi_record_update_succeeds(shared_zone_test_context):
""" """
@@ -4379,7 +4145,6 @@ def test_create_batch_multi_record_update_succeeds(shared_zone_test_context):
finally: finally:
clear_recordset_list(to_delete, client) clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_enabled
def test_create_batch_deletes_succeeds(shared_zone_test_context): def test_create_batch_deletes_succeeds(shared_zone_test_context):
""" """
Test creating batch change with DeleteRecordSet with valid record data succeeds Test creating batch change with DeleteRecordSet with valid record data succeeds
@@ -4435,8 +4200,6 @@ def test_create_batch_deletes_succeeds(shared_zone_test_context):
finally: finally:
clear_recordset_list(to_delete, client) clear_recordset_list(to_delete, client)
@pytest.mark.multi_record_enabled
@pytest.mark.serial @pytest.mark.serial
@pytest.mark.skip_production @pytest.mark.skip_production
def test_create_batch_change_with_multi_record_adds_with_multi_record_support(shared_zone_test_context): def test_create_batch_change_with_multi_record_adds_with_multi_record_support(shared_zone_test_context):

1
modules/api/src/main/scala/vinyldns/api/Boot.scala
View File

@@ -117,7 +117,6 @@ object Boot extends App {
val batchChangeValidations = new BatchChangeValidations( val batchChangeValidations = new BatchChangeValidations(
batchChangeLimit, batchChangeLimit,
batchAccessValidations, batchAccessValidations,
VinylDNSConfig.multiRecordBatchUpdateEnabled,
VinylDNSConfig.scheduledChangesEnabled VinylDNSConfig.scheduledChangesEnabled
) )
val membershipService = MembershipService(repositories) val membershipService = MembershipService(repositories)

4
modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala
View File

@@ -129,9 +129,7 @@ object VinylDNSConfig {
} else List() } else List()
lazy val maxZoneSize: Int = vinyldnsConfig.as[Option[Int]]("max-zone-size").getOrElse(60000) lazy val maxZoneSize: Int = vinyldnsConfig.as[Option[Int]]("max-zone-size").getOrElse(60000)
lazy val defaultTtl: Long = vinyldnsConfig.as[Option[Long]](s"default-ttl").getOrElse(7200L) lazy val defaultTtl: Long = vinyldnsConfig.as[Option[Long]]("default-ttl").getOrElse(7200L)
lazy val multiRecordBatchUpdateEnabled: Boolean =
vinyldnsConfig.as[Option[Boolean]]("multi-record-batch-change-enabled").getOrElse(false)
lazy val manualBatchReviewEnabled: Boolean = vinyldnsConfig lazy val manualBatchReviewEnabled: Boolean = vinyldnsConfig
.as[Option[Boolean]]("manual-batch-review-enabled") .as[Option[Boolean]]("manual-batch-review-enabled")
.getOrElse(false) .getOrElse(false)

28
modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala
View File

@@ -69,7 +69,6 @@ trait BatchChangeValidationsAlgebra {
class BatchChangeValidations( class BatchChangeValidations(
changeLimit: Int, changeLimit: Int,
accessValidation: AccessValidationsAlgebra, accessValidation: AccessValidationsAlgebra,
multiRecordEnabled: Boolean = false,
scheduledChangesEnabled: Boolean = false) scheduledChangesEnabled: Boolean = false)
extends BatchChangeValidationsAlgebra { extends BatchChangeValidationsAlgebra {
@@ -267,23 +266,6 @@ class BatchChangeValidations(
validateDeleteUpdateWithContext(deleteUpdate, groupedChanges, auth, isApproved) validateDeleteUpdateWithContext(deleteUpdate, groupedChanges, auth, isApproved)
} }
def existingRecordSetIsNotMulti(
change: ChangeForValidation,
recordSet: RecordSet): SingleValidation[Unit] =
if (!multiRecordEnabled & recordSet.records.length > 1) {
ExistingMultiRecordError(change.inputChange.inputName, recordSet).invalidNel
} else ().validNel
def newRecordSetIsNotMulti(
change: AddChangeForValidation,
groupedChanges: ChangeForValidationMap): SingleValidation[Unit] = {
lazy val matchingAddRecords = groupedChanges
.getProposedAdds(change.recordKey)
if (!multiRecordEnabled && matchingAddRecords.size > 1)
NewMultiRecordError(change.inputChange.inputName, change.inputChange.typ).invalidNel
else ().validNel
}
def newRecordSetIsNotDotted(change: AddChangeForValidation): SingleValidation[Unit] = def newRecordSetIsNotDotted(change: AddChangeForValidation): SingleValidation[Unit] =
if (change.recordName != change.zone.name && change.recordName.contains(".")) if (change.recordName != change.zone.name && change.recordName.contains("."))
ZoneDiscoveryError(change.inputChange.inputName).invalidNel ZoneDiscoveryError(change.inputChange.inputName).invalidNel
@@ -317,7 +299,6 @@ class BatchChangeValidations(
groupedChanges.getExistingRecordSet(change.recordKey) match { groupedChanges.getExistingRecordSet(change.recordKey) match {
case Some(rs) => case Some(rs) =>
userCanDeleteRecordSet(change, auth, rs.ownerGroupId, rs.records) |+| userCanDeleteRecordSet(change, auth, rs.ownerGroupId, rs.records) |+|
existingRecordSetIsNotMulti(change, rs) |+|
zoneDoesNotRequireManualReview(change, isApproved) |+| zoneDoesNotRequireManualReview(change, isApproved) |+|
ensureRecordExists(change, groupedChanges) ensureRecordExists(change, groupedChanges)
case None => RecordDoesNotExist(change.inputChange.inputName).invalidNel case None => RecordDoesNotExist(change.inputChange.inputName).invalidNel
@@ -335,7 +316,7 @@ class BatchChangeValidations(
// could potentially be grouped with a single delete // could potentially be grouped with a single delete
val typedValidations = change.inputChange.typ match { val typedValidations = change.inputChange.typ match {
case CNAME => recordIsUniqueInBatch(change, groupedChanges) case CNAME => recordIsUniqueInBatch(change, groupedChanges)
case _ => newRecordSetIsNotMulti(change, groupedChanges) case _ => ().validNel
} }
val commonValidations: SingleValidation[Unit] = { val commonValidations: SingleValidation[Unit] = {
@@ -347,7 +328,6 @@ class BatchChangeValidations(
groupedChanges.existingRecordSets groupedChanges.existingRecordSets
.get(change.zone.id, change.recordName, change.inputChange.typ), .get(change.zone.id, change.recordName, change.inputChange.typ),
batchOwnerGroupId) |+| batchOwnerGroupId) |+|
existingRecordSetIsNotMulti(change, rs) |+|
zoneDoesNotRequireManualReview(change, isApproved) zoneDoesNotRequireManualReview(change, isApproved)
case None => case None =>
RecordDoesNotExist(change.inputChange.inputName).invalidNel RecordDoesNotExist(change.inputChange.inputName).invalidNel
@@ -369,7 +349,6 @@ class BatchChangeValidations(
case Some(rs) => case Some(rs) =>
val adds = groupedChanges.getProposedAdds(change.recordKey).toList val adds = groupedChanges.getProposedAdds(change.recordKey).toList
userCanUpdateRecordSet(change, auth, rs.ownerGroupId, adds) |+| userCanUpdateRecordSet(change, auth, rs.ownerGroupId, adds) |+|
existingRecordSetIsNotMulti(change, rs) |+|
zoneDoesNotRequireManualReview(change, isApproved) |+| zoneDoesNotRequireManualReview(change, isApproved) |+|
ensureRecordExists(change, groupedChanges) ensureRecordExists(change, groupedChanges)
case None => case None =>
@@ -387,13 +366,12 @@ class BatchChangeValidations(
ownerGroupId: Option[String]): SingleValidation[ChangeForValidation] = { ownerGroupId: Option[String]): SingleValidation[ChangeForValidation] = {
val typedValidations = change.inputChange.typ match { val typedValidations = change.inputChange.typ match {
case A | AAAA | MX => case A | AAAA | MX =>
newRecordSetIsNotMulti(change, groupedChanges) |+| newRecordSetIsNotDotted(change)
newRecordSetIsNotDotted(change)
case CNAME => case CNAME =>
cnameHasUniqueNameInBatch(change, groupedChanges) |+| cnameHasUniqueNameInBatch(change, groupedChanges) |+|
newRecordSetIsNotDotted(change) newRecordSetIsNotDotted(change)
case TXT | PTR => case TXT | PTR =>
newRecordSetIsNotMulti(change, groupedChanges) ().validNel
case other => case other =>
InvalidBatchRecordType(other.toString, SupportedBatchChangeRecordTypes.get).invalidNel InvalidBatchRecordType(other.toString, SupportedBatchChangeRecordTypes.get).invalidNel
} }

117
modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala
View File

@@ -51,9 +51,7 @@ class BatchChangeValidationsSpec
private val maxChanges = 10 private val maxChanges = 10
private val accessValidations = new AccessValidations() private val accessValidations = new AccessValidations()
private val underTest = private val underTest =
new BatchChangeValidations(maxChanges, accessValidations, multiRecordEnabled = true) new BatchChangeValidations(maxChanges, accessValidations)
private val underTestMultiDisabled =
new BatchChangeValidations(maxChanges, accessValidations, multiRecordEnabled = false)
import underTest._ import underTest._
@@ -293,11 +291,8 @@ class BatchChangeValidationsSpec
None, None,
List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))), List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))),
scheduledTime = Some(DateTime.now)) scheduledTime = Some(DateTime.now))
val bcv = new BatchChangeValidations( val bcv =
maxChanges, new BatchChangeValidations(maxChanges, accessValidations, scheduledChangesEnabled = false)
accessValidations,
multiRecordEnabled = true,
scheduledChangesEnabled = false)
bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left( bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left(
ScheduledChangesDisabled) ScheduledChangesDisabled)
} }
@@ -308,11 +303,8 @@ class BatchChangeValidationsSpec
None, None,
List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))), List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))),
scheduledTime = Some(DateTime.now.minusHours(1))) scheduledTime = Some(DateTime.now.minusHours(1)))
val bcv = new BatchChangeValidations( val bcv =
maxChanges, new BatchChangeValidations(maxChanges, accessValidations, scheduledChangesEnabled = true)
accessValidations,
multiRecordEnabled = true,
scheduledChangesEnabled = true)
bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left( bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left(
ScheduledTimeMustBeInFuture) ScheduledTimeMustBeInFuture)
} }
@@ -1953,8 +1945,7 @@ class BatchChangeValidationsSpec
result(0) shouldBe valid result(0) shouldBe valid
} }
property( property("validateChangesWithContext: succeed update/delete to a multi record existing RecordSet") {
"validateChangesWithContext: succeed update/delete to a multi record existing RecordSet if multi enabled") {
val existing = List( val existing = List(
sharedZoneRecord.copy( sharedZoneRecord.copy(
name = updateSharedAddChange.recordName, name = updateSharedAddChange.recordName,
@@ -1992,49 +1983,7 @@ class BatchChangeValidationsSpec
result(5) shouldBe valid result(5) shouldBe valid
} }
property( property("validateChangesWithContext: succeed on add/update to a multi record") {
"validateChangesWithContext: fail on update/delete to a multi record existing RecordSet if multi disabled") {
val existing = List(
sharedZoneRecord.copy(
name = updateSharedAddChange.recordName,
records = List(AAAAData("1::1"), AAAAData("2::2"))),
sharedZoneRecord.copy(
name = deleteSharedChange.recordName,
records = List(AAAAData("1::1"), AAAAData("2::2"))),
rsOk.copy(name = updatePrivateAddChange.recordName),
rsOk.copy(name = deletePrivateChange.recordName)
)
val result = underTestMultiDisabled.validateChangesWithContext(
ChangeForValidationMap(
List(
updateSharedAddChange.validNel,
updateSharedDeleteChange.validNel,
deleteSharedChange.validNel,
updatePrivateAddChange.validNel,
updatePrivateDeleteChange.validNel,
deletePrivateChange.validNel
),
ExistingRecordSets(existing)
),
okAuth,
false,
Some(okGroup.id)
)
result(0) should haveInvalid[DomainValidationError](
ExistingMultiRecordError(updateSharedAddChange.inputChange.inputName, existing(0)))
result(1) should haveInvalid[DomainValidationError](
ExistingMultiRecordError(updateSharedDeleteChange.inputChange.inputName, existing(0)))
result(2) should haveInvalid[DomainValidationError](
ExistingMultiRecordError(deleteSharedChange.inputChange.inputName, existing(1)))
// non duplicate
result(3) shouldBe valid
result(4) shouldBe valid
result(5) shouldBe valid
}
property("validateChangesWithContext: succeed on add/update to a multi record if multi enabled") {
val existing = List( val existing = List(
sharedZoneRecord.copy(name = updateSharedAddChange.recordName) sharedZoneRecord.copy(name = updateSharedAddChange.recordName)
) )
@@ -2079,58 +2028,6 @@ class BatchChangeValidationsSpec
result(5) shouldBe valid result(5) shouldBe valid
} }
property("validateChangesWithContext: fail on add/update to a multi record if multi disabled") {
val existing = List(
sharedZoneRecord.copy(
name = updateSharedAddChange.recordName,
records = List(AAAAData("1::1"))
)
)
val update1 = updateSharedAddChange.copy(
inputChange =
AddChangeInput("shared-update.shared", RecordType.AAAA, ttl, AAAAData("1:2:3:4:5:6:7:8"))
)
val update2 = updateSharedAddChange.copy(
inputChange = AddChangeInput("shared-update.shared", RecordType.AAAA, ttl, AAAAData("1::1"))
)
val add1 = createSharedAddChange.copy(
inputChange = AddChangeInput("shared-add.shared", RecordType.A, ttl, AData("1.2.3.4"))
)
val add2 = createSharedAddChange.copy(
inputChange = AddChangeInput("shared-add.shared", RecordType.A, ttl, AData("5.6.7.8"))
)
val result = underTestMultiDisabled.validateChangesWithContext(
ChangeForValidationMap(
List(
updateSharedDeleteChange.validNel,
update1.validNel,
update2.validNel,
add1.validNel,
add2.validNel,
updatePrivateAddChange.validNel
),
ExistingRecordSets(existing)
),
okAuth,
false,
Some(okGroup.id)
)
result(0) shouldBe valid
result(1) should haveInvalid[DomainValidationError](
NewMultiRecordError(update1.inputChange.inputName, update1.inputChange.typ))
result(2) should haveInvalid[DomainValidationError](
NewMultiRecordError(update2.inputChange.inputName, update2.inputChange.typ))
result(3) should haveInvalid[DomainValidationError](
NewMultiRecordError(add1.inputChange.inputName, add1.inputChange.typ))
result(4) should haveInvalid[DomainValidationError](
NewMultiRecordError(add2.inputChange.inputName, add2.inputChange.typ))
// non duplicate
result(5) shouldBe valid
}
property("""validateChangesWithContext: should fail validateAddWithContext with property("""validateChangesWithContext: should fail validateAddWithContext with
|ZoneDiscoveryError if new record is dotted host but not a TXT record type""".stripMargin) { |ZoneDiscoveryError if new record is dotted host but not a TXT record type""".stripMargin) {
val addA = AddChangeForValidation( val addA = AddChangeForValidation(

35
modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala
View File

@@ -152,23 +152,6 @@ final case class MissingOwnerGroupId(recordName: String, zoneName: String)
s"""Zone "$zoneName" is a shared zone, so owner group ID must be specified for record "$recordName".""" s"""Zone "$zoneName" is a shared zone, so owner group ID must be specified for record "$recordName"."""
} }
final case class ExistingMultiRecordError(fqdn: String, record: RecordSet)
extends DomainValidationError {
def message: String =
s"""RecordSet with name $fqdn and type ${record.typ.toString} cannot be updated in a single Batch Change
|because it contains multiple DNS records (${record.records.length}).""".stripMargin
.replaceAll("\n", " ")
}
final case class NewMultiRecordError(changeName: String, changeType: RecordType)
extends DomainValidationError {
def message: String =
s"""Multi-record recordsets are not enabled for this instance of VinylDNS.
|Cannot create a new record set with multiple records for inputName $changeName and
|type $changeType.""".stripMargin
.replaceAll("\n", " ")
}
final case class CnameAtZoneApexError(zoneName: String) extends DomainValidationError { final case class CnameAtZoneApexError(zoneName: String) extends DomainValidationError {
def message: String = s"""CNAME cannot be the same name as zone "$zoneName".""" def message: String = s"""CNAME cannot be the same name as zone "$zoneName"."""
} }
@@ -189,4 +172,22 @@ final case class DeleteRecordDataDoesNotExist(inputName: String, recordData: Rec
def message: String = s"""Record data $recordData does not exist for "$inputName".""" def message: String = s"""Record data $recordData does not exist for "$inputName"."""
} }
// Deprecated errors
final case class ExistingMultiRecordError(fqdn: String, record: RecordSet)
extends DomainValidationError {
def message: String =
s"""RecordSet with name $fqdn and type ${record.typ.toString} cannot be updated in a single Batch Change
|because it contains multiple DNS records (${record.records.length}).""".stripMargin
.replaceAll("\n", " ")
}
final case class NewMultiRecordError(changeName: String, changeType: RecordType)
extends DomainValidationError {
def message: String =
s"""Multi-record recordsets are not enabled for this instance of VinylDNS.
|Cannot create a new record set with multiple records for inputName $changeName and
|type $changeType.""".stripMargin
.replaceAll("\n", " ")
}
// $COVERAGE-ON$ // $COVERAGE-ON$

6
modules/portal/app/models/Meta.scala
View File

@@ -23,8 +23,7 @@ case class Meta(
batchChangeLimit: Int, batchChangeLimit: Int,
defaultTtl: Long, defaultTtl: Long,
manualBatchChangeReviewEnabled: Boolean, manualBatchChangeReviewEnabled: Boolean,
scheduledBatchChangesEnabled: Boolean, scheduledBatchChangesEnabled: Boolean)
multiRecordBatchChangeEnabled: Boolean)
object Meta { object Meta {
def apply(config: Configuration): Meta = def apply(config: Configuration): Meta =
Meta( Meta(
@@ -33,7 +32,6 @@ object Meta {
config.getOptional[Int]("batch-change-limit").getOrElse(1000), config.getOptional[Int]("batch-change-limit").getOrElse(1000),
config.getOptional[Long]("default-ttl").getOrElse(7200L), config.getOptional[Long]("default-ttl").getOrElse(7200L),
config.getOptional[Boolean]("manual-batch-review-enabled").getOrElse(false), config.getOptional[Boolean]("manual-batch-review-enabled").getOrElse(false),
config.getOptional[Boolean]("scheduled-changes-enabled").getOrElse(false), config.getOptional[Boolean]("scheduled-changes-enabled").getOrElse(false)
config.getOptional[Boolean]("multi-record-batch-change-enabled").getOrElse(false)
) )
} }

22
modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html
View File

@@ -149,20 +149,20 @@
</p> </p>
</td> </td>
<td ng-if="change.type=='A'"> <td ng-if="change.type=='A'">
<input name="record_address_{{$index}}" type="text" ng-model="change.record.address" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. 1.1.1.1" ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled" ipv4> <input name="record_address_{{$index}}" type="text" ng-model="change.record.address" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. 1.1.1.1" ipv4>
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span> <span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span>
<span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.ipv4" class="batch-change-error-help">must be a valid IPv4 Address!</span> <span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.ipv4" class="batch-change-error-help">must be a valid IPv4 Address!</span>
</p> </p>
<p class="help-block" ng-if="change.changeType=='DeleteRecordSet' && @meta.multiRecordBatchChangeEnabled">Record Data is optional.</p> <p class="help-block" ng-if="change.changeType=='DeleteRecordSet'">Record Data is optional.</p>
</td> </td>
<td ng-if="change.type=='AAAA'"> <td ng-if="change.type=='AAAA'">
<input name="record_address_{{$index}}" type="text" ng-model="change.record.address" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. fd69:27cc:fe91::60" ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled" ipv6> <input name="record_address_{{$index}}" type="text" ng-model="change.record.address" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. fd69:27cc:fe91::60" ipv6>
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span> <span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span>
<span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.ipv6" class="batch-change-error-help">must be a valid IPv6 Address!</span> <span ng-show="createBatchChangeForm.record_address_{{$index}}.$error.ipv6" class="batch-change-error-help">must be a valid IPv6 Address!</span>
</p> </p>
<p class="help-block" ng-if="change.changeType=='DeleteRecordSet' && @meta.multiRecordBatchChangeEnabled">Record Data is optional.</p> <p class="help-block" ng-if="change.changeType=='DeleteRecordSet'">Record Data is optional.</p>
</td> </td>
<td ng-if="change.type=='CNAME'"> <td ng-if="change.type=='CNAME'">
<input name="record_cname_{{$index}}" type="text" ng-model="change.record.cname" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. test.example.com." ng-disabled="change.changeType=='DeleteRecordSet'" fqdn> <input name="record_cname_{{$index}}" type="text" ng-model="change.record.cname" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. test.example.com." ng-disabled="change.changeType=='DeleteRecordSet'" fqdn>
@@ -172,22 +172,22 @@
</p> </p>
</td> </td>
<td ng-if="change.type=='PTR'"> <td ng-if="change.type=='PTR'">
<input name="record_ptr_{{$index}}" type="text" ng-model="change.record.ptrdname" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. test.com." ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled"> <input name="record_ptr_{{$index}}" type="text" ng-model="change.record.ptrdname" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. test.com.">
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_ptr_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span> <span ng-show="createBatchChangeForm.record_ptr_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span>
</p> </p>
<p class="help-block" ng-if="change.changeType=='DeleteRecordSet' && @meta.multiRecordBatchChangeEnabled">Record Data is optional.</p> <p class="help-block" ng-if="change.changeType=='DeleteRecordSet'">Record Data is optional.</p>
</td> </td>
<td ng-if="change.type=='TXT'"> <td ng-if="change.type=='TXT'">
<textarea name="record_txt_{{$index}}" type="text" ng-model="change.record.text" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. attr=val" ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled"></textarea> <textarea name="record_txt_{{$index}}" type="text" ng-model="change.record.text" ng-required="change.changeType=='Add'" class="form-control" placeholder="e.g. attr=val"></textarea>
<p class="help-block" ng-if="change.changeType=='DeleteRecordSet' && @meta.multiRecordBatchChangeEnabled">Record Data is optional.</p> <p class="help-block" ng-if="change.changeType=='DeleteRecordSet'">Record Data is optional.</p>
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_txt_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span> <span ng-show="createBatchChangeForm.record_txt_{{$index}}.$error.required" class="batch-change-error-help">Record data is required!</span>
</p> </p>
</td> </td>
<td ng-if="change.type=='MX'"> <td ng-if="change.type=='MX'">
<label class="batch-label">Preference</label> <label class="batch-label">Preference</label>
<input name="record_mx_preference_{{$index}}" type="number" ng-model="change.record.preference" ng-required="change.changeType=='Add' || change.record.exchange" class="form-control" placeholder="e.g. 1" ng-min="0" ng-max="65535" ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled"> <input name="record_mx_preference_{{$index}}" type="number" ng-model="change.record.preference" ng-required="change.changeType=='Add' || change.record.exchange" class="form-control" placeholder="e.g. 1" ng-min="0" ng-max="65535">
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_mx_preference_{{$index}}.$error.required" class="batch-change-error-help">Preference is required!</span> <span ng-show="createBatchChangeForm.record_mx_preference_{{$index}}.$error.required" class="batch-change-error-help">Preference is required!</span>
<span ng-show="createBatchChangeForm.record_mx_preference_{{$index}}.$error.min" class="batch-change-error-help">Must be between 0 and 65535!</span> <span ng-show="createBatchChangeForm.record_mx_preference_{{$index}}.$error.min" class="batch-change-error-help">Must be between 0 and 65535!</span>
@@ -198,12 +198,12 @@
<br /> <br />
<label class="batch-label">Exchange</label> <label class="batch-label">Exchange</label>
<input name="record_mx_exchange_{{$index}}" type="text" ng-model="change.record.exchange" ng-required="change.changeType=='Add' || change.record.preference==0 || change.record.preference>0" class="form-control" placeholder="e.g. test.example.com." ng-disabled="change.changeType=='DeleteRecordSet' && !@meta.multiRecordBatchChangeEnabled" fqdn> <input name="record_mx_exchange_{{$index}}" type="text" ng-model="change.record.exchange" ng-required="change.changeType=='Add' || change.record.preference==0 || change.record.preference>0" class="form-control" placeholder="e.g. test.example.com." fqdn>
<p ng-show="createBatchChangeForm.$submitted"> <p ng-show="createBatchChangeForm.$submitted">
<span ng-show="createBatchChangeForm.record_mx_exchange_{{$index}}.$error.required" class="batch-change-error-help">Exchange data is required!</span> <span ng-show="createBatchChangeForm.record_mx_exchange_{{$index}}.$error.required" class="batch-change-error-help">Exchange data is required!</span>
<span ng-show="createBatchChangeForm.record_mx_exchange_{{$index}}.$error.fqdn" class="batch-change-error-help">Exchange data must be absolute!</span> <span ng-show="createBatchChangeForm.record_mx_exchange_{{$index}}.$error.fqdn" class="batch-change-error-help">Exchange data must be absolute!</span>
</p> </p>
<p class="help-block" ng-if="change.changeType=='DeleteRecordSet' && @meta.multiRecordBatchChangeEnabled">Record Data is optional.</p> <p class="help-block" ng-if="change.changeType=='DeleteRecordSet'">Record Data is optional.</p>
</td> </td>
<!--end record data name based on record type and change type--> <!--end record data name based on record type and change type-->
<td class="col-md-3" ng-if="batchChangeErrors"> <td class="col-md-3" ng-if="batchChangeErrors">

8
modules/portal/test/models/MetaSpec.scala
View File

@@ -65,13 +65,5 @@ class MetaSpec extends Specification with Mockito {
val config = Map("scheduled-changes-enabled" -> true) val config = Map("scheduled-changes-enabled" -> true)
Meta(Configuration.from(config)).scheduledBatchChangesEnabled must beTrue Meta(Configuration.from(config)).scheduledBatchChangesEnabled must beTrue
} }
"default to false if multi-record-batch-change-enabled is not found" in {
val config = Map("vinyldns.version" -> "foo-bar")
Meta(Configuration.from(config)).multiRecordBatchChangeEnabled must beFalse
}
"set to true if multi-record-batch-change-enabled is true in config" in {
val config = Map("multi-record-batch-change-enabled" -> true)
Meta(Configuration.from(config)).multiRecordBatchChangeEnabled must beTrue
}
} }
} }