diff --git a/docker/functest/run.sh b/docker/functest/run.sh index 323479619..2a0fdf2e6 100644 --- a/docker/functest/run.sh +++ b/docker/functest/run.sh @@ -54,28 +54,23 @@ find . -name "__pycache__" -delete result=0 # If PROD_ENV is not true, we are in a local docker environment so do not skip anything if [ "${PROD_ENV}" = "true" ]; then - # -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 \"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 + PARALLEL_TEST_MARKER="not skip_production and not serial" + SERIAL_TEST_MARKER="not skip_production and serial" else - # run parallel tests first (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" - ./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 + PARALLEL_TEST_MARKER="not serial" + SERIAL_TEST_MARKER="serial" +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=$? - 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 exit $result diff --git a/modules/api/functional_test/live_tests/batch/create_batch_change_test.py b/modules/api/functional_test/live_tests/batch/create_batch_change_test.py index db4d353bf..2578a06b9 100644 --- a/modules/api/functional_test/live_tests/batch/create_batch_change_test.py +++ b/modules/api/functional_test/live_tests/batch/create_batch_change_test.py @@ -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') -@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 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: 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 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_recordset_list(to_delete, ok_client) - -@pytest.mark.multi_record_enabled @pytest.mark.skip_production 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: clear_recordset_list(to_delete, client) -@pytest.mark.multi_record_enabled def test_create_batch_deletes_succeeds(shared_zone_test_context): """ 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: clear_recordset_list(to_delete, client) - -@pytest.mark.multi_record_enabled @pytest.mark.serial @pytest.mark.skip_production def test_create_batch_change_with_multi_record_adds_with_multi_record_support(shared_zone_test_context): diff --git a/modules/api/src/main/scala/vinyldns/api/Boot.scala b/modules/api/src/main/scala/vinyldns/api/Boot.scala index 120b263d5..f2e07992c 100644 --- a/modules/api/src/main/scala/vinyldns/api/Boot.scala +++ b/modules/api/src/main/scala/vinyldns/api/Boot.scala @@ -117,7 +117,6 @@ object Boot extends App { val batchChangeValidations = new BatchChangeValidations( batchChangeLimit, batchAccessValidations, - VinylDNSConfig.multiRecordBatchUpdateEnabled, VinylDNSConfig.scheduledChangesEnabled ) val membershipService = MembershipService(repositories) diff --git a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala index 33cbaaf4c..ae16cf4d1 100644 --- a/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala +++ b/modules/api/src/main/scala/vinyldns/api/VinylDNSConfig.scala @@ -129,9 +129,7 @@ object VinylDNSConfig { } else List() 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 multiRecordBatchUpdateEnabled: Boolean = - vinyldnsConfig.as[Option[Boolean]]("multi-record-batch-change-enabled").getOrElse(false) + lazy val defaultTtl: Long = vinyldnsConfig.as[Option[Long]]("default-ttl").getOrElse(7200L) lazy val manualBatchReviewEnabled: Boolean = vinyldnsConfig .as[Option[Boolean]]("manual-batch-review-enabled") .getOrElse(false) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala index 4ed73111f..22860bdb3 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala @@ -69,7 +69,6 @@ trait BatchChangeValidationsAlgebra { class BatchChangeValidations( changeLimit: Int, accessValidation: AccessValidationsAlgebra, - multiRecordEnabled: Boolean = false, scheduledChangesEnabled: Boolean = false) extends BatchChangeValidationsAlgebra { @@ -267,23 +266,6 @@ class BatchChangeValidations( 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] = if (change.recordName != change.zone.name && change.recordName.contains(".")) ZoneDiscoveryError(change.inputChange.inputName).invalidNel @@ -317,7 +299,6 @@ class BatchChangeValidations( groupedChanges.getExistingRecordSet(change.recordKey) match { case Some(rs) => userCanDeleteRecordSet(change, auth, rs.ownerGroupId, rs.records) |+| - existingRecordSetIsNotMulti(change, rs) |+| zoneDoesNotRequireManualReview(change, isApproved) |+| ensureRecordExists(change, groupedChanges) case None => RecordDoesNotExist(change.inputChange.inputName).invalidNel @@ -335,7 +316,7 @@ class BatchChangeValidations( // could potentially be grouped with a single delete val typedValidations = change.inputChange.typ match { case CNAME => recordIsUniqueInBatch(change, groupedChanges) - case _ => newRecordSetIsNotMulti(change, groupedChanges) + case _ => ().validNel } val commonValidations: SingleValidation[Unit] = { @@ -347,7 +328,6 @@ class BatchChangeValidations( groupedChanges.existingRecordSets .get(change.zone.id, change.recordName, change.inputChange.typ), batchOwnerGroupId) |+| - existingRecordSetIsNotMulti(change, rs) |+| zoneDoesNotRequireManualReview(change, isApproved) case None => RecordDoesNotExist(change.inputChange.inputName).invalidNel @@ -369,7 +349,6 @@ class BatchChangeValidations( case Some(rs) => val adds = groupedChanges.getProposedAdds(change.recordKey).toList userCanUpdateRecordSet(change, auth, rs.ownerGroupId, adds) |+| - existingRecordSetIsNotMulti(change, rs) |+| zoneDoesNotRequireManualReview(change, isApproved) |+| ensureRecordExists(change, groupedChanges) case None => @@ -387,13 +366,12 @@ class BatchChangeValidations( ownerGroupId: Option[String]): SingleValidation[ChangeForValidation] = { val typedValidations = change.inputChange.typ match { case A | AAAA | MX => - newRecordSetIsNotMulti(change, groupedChanges) |+| - newRecordSetIsNotDotted(change) + newRecordSetIsNotDotted(change) case CNAME => cnameHasUniqueNameInBatch(change, groupedChanges) |+| newRecordSetIsNotDotted(change) case TXT | PTR => - newRecordSetIsNotMulti(change, groupedChanges) + ().validNel case other => InvalidBatchRecordType(other.toString, SupportedBatchChangeRecordTypes.get).invalidNel } diff --git a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala index 1a4f97070..a2ea5834f 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeValidationsSpec.scala @@ -51,9 +51,7 @@ class BatchChangeValidationsSpec private val maxChanges = 10 private val accessValidations = new AccessValidations() private val underTest = - new BatchChangeValidations(maxChanges, accessValidations, multiRecordEnabled = true) - private val underTestMultiDisabled = - new BatchChangeValidations(maxChanges, accessValidations, multiRecordEnabled = false) + new BatchChangeValidations(maxChanges, accessValidations) import underTest._ @@ -293,11 +291,8 @@ class BatchChangeValidationsSpec None, List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))), scheduledTime = Some(DateTime.now)) - val bcv = new BatchChangeValidations( - maxChanges, - accessValidations, - multiRecordEnabled = true, - scheduledChangesEnabled = false) + val bcv = + new BatchChangeValidations(maxChanges, accessValidations, scheduledChangesEnabled = false) bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left( ScheduledChangesDisabled) } @@ -308,11 +303,8 @@ class BatchChangeValidationsSpec None, List(AddChangeInput("private-create", RecordType.A, ttl, AData("1.1.1.1"))), scheduledTime = Some(DateTime.now.minusHours(1))) - val bcv = new BatchChangeValidations( - maxChanges, - accessValidations, - multiRecordEnabled = true, - scheduledChangesEnabled = true) + val bcv = + new BatchChangeValidations(maxChanges, accessValidations, scheduledChangesEnabled = true) bcv.validateBatchChangeInput(input, None, okAuth).value.unsafeRunSync() shouldBe Left( ScheduledTimeMustBeInFuture) } @@ -1953,8 +1945,7 @@ class BatchChangeValidationsSpec result(0) shouldBe valid } - property( - "validateChangesWithContext: succeed update/delete to a multi record existing RecordSet if multi enabled") { + property("validateChangesWithContext: succeed update/delete to a multi record existing RecordSet") { val existing = List( sharedZoneRecord.copy( name = updateSharedAddChange.recordName, @@ -1992,49 +1983,7 @@ class BatchChangeValidationsSpec result(5) shouldBe valid } - property( - "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") { + property("validateChangesWithContext: succeed on add/update to a multi record") { val existing = List( sharedZoneRecord.copy(name = updateSharedAddChange.recordName) ) @@ -2079,58 +2028,6 @@ class BatchChangeValidationsSpec 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 |ZoneDiscoveryError if new record is dotted host but not a TXT record type""".stripMargin) { val addA = AddChangeForValidation( diff --git a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala index 1a7b207b5..02305f4b7 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala @@ -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".""" } -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 { 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".""" } +// 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$ diff --git a/modules/portal/app/models/Meta.scala b/modules/portal/app/models/Meta.scala index c9c0771c3..b69edaeb6 100644 --- a/modules/portal/app/models/Meta.scala +++ b/modules/portal/app/models/Meta.scala @@ -23,8 +23,7 @@ case class Meta( batchChangeLimit: Int, defaultTtl: Long, manualBatchChangeReviewEnabled: Boolean, - scheduledBatchChangesEnabled: Boolean, - multiRecordBatchChangeEnabled: Boolean) + scheduledBatchChangesEnabled: Boolean) object Meta { def apply(config: Configuration): Meta = Meta( @@ -33,7 +32,6 @@ object Meta { config.getOptional[Int]("batch-change-limit").getOrElse(1000), config.getOptional[Long]("default-ttl").getOrElse(7200L), config.getOptional[Boolean]("manual-batch-review-enabled").getOrElse(false), - config.getOptional[Boolean]("scheduled-changes-enabled").getOrElse(false), - config.getOptional[Boolean]("multi-record-batch-change-enabled").getOrElse(false) + config.getOptional[Boolean]("scheduled-changes-enabled").getOrElse(false) ) } diff --git a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html index d76151915..b5ef0aeb7 100644 --- a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html +++ b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html @@ -149,20 +149,20 @@

- +

Record data is required! must be a valid IPv4 Address!

-

Record Data is optional.

+

Record Data is optional.

- +

Record data is required! must be a valid IPv6 Address!

-

Record Data is optional.

+

Record Data is optional.

@@ -172,22 +172,22 @@

- +

Record data is required!

-

Record Data is optional.

+

Record Data is optional.

- -

Record Data is optional.

+ +

Record Data is optional.

Record data is required!

- +

Preference is required! Must be between 0 and 65535! @@ -198,12 +198,12 @@
- +

Exchange data is required! Exchange data must be absolute!

-

Record Data is optional.

+

Record Data is optional.

diff --git a/modules/portal/test/models/MetaSpec.scala b/modules/portal/test/models/MetaSpec.scala index d292f6e9f..e75fc9f7c 100644 --- a/modules/portal/test/models/MetaSpec.scala +++ b/modules/portal/test/models/MetaSpec.scala @@ -65,13 +65,5 @@ class MetaSpec extends Specification with Mockito { val config = Map("scheduled-changes-enabled" -> true) 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 - } } }