Group {{membership.group.name}}
Description: {{membership.group.description}}
-Group Email: {{membership.group.email}}
- -diff --git a/modules/api/src/main/resources/reference.conf b/modules/api/src/main/resources/reference.conf index b82cb74d6..aff3f95cc 100644 --- a/modules/api/src/main/resources/reference.conf +++ b/modules/api/src/main/resources/reference.conf @@ -110,8 +110,8 @@ vinyldns { limits { batchchange-routing-max-items-limit = 100 membership-routing-default-max-items = 100 - membership-routing-max-items-limit = 1000 - membership-routing-max-groups-list-limit = 1500 + membership-routing-max-items-limit = 100 + membership-routing-max-groups-list-limit = 100 recordset-routing-default-max-items= 100 zone-routing-default-max-items = 100 zone-routing-max-items-limit = 100 diff --git a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeConverter.scala b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeConverter.scala index 8efa109fb..647e348e8 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeConverter.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeConverter.scala @@ -27,12 +27,15 @@ import vinyldns.api.domain.record.RecordSetChangeGenerator import vinyldns.core.domain.record._ import vinyldns.core.domain.zone.Zone import vinyldns.core.domain.batch._ -import vinyldns.core.domain.record.RecordType.RecordType +import vinyldns.core.domain.record.RecordType.{RecordType, UNKNOWN} import vinyldns.core.queue.MessageQueue class BatchChangeConverter(batchChangeRepo: BatchChangeRepository, messageQueue: MessageQueue) extends BatchChangeConverterAlgebra { + private val notExistCompletedMessage: String = "This record does not exist." + + "No further action is required." + private val failedMessage: String = "Error queueing RecordSetChange for processing" private val logger = LoggerFactory.getLogger(classOf[BatchChangeConverter]) def sendBatchForProcessing( @@ -68,6 +71,12 @@ class BatchChangeConverter(batchChangeRepo: BatchChangeRepository, messageQueue: ): BatchResult[Unit] = { val convertedIds = recordSetChanges.flatMap(_.singleBatchChangeIds).toSet singleChanges.find(ch => !convertedIds.contains(ch.id)) match { + // Each single change has a corresponding recordset id + // If they're not equal, then there's a delete request for a record that doesn't exist. So we allow this to process + case Some(_) if singleChanges.map(_.id).length != recordSetChanges.map(_.id).length && !singleChanges.map(_.typ).contains(UNKNOWN) => + logger.info(s"Successfully converted SingleChanges [${singleChanges + .map(_.id)}] to RecordSetChanges [${recordSetChanges.map(_.id)}]") + ().toRightBatchResult case Some(change) => BatchConversionError(change).toLeftBatchResult case None => logger.info(s"Successfully converted SingleChanges [${singleChanges @@ -104,21 +113,33 @@ class BatchChangeConverter(batchChangeRepo: BatchChangeRepository, messageQueue: rsChange.singleBatchChangeIds.map(batchId => (batchId, rsChange.id)) }.toMap val withStatus = batchChange.changes.map { change => - idsMap - .get(change.id) - .map { _ => change } // a recordsetchange was successfully queued for this change - .getOrElse { - // failure here means there was a message queue issue for this change - change.withFailureMessage("Error queueing RecordSetChange for processing") + idsMap + .get(change.id) + .map { _ => + // a recordsetchange was successfully queued for this change + change + } + .getOrElse { + // Match and check if it's a delete change for a record that doesn't exists. + change match { + case _: SingleDeleteRRSetChange if change.recordSetId.isEmpty => + // Mark as Complete since we don't want to throw it as an error + change.withDoesNotExistMessage(notExistCompletedMessage) + case _ => + // Failure here means there was a message queue issue for this change + change.withFailureMessage(failedMessage) } - } + } + } batchChange.copy(changes = withStatus) } def storeQueuingFailures(batchChange: BatchChange): BatchResult[Unit] = { - val failedChanges = batchChange.changes.collect { - case change if change.status == SingleChangeStatus.Failed => change } - batchChangeRepo.updateSingleChanges(failedChanges).as(()) + // Update if Single change is Failed or if a record that does not exist is deleted + val failedAndNotExistsChanges = batchChange.changes.collect { + case change if change.status == SingleChangeStatus.Failed || change.systemMessage.contains(notExistCompletedMessage) => change + } + batchChangeRepo.updateSingleChanges(failedAndNotExistsChanges).as(()) }.toBatchResult def createRecordSetChangesForBatch( 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 85caea6d6..0ed20d0cf 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 @@ -348,7 +348,7 @@ class BatchChangeValidations( userCanDeleteRecordSet(change, auth, rs.ownerGroupId, rs.records) |+| zoneDoesNotRequireManualReview(change, isApproved) |+| ensureRecordExists(change, groupedChanges) - case None => RecordDoesNotExist(change.inputChange.inputName).invalidNel + case None => RecordDoesNotExist(change.inputChange.inputName).validNel } validations.map(_ => change) } @@ -402,7 +402,7 @@ class BatchChangeValidations( zoneDoesNotRequireManualReview(change, isApproved) |+| ensureRecordExists(change, groupedChanges) case None => - RecordDoesNotExist(change.inputChange.inputName).invalidNel + RecordDoesNotExist(change.inputChange.inputName).validNel } validations.map(_ => change) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala index 02cadc115..423ac4d04 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipProtocol.scala @@ -59,7 +59,9 @@ final case class GroupChangeInfo( userId: String, oldGroup: Option[GroupInfo] = None, id: String = UUID.randomUUID().toString, - created: String = DateTime.now.getMillis.toString + created: DateTime = DateTime.now, + userName: String, + groupChangeMessage: String ) object GroupChangeInfo { @@ -69,7 +71,9 @@ object GroupChangeInfo { userId = groupChange.userId, oldGroup = groupChange.oldGroup.map(GroupInfo.apply), id = groupChange.id, - created = groupChange.created.getMillis.toString + created = groupChange.created, + userName = groupChange.userName.getOrElse("unknown user"), + groupChangeMessage = groupChange.groupChangeMessage.getOrElse("") ) } diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala index 9345bb4ff..08e7cc240 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala @@ -216,12 +216,18 @@ class MembershipService( ): ListMyGroupsResponse = { val allMyGroups = allGroups .filter(_.status == GroupStatus.Active) - .sortBy(_.id) + .sortBy(_.name.toLowerCase) .map(x => GroupInfo.fromGroup(x, abridged, Some(authPrincipal))) - val filtered = allMyGroups - .filter(grp => groupNameFilter.map(_.toLowerCase).forall(grp.name.toLowerCase.contains(_))) - .filter(grp => startFrom.forall(grp.id > _)) + val filtered = if(startFrom.isDefined){ + val prevPageGroup = allMyGroups.filter(_.id == startFrom.get).head.name + allMyGroups + .filter(grp => groupNameFilter.map(_.toLowerCase).forall(grp.name.toLowerCase.contains(_))) + .filter(grp => grp.name.toLowerCase > prevPageGroup.toLowerCase) + } else { + allMyGroups + .filter(grp => groupNameFilter.map(_.toLowerCase).forall(grp.name.toLowerCase.contains(_))) + } val nextId = if (filtered.length > maxItems) Some(filtered(maxItems - 1).id) else None val groups = filtered.take(maxItems) @@ -229,6 +235,23 @@ class MembershipService( ListMyGroupsResponse(groups, groupNameFilter, startFrom, nextId, maxItems, ignoreAccess) } + def getGroupChange( + groupChangeId: String, + authPrincipal: AuthPrincipal + ): Result[GroupChangeInfo] = + for { + result <- groupChangeRepo + .getGroupChange(groupChangeId) + .toResult[Option[GroupChange]] + _ <- isGroupChangePresent(result).toResult + _ <- canSeeGroup(result.get.newGroup.id, authPrincipal).toResult + groupChangeMessage <- determineGroupDifference(Seq(result.get)) + groupChanges = (groupChangeMessage, Seq(result.get)).zipped.map{ (a, b) => b.copy(groupChangeMessage = Some(a)) } + userIds = Seq(result.get).map(_.userId).toSet + users <- getUsers(userIds).map(_.users) + userMap = users.map(u => (u.id, u.userName)).toMap + } yield groupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))).head + def getGroupActivity( groupId: String, startFrom: Option[String], @@ -240,13 +263,65 @@ class MembershipService( result <- groupChangeRepo .getGroupChanges(groupId, startFrom, maxItems) .toResult[ListGroupChangesResults] + groupChangeMessage <- determineGroupDifference(result.changes) + groupChanges = (groupChangeMessage, result.changes).zipped.map{ (a, b) => b.copy(groupChangeMessage = Some(a)) } + userIds = result.changes.map(_.userId).toSet + users <- getUsers(userIds).map(_.users) + userMap = users.map(u => (u.id, u.userName)).toMap } yield ListGroupChangesResponse( - result.changes.map(GroupChangeInfo.apply), + groupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))), startFrom, result.lastEvaluatedTimeStamp, maxItems ) + def determineGroupDifference(groupChange: Seq[GroupChange]): Result[Seq[String]] = { + var groupChangeMessage: Seq[String] = Seq.empty[String] + + for (change <- groupChange) { + val sb = new StringBuilder + if (change.oldGroup.isDefined) { + if (change.oldGroup.get.name != change.newGroup.name) { + sb.append(s"Group name changed to '${change.newGroup.name}'. ") + } + if (change.oldGroup.get.email != change.newGroup.email) { + sb.append(s"Group email changed to '${change.newGroup.email}'. ") + } + if (change.oldGroup.get.description != change.newGroup.description) { + sb.append(s"Group description changed to '${change.newGroup.description.get}'. ") + } + val adminAddDifference = change.newGroup.adminUserIds.diff(change.oldGroup.get.adminUserIds) + if (adminAddDifference.nonEmpty) { + sb.append(s"Group admin/s with userId/s (${adminAddDifference.mkString(",")}) added. ") + } + val adminRemoveDifference = change.oldGroup.get.adminUserIds.diff(change.newGroup.adminUserIds) + if (adminRemoveDifference.nonEmpty) { + sb.append(s"Group admin/s with userId/s (${adminRemoveDifference.mkString(",")}) removed. ") + } + val memberAddDifference = change.newGroup.memberIds.diff(change.oldGroup.get.memberIds) + if (memberAddDifference.nonEmpty) { + sb.append(s"Group member/s with userId/s (${memberAddDifference.mkString(",")}) added. ") + } + val memberRemoveDifference = change.oldGroup.get.memberIds.diff(change.newGroup.memberIds) + if (memberRemoveDifference.nonEmpty) { + sb.append(s"Group member/s with userId/s (${memberRemoveDifference.mkString(",")}) removed. ") + } + groupChangeMessage = groupChangeMessage :+ sb.toString().trim + } + // It'll be in else statement if the group was created or deleted + else { + if (change.changeType == GroupChangeType.Create) { + sb.append("Group Created.") + } + else if (change.changeType == GroupChangeType.Delete){ + sb.append("Group Deleted.") + } + groupChangeMessage = groupChangeMessage :+ sb.toString() + } + } + groupChangeMessage + }.toResult + /** * Retrieves the requested User from the given userIdentifier, which can be a userId or username * @param userIdentifier The userId or username diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipServiceAlgebra.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipServiceAlgebra.scala index 5fb2f737b..c71d62c2a 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipServiceAlgebra.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipServiceAlgebra.scala @@ -39,6 +39,8 @@ trait MembershipServiceAlgebra { def getGroup(id: String, authPrincipal: AuthPrincipal): Result[Group] + def getGroupChange(id: String, authPrincipal: AuthPrincipal): Result[GroupChangeInfo] + def listMyGroups( groupNameFilter: Option[String], startFrom: Option[String], diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipValidations.scala index cccd1d5e2..898caff91 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipValidations.scala @@ -19,7 +19,7 @@ package vinyldns.api.domain.membership import vinyldns.api.Interfaces.ensuring import vinyldns.core.domain.auth.AuthPrincipal import vinyldns.api.domain.zone.NotAuthorizedError -import vinyldns.core.domain.membership.Group +import vinyldns.core.domain.membership.{Group, GroupChange} object MembershipValidations { @@ -44,4 +44,9 @@ object MembershipValidations { ensuring(NotAuthorizedError("Not authorized")) { authPrincipal.isGroupMember(groupId) || authPrincipal.isSystemAdmin || canViewGroupDetails } + + def isGroupChangePresent(groupChange: Option[GroupChange]): Either[Throwable, Unit] = + ensuring(InvalidGroupRequestError("Invalid Group Change ID")) { + groupChange.isDefined + } } diff --git a/modules/api/src/main/scala/vinyldns/api/route/MembershipJsonProtocol.scala b/modules/api/src/main/scala/vinyldns/api/route/MembershipJsonProtocol.scala index 4c3084c29..7ec82e732 100644 --- a/modules/api/src/main/scala/vinyldns/api/route/MembershipJsonProtocol.scala +++ b/modules/api/src/main/scala/vinyldns/api/route/MembershipJsonProtocol.scala @@ -120,7 +120,9 @@ trait MembershipJsonProtocol extends JsonValidation { (js \ "userId").required[String]("Missing userId"), (js \ "oldGroup").optional[GroupInfo], (js \ "id").default[String](UUID.randomUUID().toString), - (js \ "created").default[String](DateTime.now.getMillis.toString) - ).mapN(GroupChangeInfo.apply) + (js \ "created").default[DateTime](DateTime.now), + (js \ "userName").required[String]("Missing userName"), + (js \ "groupChangeMessage").required[String]("Missing groupChangeMessage"), + ).mapN(GroupChangeInfo.apply) } } diff --git a/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala b/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala index 8ec1399e1..8b4b59509 100644 --- a/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala +++ b/modules/api/src/main/scala/vinyldns/api/route/MembershipRouting.scala @@ -80,7 +80,7 @@ class MembershipRoute( } ~ (get & monitor("Endpoint.listMyGroups")) { parameters( - "startFrom".?, + "startFrom".as[String].?, "maxItems".as[Int].?(DEFAULT_MAX_ITEMS), "groupNameFilter".?, "ignoreAccess".as[Boolean].?(false), @@ -179,6 +179,13 @@ class MembershipRoute( } } } ~ + path("groups" / "change" / Segment) { groupChangeId => + (get & monitor("Endpoint.groupSingleChange")) { + authenticateAndExecute(membershipService.getGroupChange(groupChangeId, _)) { groupChange => + complete(StatusCodes.OK, groupChange) + } + } + } ~ path("users" / Segment / "lock") { id => (put & monitor("Endpoint.lockUser")) { authenticateAndExecute(membershipService.updateUserLockStatus(id, LockStatus.Locked, _)) { diff --git a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py index 814dda102..341567e4a 100644 --- a/modules/api/src/test/functional/tests/batch/create_batch_change_test.py +++ b/modules/api/src/test/functional/tests/batch/create_batch_change_test.py @@ -1542,6 +1542,7 @@ def test_a_recordtype_update_delete_checks(shared_zone_test_context): get_change_A_AAAA_json(rs_delete_fqdn, change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_update_fqdn, change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_update_fqdn, ttl=300), + get_change_A_AAAA_json(f"non-existent.{ok_zone_name}", change_type="DeleteRecordSet"), # input validations failures get_change_A_AAAA_json("$invalid.host.name.", change_type="DeleteRecordSet"), @@ -1555,7 +1556,6 @@ def test_a_recordtype_update_delete_checks(shared_zone_test_context): get_change_A_AAAA_json("zone.discovery.error.", change_type="DeleteRecordSet"), # context validation failures: record does not exist, not authorized - get_change_A_AAAA_json(f"non-existent.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_delete_dummy_fqdn, change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_update_dummy_fqdn, change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_update_dummy_fqdn, ttl=300), @@ -1592,43 +1592,40 @@ def test_a_recordtype_update_delete_checks(shared_zone_test_context): assert_successful_change_in_error_response(response[0], input_name=rs_delete_fqdn, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[1], input_name=rs_update_fqdn, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[2], input_name=rs_update_fqdn, ttl=300) + assert_successful_change_in_error_response(response[3], input_name=f"non-existent.{ok_zone_name}", change_type="DeleteRecordSet") # input validations failures - assert_failed_change_in_error_response(response[3], input_name="$invalid.host.name.", + assert_failed_change_in_error_response(response[4], input_name="$invalid.host.name.", change_type="DeleteRecordSet", error_messages=['Invalid domain name: "$invalid.host.name.", valid domain names must be letters, ' 'numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[4], input_name="reverse.zone.in-addr.arpa.", + assert_failed_change_in_error_response(response[5], input_name="reverse.zone.in-addr.arpa.", change_type="DeleteRecordSet", error_messages=['Invalid Record Type In Reverse Zone: record with name "reverse.zone.in-addr.arpa." and type "A" ' 'is not allowed in a reverse zone.']) - assert_failed_change_in_error_response(response[5], input_name="$another.invalid.host.name.", ttl=300, + assert_failed_change_in_error_response(response[6], input_name="$another.invalid.host.name.", ttl=300, error_messages=['Invalid domain name: "$another.invalid.host.name.", valid domain names must be letters, ' 'numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[6], input_name="$another.invalid.host.name.", + assert_failed_change_in_error_response(response[7], input_name="$another.invalid.host.name.", change_type="DeleteRecordSet", error_messages=['Invalid domain name: "$another.invalid.host.name.", valid domain names must be letters, ' 'numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[7], input_name="another.reverse.zone.in-addr.arpa.", ttl=10, + assert_failed_change_in_error_response(response[8], input_name="another.reverse.zone.in-addr.arpa.", ttl=10, error_messages=['Invalid Record Type In Reverse Zone: record with name "another.reverse.zone.in-addr.arpa." ' 'and type "A" is not allowed in a reverse zone.', 'Invalid TTL: "10", must be a number between 30 and 2147483647.']) - assert_failed_change_in_error_response(response[8], input_name="another.reverse.zone.in-addr.arpa.", + assert_failed_change_in_error_response(response[9], input_name="another.reverse.zone.in-addr.arpa.", change_type="DeleteRecordSet", error_messages=['Invalid Record Type In Reverse Zone: record with name "another.reverse.zone.in-addr.arpa." ' 'and type "A" is not allowed in a reverse zone.']) # zone discovery failure - assert_failed_change_in_error_response(response[9], input_name="zone.discovery.error.", + assert_failed_change_in_error_response(response[10], input_name="zone.discovery.error.", change_type="DeleteRecordSet", error_messages=['Zone Discovery Failed: zone for "zone.discovery.error." does not exist in VinylDNS. ' 'If zone exists, then it must be connected to in VinylDNS.']) # context validation failures: record does not exist, not authorized - assert_failed_change_in_error_response(response[10], input_name=f"non-existent.{ok_zone_name}", - change_type="DeleteRecordSet", - error_messages=[ - f'Record "non-existent.{ok_zone_name}" Does Not Exist: cannot delete a record that does not exist.']) assert_failed_change_in_error_response(response[11], input_name=rs_delete_dummy_fqdn, change_type="DeleteRecordSet", error_messages=[f'User \"ok\" is not authorized. Contact zone owner group: {dummy_group_name} at test@test.com to make DNS changes.']) @@ -1779,6 +1776,8 @@ def test_aaaa_recordtype_update_delete_checks(shared_zone_test_context): get_change_A_AAAA_json(rs_delete_fqdn, record_type="AAAA", change_type="DeleteRecordSet", address="1:0::4:5:6:7:8"), get_change_A_AAAA_json(rs_update_fqdn, record_type="AAAA", ttl=300, address="1:2:3:4:5:6:7:8"), get_change_A_AAAA_json(rs_update_fqdn, record_type="AAAA", change_type="DeleteRecordSet"), + get_change_A_AAAA_json(f"delete-nonexistent.{ok_zone_name}", record_type="AAAA", change_type="DeleteRecordSet"), + get_change_A_AAAA_json(f"update-nonexistent.{ok_zone_name}", record_type="AAAA", change_type="DeleteRecordSet"), # input validations failures get_change_A_AAAA_json(f"invalid-name$.{ok_zone_name}", record_type="AAAA", change_type="DeleteRecordSet"), @@ -1790,8 +1789,6 @@ def test_aaaa_recordtype_update_delete_checks(shared_zone_test_context): get_change_A_AAAA_json("no.zone.at.all.", record_type="AAAA", change_type="DeleteRecordSet"), # context validation failures - get_change_A_AAAA_json(f"delete-nonexistent.{ok_zone_name}", record_type="AAAA", change_type="DeleteRecordSet"), - get_change_A_AAAA_json(f"update-nonexistent.{ok_zone_name}", record_type="AAAA", change_type="DeleteRecordSet"), get_change_A_AAAA_json(f"update-nonexistent.{ok_zone_name}", record_type="AAAA", address="1::1"), get_change_A_AAAA_json(rs_delete_dummy_fqdn, record_type="AAAA", change_type="DeleteRecordSet"), get_change_A_AAAA_json(rs_update_dummy_fqdn, record_type="AAAA", address="1::1"), @@ -1824,39 +1821,37 @@ def test_aaaa_recordtype_update_delete_checks(shared_zone_test_context): record_data="1:2:3:4:5:6:7:8") assert_successful_change_in_error_response(response[2], input_name=rs_update_fqdn, record_type="AAAA", record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[3], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="AAAA", + record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"update-nonexistent.{ok_zone_name}", record_type="AAAA", + record_data=None, change_type="DeleteRecordSet") # input validations failures: invalid input name, reverse zone error, invalid ttl - assert_failed_change_in_error_response(response[3], input_name=f"invalid-name$.{ok_zone_name}", record_type="AAAA", + assert_failed_change_in_error_response(response[5], input_name=f"invalid-name$.{ok_zone_name}", record_type="AAAA", record_data=None, change_type="DeleteRecordSet", error_messages=[f'Invalid domain name: "invalid-name$.{ok_zone_name}", ' f'valid domain names must be letters, numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[4], input_name="reverse.zone.in-addr.arpa.", record_type="AAAA", + assert_failed_change_in_error_response(response[6], input_name="reverse.zone.in-addr.arpa.", record_type="AAAA", record_data=None, change_type="DeleteRecordSet", error_messages=["Invalid Record Type In Reverse Zone: record with name \"reverse.zone.in-addr.arpa.\" and " "type \"AAAA\" is not allowed in a reverse zone."]) - assert_failed_change_in_error_response(response[5], input_name=f"bad-ttl-and-invalid-name$-update.{ok_zone_name}", + assert_failed_change_in_error_response(response[7], input_name=f"bad-ttl-and-invalid-name$-update.{ok_zone_name}", record_type="AAAA", record_data=None, change_type="DeleteRecordSet", error_messages=[f'Invalid domain name: "bad-ttl-and-invalid-name$-update.{ok_zone_name}", ' f'valid domain names must be letters, numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[6], input_name=f"bad-ttl-and-invalid-name$-update.{ok_zone_name}", ttl=29, + assert_failed_change_in_error_response(response[8], input_name=f"bad-ttl-and-invalid-name$-update.{ok_zone_name}", ttl=29, record_type="AAAA", record_data="1:2:3:4:5:6:7:8", error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.', f'Invalid domain name: "bad-ttl-and-invalid-name$-update.{ok_zone_name}", ' f'valid domain names must be letters, numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) # zone discovery failure - assert_failed_change_in_error_response(response[7], input_name="no.zone.at.all.", record_type="AAAA", + assert_failed_change_in_error_response(response[9], input_name="no.zone.at.all.", record_type="AAAA", record_data=None, change_type="DeleteRecordSet", error_messages=["Zone Discovery Failed: zone for \"no.zone.at.all.\" does not exist in VinylDNS. " "If zone exists, then it must be connected to in VinylDNS."]) # context validation failures: record does not exist, not authorized - assert_failed_change_in_error_response(response[8], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="AAAA", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"delete-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) - assert_failed_change_in_error_response(response[9], input_name=f"update-nonexistent.{ok_zone_name}", record_type="AAAA", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"update-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) assert_successful_change_in_error_response(response[10], input_name=f"update-nonexistent.{ok_zone_name}", record_type="AAAA", record_data="1::1") assert_failed_change_in_error_response(response[11], input_name=rs_delete_dummy_fqdn, record_type="AAAA", record_data=None, change_type="DeleteRecordSet", @@ -2056,6 +2051,8 @@ def test_cname_recordtype_update_delete_checks(shared_zone_test_context): get_change_CNAME_json(f"delete3.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_CNAME_json(f"update3.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_CNAME_json(f"update3.{ok_zone_name}", ttl=300), + get_change_CNAME_json(f"non-existent-delete.{ok_zone_name}", change_type="DeleteRecordSet"), + get_change_CNAME_json(f"non-existent-update.{ok_zone_name}", change_type="DeleteRecordSet"), # valid changes - reverse zone get_change_CNAME_json(f"200.{ip4_zone_name}", change_type="DeleteRecordSet"), @@ -2071,8 +2068,6 @@ def test_cname_recordtype_update_delete_checks(shared_zone_test_context): get_change_CNAME_json("zone.discovery.error.", change_type="DeleteRecordSet"), # context validation failures: record does not exist, not authorized, failure on update with multiple adds - get_change_CNAME_json(f"non-existent-delete.{ok_zone_name}", change_type="DeleteRecordSet"), - get_change_CNAME_json(f"non-existent-update.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_CNAME_json(f"non-existent-update.{ok_zone_name}"), get_change_CNAME_json(f"delete-unauthorized3.{dummy_zone_name}", change_type="DeleteRecordSet"), get_change_CNAME_json(f"update-unauthorized3.{dummy_zone_name}", change_type="DeleteRecordSet"), @@ -2109,25 +2104,29 @@ def test_cname_recordtype_update_delete_checks(shared_zone_test_context): change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[2], input_name=f"update3.{ok_zone_name}", record_type="CNAME", ttl=300, record_data="test.com.") + assert_successful_change_in_error_response(response[3], input_name=f"non-existent-delete.{ok_zone_name}", record_type="CNAME", + change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"non-existent-update.{ok_zone_name}", record_type="CNAME", + change_type="DeleteRecordSet") # valid changes - reverse zone - assert_successful_change_in_error_response(response[3], input_name=f"200.{ip4_zone_name}", + assert_successful_change_in_error_response(response[5], input_name=f"200.{ip4_zone_name}", record_type="CNAME", change_type="DeleteRecordSet") - assert_successful_change_in_error_response(response[4], input_name=f"201.{ip4_zone_name}", + assert_successful_change_in_error_response(response[6], input_name=f"201.{ip4_zone_name}", record_type="CNAME", change_type="DeleteRecordSet") - assert_successful_change_in_error_response(response[5], input_name=f"201.{ip4_zone_name}", + assert_successful_change_in_error_response(response[7], input_name=f"201.{ip4_zone_name}", record_type="CNAME", ttl=300, record_data="test.com.") # ttl, domain name, data - assert_failed_change_in_error_response(response[6], input_name="$invalid.host.name.", record_type="CNAME", + assert_failed_change_in_error_response(response[8], input_name="$invalid.host.name.", record_type="CNAME", change_type="DeleteRecordSet", error_messages=['Invalid domain name: "$invalid.host.name.", valid domain names must be letters, numbers, ' 'underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[7], input_name="$another.invalid.host.name.", + assert_failed_change_in_error_response(response[9], input_name="$another.invalid.host.name.", record_type="CNAME", change_type="DeleteRecordSet", error_messages=['Invalid domain name: "$another.invalid.host.name.", valid domain names must be letters, numbers, ' 'underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[8], input_name="$another.invalid.host.name.", ttl=20, + assert_failed_change_in_error_response(response[10], input_name="$another.invalid.host.name.", ttl=20, record_type="CNAME", record_data="$another.invalid.cname.", error_messages=['Invalid TTL: "20", must be a number between 30 and 2147483647.', 'Invalid domain name: "$another.invalid.host.name.", valid domain names must be letters, numbers, ' @@ -2136,20 +2135,12 @@ def test_cname_recordtype_update_delete_checks(shared_zone_test_context): 'underscores, and hyphens, joined by dots, and terminated with a dot.']) # zone discovery failure - assert_failed_change_in_error_response(response[9], input_name="zone.discovery.error.", record_type="CNAME", + assert_failed_change_in_error_response(response[11], input_name="zone.discovery.error.", record_type="CNAME", change_type="DeleteRecordSet", error_messages=[ 'Zone Discovery Failed: zone for "zone.discovery.error." does not exist in VinylDNS. If zone exists, then it must be connected to in VinylDNS.']) # context validation failures: record does not exist, not authorized - assert_failed_change_in_error_response(response[10], input_name=f"non-existent-delete.{ok_zone_name}", record_type="CNAME", - change_type="DeleteRecordSet", - error_messages=[ - f'Record "non-existent-delete.{ok_zone_name}" Does Not Exist: cannot delete a record that does not exist.']) - assert_failed_change_in_error_response(response[11], input_name=f"non-existent-update.{ok_zone_name}", record_type="CNAME", - change_type="DeleteRecordSet", - error_messages=[ - f'Record "non-existent-update.{ok_zone_name}" Does Not Exist: cannot delete a record that does not exist.']) assert_successful_change_in_error_response(response[12], input_name=f"non-existent-update.{ok_zone_name}", record_type="CNAME", record_data="test.com.") assert_failed_change_in_error_response(response[13], input_name=f"delete-unauthorized3.{dummy_zone_name}", @@ -2383,6 +2374,8 @@ def test_ipv4_ptr_recordtype_update_delete_checks(shared_zone_test_context): get_change_PTR_json(f"{ip4_prefix}.25", change_type="DeleteRecordSet"), get_change_PTR_json(f"{ip4_prefix}.193", ttl=300, ptrdname="has-updated.ptr."), get_change_PTR_json(f"{ip4_prefix}.193", change_type="DeleteRecordSet"), + get_change_PTR_json(f"{ip4_prefix}.199", change_type="DeleteRecordSet"), + get_change_PTR_json(f"{ip4_prefix}.200", change_type="DeleteRecordSet"), # valid changes: delete and add of same record name but different type get_change_CNAME_json(f"21.{ip4_zone_name}", change_type="DeleteRecordSet"), @@ -2399,9 +2392,7 @@ def test_ipv4_ptr_recordtype_update_delete_checks(shared_zone_test_context): get_change_PTR_json("192.1.1.25", change_type="DeleteRecordSet"), # context validation failures - get_change_PTR_json(f"{ip4_prefix}.199", change_type="DeleteRecordSet"), get_change_PTR_json(f"{ip4_prefix}.200", ttl=300, ptrdname="has-updated.ptr."), - get_change_PTR_json(f"{ip4_prefix}.200", change_type="DeleteRecordSet"), ] } @@ -2422,25 +2413,29 @@ def test_ipv4_ptr_recordtype_update_delete_checks(shared_zone_test_context): record_data="has-updated.ptr.") assert_successful_change_in_error_response(response[2], input_name=f"{ip4_prefix}.193", record_type="PTR", record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[3], input_name=f"{ip4_prefix}.199", record_type="PTR", + record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"{ip4_prefix}.200", record_type="PTR", + record_data=None, change_type="DeleteRecordSet") # successful changes: add and delete of same record name but different type - assert_successful_change_in_error_response(response[3], input_name=f"21.{ip4_zone_name}", + assert_successful_change_in_error_response(response[5], input_name=f"21.{ip4_zone_name}", record_type="CNAME", record_data=None, change_type="DeleteRecordSet") - assert_successful_change_in_error_response(response[4], input_name=f"{ip4_prefix}.21", record_type="PTR", + assert_successful_change_in_error_response(response[6], input_name=f"{ip4_prefix}.21", record_type="PTR", record_data="replace-cname.ptr.") - assert_successful_change_in_error_response(response[5], input_name=f"17.{ip4_zone_name}", + assert_successful_change_in_error_response(response[7], input_name=f"17.{ip4_zone_name}", record_type="CNAME", record_data="replace-ptr.cname.") - assert_successful_change_in_error_response(response[6], input_name=f"{ip4_prefix}.17", record_type="PTR", + assert_successful_change_in_error_response(response[8], input_name=f"{ip4_prefix}.17", record_type="PTR", record_data=None, change_type="DeleteRecordSet") # input validations failures: invalid IP, ttl, and record data - assert_failed_change_in_error_response(response[7], input_name="1.1.1", record_type="PTR", record_data=None, + assert_failed_change_in_error_response(response[9], input_name="1.1.1", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=['Invalid IP address: "1.1.1".']) - assert_failed_change_in_error_response(response[8], input_name="192.0.2.", record_type="PTR", record_data=None, + assert_failed_change_in_error_response(response[10], input_name="192.0.2.", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=['Invalid IP address: "192.0.2.".']) - assert_failed_change_in_error_response(response[9], ttl=29, input_name="192.0.2.", record_type="PTR", + assert_failed_change_in_error_response(response[11], ttl=29, input_name="192.0.2.", record_type="PTR", record_data="failed-update$.ptr.", error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.', 'Invalid IP address: "192.0.2.".', @@ -2448,19 +2443,13 @@ def test_ipv4_ptr_recordtype_update_delete_checks(shared_zone_test_context): 'joined by dots, and terminated with a dot.']) # zone discovery failure - assert_failed_change_in_error_response(response[10], input_name="192.1.1.25", record_type="PTR", + assert_failed_change_in_error_response(response[12], input_name="192.1.1.25", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=["Zone Discovery Failed: zone for \"192.1.1.25\" does not exist in VinylDNS. If zone exists, " "then it must be connected to in VinylDNS."]) # context validation failures: record does not exist - assert_failed_change_in_error_response(response[11], input_name=f"{ip4_prefix}.199", record_type="PTR", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"{ip4_prefix}.199\" Does Not Exist: cannot delete a record that does not exist."]) - assert_successful_change_in_error_response(response[12], ttl=300, input_name=f"{ip4_prefix}.200", record_type="PTR", record_data="has-updated.ptr.") - assert_failed_change_in_error_response(response[13], input_name=f"{ip4_prefix}.200", record_type="PTR", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"{ip4_prefix}.200\" Does Not Exist: cannot delete a record that does not exist."]) + assert_successful_change_in_error_response(response[13], ttl=300, input_name=f"{ip4_prefix}.200", record_type="PTR", record_data="has-updated.ptr.") finally: clear_recordset_list(to_delete, ok_client) @@ -2555,6 +2544,8 @@ def test_ipv6_ptr_recordtype_update_delete_checks(shared_zone_test_context): get_change_PTR_json(f"{ip6_prefix}:1000::aaaa", change_type="DeleteRecordSet"), get_change_PTR_json(f"{ip6_prefix}:1000::62", ttl=300, ptrdname="has-updated.ptr."), get_change_PTR_json(f"{ip6_prefix}:1000::62", change_type="DeleteRecordSet"), + get_change_PTR_json(f"{ip6_prefix}:1000::60", change_type="DeleteRecordSet"), + get_change_PTR_json(f"{ip6_prefix}:1000::65", change_type="DeleteRecordSet"), # input validations failures get_change_PTR_json("fd69:27cc:fe91de::ab", change_type="DeleteRecordSet"), @@ -2565,9 +2556,7 @@ def test_ipv6_ptr_recordtype_update_delete_checks(shared_zone_test_context): get_change_PTR_json("fedc:ba98:7654::abc", change_type="DeleteRecordSet"), # context validation failures - get_change_PTR_json(f"{ip6_prefix}:1000::60", change_type="DeleteRecordSet"), get_change_PTR_json(f"{ip6_prefix}:1000::65", ttl=300, ptrdname="has-updated.ptr."), - get_change_PTR_json(f"{ip6_prefix}:1000::65", change_type="DeleteRecordSet") ] } @@ -2588,15 +2577,19 @@ def test_ipv6_ptr_recordtype_update_delete_checks(shared_zone_test_context): record_type="PTR", record_data="has-updated.ptr.") assert_successful_change_in_error_response(response[2], input_name=f"{ip6_prefix}:1000::62", record_type="PTR", record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[3], input_name=f"{ip6_prefix}:1000::60", record_type="PTR", + record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"{ip6_prefix}:1000::65", record_type="PTR", + record_data=None, change_type="DeleteRecordSet") # input validations failures: invalid IP, ttl, and record data - assert_failed_change_in_error_response(response[3], input_name="fd69:27cc:fe91de::ab", record_type="PTR", + assert_failed_change_in_error_response(response[5], input_name="fd69:27cc:fe91de::ab", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=['Invalid IP address: "fd69:27cc:fe91de::ab".']) - assert_failed_change_in_error_response(response[4], input_name="fd69:27cc:fe91de::ba", record_type="PTR", + assert_failed_change_in_error_response(response[6], input_name="fd69:27cc:fe91de::ba", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=['Invalid IP address: "fd69:27cc:fe91de::ba".']) - assert_failed_change_in_error_response(response[5], ttl=29, input_name="fd69:27cc:fe91de::ba", + assert_failed_change_in_error_response(response[7], ttl=29, input_name="fd69:27cc:fe91de::ba", record_type="PTR", record_data="failed-update$.ptr.", error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.', 'Invalid IP address: "fd69:27cc:fe91de::ba".', @@ -2604,20 +2597,14 @@ def test_ipv6_ptr_recordtype_update_delete_checks(shared_zone_test_context): 'and hyphens, joined by dots, and terminated with a dot.']) # zone discovery failure - assert_failed_change_in_error_response(response[6], input_name="fedc:ba98:7654::abc", record_type="PTR", + assert_failed_change_in_error_response(response[8], input_name="fedc:ba98:7654::abc", record_type="PTR", record_data=None, change_type="DeleteRecordSet", error_messages=["Zone Discovery Failed: zone for \"fedc:ba98:7654::abc\" does not exist in VinylDNS. " "If zone exists, then it must be connected to in VinylDNS."]) # context validation failures: record does not exist, failure on update with double add - assert_failed_change_in_error_response(response[7], input_name=f"{ip6_prefix}:1000::60", record_type="PTR", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"{ip6_prefix}:1000::60\" Does Not Exist: cannot delete a record that does not exist."]) - assert_successful_change_in_error_response(response[8], ttl=300, input_name=f"{ip6_prefix}:1000::65", + assert_successful_change_in_error_response(response[9], ttl=300, input_name=f"{ip6_prefix}:1000::65", record_type="PTR", record_data="has-updated.ptr.") - assert_failed_change_in_error_response(response[9], input_name=f"{ip6_prefix}:1000::65", record_type="PTR", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"{ip6_prefix}:1000::65\" Does Not Exist: cannot delete a record that does not exist."]) finally: clear_recordset_list(to_delete, ok_client) @@ -2744,6 +2731,8 @@ def test_txt_recordtype_update_delete_checks(shared_zone_test_context): get_change_TXT_json(rs_delete_fqdn, change_type="DeleteRecordSet"), get_change_TXT_json(rs_update_fqdn, change_type="DeleteRecordSet"), get_change_TXT_json(rs_update_fqdn, ttl=300), + get_change_TXT_json(f"delete-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), + get_change_TXT_json(f"update-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), # input validations failures get_change_TXT_json(f"invalid-name$.{ok_zone_name}", change_type="DeleteRecordSet"), @@ -2753,8 +2742,6 @@ def test_txt_recordtype_update_delete_checks(shared_zone_test_context): get_change_TXT_json("no.zone.at.all.", change_type="DeleteRecordSet"), # context validation failures - get_change_TXT_json(f"delete-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), - get_change_TXT_json(f"update-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_TXT_json(f"update-nonexistent.{ok_zone_name}", text="test"), get_change_TXT_json(rs_delete_dummy_fqdn, change_type="DeleteRecordSet"), get_change_TXT_json(rs_update_dummy_fqdn, text="test"), @@ -2784,25 +2771,23 @@ def test_txt_recordtype_update_delete_checks(shared_zone_test_context): assert_successful_change_in_error_response(response[0], input_name=rs_delete_fqdn, record_type="TXT", record_data=None, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[1], input_name=rs_update_fqdn, record_type="TXT", record_data=None, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[2], ttl=300, input_name=rs_update_fqdn, record_type="TXT", record_data="test") + assert_successful_change_in_error_response(response[3], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="TXT", record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"update-nonexistent.{ok_zone_name}", record_type="TXT", record_data=None, change_type="DeleteRecordSet") # input validations failures: invalid input name, reverse zone error, invalid ttl - assert_failed_change_in_error_response(response[3], input_name=f"invalid-name$.{ok_zone_name}", record_type="TXT", record_data="test", change_type="DeleteRecordSet", + assert_failed_change_in_error_response(response[5], input_name=f"invalid-name$.{ok_zone_name}", record_type="TXT", record_data="test", change_type="DeleteRecordSet", error_messages=[f'Invalid domain name: "invalid-name$.{ok_zone_name}", valid domain names must be ' f'letters, numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[4], input_name=f"invalid-ttl.{ok_zone_name}", ttl=29, record_type="TXT", record_data="bad-ttl", + assert_failed_change_in_error_response(response[6], input_name=f"invalid-ttl.{ok_zone_name}", ttl=29, record_type="TXT", record_data="bad-ttl", error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.']) # zone discovery failure - assert_failed_change_in_error_response(response[5], input_name="no.zone.at.all.", record_type="TXT", record_data=None, change_type="DeleteRecordSet", + assert_failed_change_in_error_response(response[7], input_name="no.zone.at.all.", record_type="TXT", record_data=None, change_type="DeleteRecordSet", error_messages=[ "Zone Discovery Failed: zone for \"no.zone.at.all.\" does not exist in VinylDNS. " "If zone exists, then it must be connected to in VinylDNS."]) # context validation failures: record does not exist, not authorized - assert_failed_change_in_error_response(response[6], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="TXT", record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"delete-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) - assert_failed_change_in_error_response(response[7], input_name=f"update-nonexistent.{ok_zone_name}", record_type="TXT", record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"update-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) assert_successful_change_in_error_response(response[8], input_name=f"update-nonexistent.{ok_zone_name}", record_type="TXT", record_data="test") assert_failed_change_in_error_response(response[9], input_name=rs_delete_dummy_fqdn, record_type="TXT", record_data=None, change_type="DeleteRecordSet", error_messages=[f"User \"ok\" is not authorized. Contact zone owner group: {dummy_group_name} at test@test.com to make DNS changes."]) @@ -2953,6 +2938,8 @@ def test_mx_recordtype_update_delete_checks(shared_zone_test_context): get_change_MX_json(rs_delete_fqdn, change_type="DeleteRecordSet"), get_change_MX_json(rs_update_fqdn, change_type="DeleteRecordSet"), get_change_MX_json(rs_update_fqdn, ttl=300), + get_change_MX_json(f"delete-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), + get_change_MX_json(f"update-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), # input validations failures get_change_MX_json(f"invalid-name$.{ok_zone_name}", change_type="DeleteRecordSet"), @@ -2964,8 +2951,6 @@ def test_mx_recordtype_update_delete_checks(shared_zone_test_context): get_change_MX_json("no.zone.at.all.", change_type="DeleteRecordSet"), # context validation failures - get_change_MX_json(f"delete-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), - get_change_MX_json(f"update-nonexistent.{ok_zone_name}", change_type="DeleteRecordSet"), get_change_MX_json(f"update-nonexistent.{ok_zone_name}", preference=1000, exchange="foo.bar."), get_change_MX_json(rs_delete_dummy_fqdn, change_type="DeleteRecordSet"), get_change_MX_json(rs_update_dummy_fqdn, preference=1000, exchange="foo.bar."), @@ -2995,37 +2980,35 @@ def test_mx_recordtype_update_delete_checks(shared_zone_test_context): assert_successful_change_in_error_response(response[0], input_name=rs_delete_fqdn, record_type="MX", record_data=None, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[1], input_name=rs_update_fqdn, record_type="MX", record_data=None, change_type="DeleteRecordSet") assert_successful_change_in_error_response(response[2], ttl=300, input_name=rs_update_fqdn, record_type="MX", record_data={"preference": 1, "exchange": "foo.bar."}) + assert_successful_change_in_error_response(response[3], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="MX", + record_data=None, change_type="DeleteRecordSet") + assert_successful_change_in_error_response(response[4], input_name=f"update-nonexistent.{ok_zone_name}", record_type="MX", + record_data=None, change_type="DeleteRecordSet") # input validations failures: invalid input name, reverse zone error, invalid ttl - assert_failed_change_in_error_response(response[3], input_name=f"invalid-name$.{ok_zone_name}", record_type="MX", record_data={"preference": 1, "exchange": "foo.bar."}, + assert_failed_change_in_error_response(response[5], input_name=f"invalid-name$.{ok_zone_name}", record_type="MX", record_data={"preference": 1, "exchange": "foo.bar."}, change_type="DeleteRecordSet", error_messages=[f'Invalid domain name: "invalid-name$.{ok_zone_name}", valid domain names must be letters, ' f'numbers, underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[4], input_name=f"delete.{ok_zone_name}", ttl=29, record_type="MX", + assert_failed_change_in_error_response(response[6], input_name=f"delete.{ok_zone_name}", ttl=29, record_type="MX", record_data={"preference": 1, "exchange": "foo.bar."}, error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.']) - assert_failed_change_in_error_response(response[5], input_name=f"bad-exchange.{ok_zone_name}", record_type="MX", + assert_failed_change_in_error_response(response[7], input_name=f"bad-exchange.{ok_zone_name}", record_type="MX", record_data={"preference": 1, "exchange": "foo$.bar."}, error_messages=['Invalid domain name: "foo$.bar.", valid domain names must be letters, numbers, ' 'underscores, and hyphens, joined by dots, and terminated with a dot.']) - assert_failed_change_in_error_response(response[6], input_name=f"mx.{ip4_zone_name}", record_type="MX", + assert_failed_change_in_error_response(response[8], input_name=f"mx.{ip4_zone_name}", record_type="MX", record_data={"preference": 1, "exchange": "foo.bar."}, error_messages=[f'Invalid Record Type In Reverse Zone: record with name "mx.{ip4_zone_name}" ' f'and type "MX" is not allowed in a reverse zone.']) # zone discovery failure - assert_failed_change_in_error_response(response[7], input_name="no.zone.at.all.", record_type="MX", + assert_failed_change_in_error_response(response[9], input_name="no.zone.at.all.", record_type="MX", record_data=None, change_type="DeleteRecordSet", error_messages=["Zone Discovery Failed: zone for \"no.zone.at.all.\" does not exist in VinylDNS. " "If zone exists, then it must be connected to in VinylDNS."]) # context validation failures: record does not exist, not authorized - assert_failed_change_in_error_response(response[8], input_name=f"delete-nonexistent.{ok_zone_name}", record_type="MX", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"delete-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) - assert_failed_change_in_error_response(response[9], input_name=f"update-nonexistent.{ok_zone_name}", record_type="MX", - record_data=None, change_type="DeleteRecordSet", - error_messages=[f"Record \"update-nonexistent.{ok_zone_name}\" Does Not Exist: cannot delete a record that does not exist."]) assert_successful_change_in_error_response(response[10], input_name=f"update-nonexistent.{ok_zone_name}", record_type="MX", record_data={"preference": 1000, "exchange": "foo.bar."}) assert_failed_change_in_error_response(response[11], input_name=rs_delete_dummy_fqdn, record_type="MX", @@ -3734,39 +3717,27 @@ def test_create_batch_with_zone_name_requiring_manual_review(shared_zone_test_co rejecter.reject_batch_change(response["id"], status=200) -def test_create_batch_delete_record_for_invalid_record_data_fails(shared_zone_test_context): +def test_create_batch_delete_record_that_does_not_exists_completes(shared_zone_test_context): """ - Test delete record set fails for non-existent record and non-existent record data + Test delete record set completes for non-existent record """ client = shared_zone_test_context.ok_vinyldns_client ok_zone_name = shared_zone_test_context.ok_zone["name"] - a_delete_name = generate_record_name() - a_delete_fqdn = a_delete_name + f".{ok_zone_name}" - a_delete = create_recordset(shared_zone_test_context.ok_zone, a_delete_fqdn, "A", [{"address": "1.1.1.1"}]) - batch_change_input = { "comments": "test delete record failures", "changes": [ - get_change_A_AAAA_json(f"delete-non-existent-record.{ok_zone_name}", change_type="DeleteRecordSet"), - get_change_A_AAAA_json(a_delete_fqdn, address="4.5.6.7", change_type="DeleteRecordSet") + get_change_A_AAAA_json(f"delete-non-existent-record.{ok_zone_name}", change_type="DeleteRecordSet") ] } - to_delete = [] + response = client.create_batch_change(batch_change_input, status=202) + get_batch = client.get_batch_change(response["id"]) - try: - create_rs = client.create_recordset(a_delete, status=202) - to_delete.append(client.wait_until_recordset_change_status(create_rs, "Complete")) + assert_that(get_batch["changes"][0]["systemMessage"], is_("This record does not exist." + + "No further action is required.")) - errors = client.create_batch_change(batch_change_input, status=400) - - assert_failed_change_in_error_response(errors[0], input_name=f"delete-non-existent-record.{ok_zone_name}", record_data="1.1.1.1", change_type="DeleteRecordSet", - error_messages=[f'Record "delete-non-existent-record.{ok_zone_name}" Does Not Exist: cannot delete a record that does not exist.']) - assert_failed_change_in_error_response(errors[1], input_name=a_delete_fqdn, record_data="4.5.6.7", change_type="DeleteRecordSet", - error_messages=["Record data 4.5.6.7 does not exist for \"" + a_delete_fqdn + "\"."]) - finally: - clear_recordset_list(to_delete, client) + assert_successful_change_in_error_response(response["changes"][0], input_name=f"delete-non-existent-record.{ok_zone_name}", record_data="1.1.1.1", change_type="DeleteRecordSet") @pytest.mark.serial diff --git a/modules/api/src/test/functional/tests/membership/get_group_changes_test.py b/modules/api/src/test/functional/tests/membership/get_group_changes_test.py index 7c9f4ac5a..fda94f4c6 100644 --- a/modules/api/src/test/functional/tests/membership/get_group_changes_test.py +++ b/modules/api/src/test/functional/tests/membership/get_group_changes_test.py @@ -26,16 +26,12 @@ def test_list_group_activity_start_from_success(group_activity_context, shared_z # we grab 3 items, which when sorted by most recent will give the 3 most recent items page_one = client.get_group_changes(created_group["id"], max_items=3, status=200) - # our start from will align with the created on the 3rd change in the list - start_from_index = 2 - start_from = page_one["changes"][start_from_index]["created"] # start from a known good timestamp - # now, we say give me all changes since the start_from, which should yield 8-7-6-5-4 - result = client.get_group_changes(created_group["id"], start_from=start_from, max_items=5, status=200) + result = client.get_group_changes(created_group["id"], start_from=page_one["nextId"], max_items=5, status=200) assert_that(result["changes"], has_length(5)) assert_that(result["maxItems"], is_(5)) - assert_that(result["startFrom"], is_(start_from)) + assert_that(result["startFrom"], is_(page_one["nextId"])) assert_that(result["nextId"], is_not(none())) # we should have, in order, changes 8 7 6 5 4 diff --git a/modules/api/src/test/functional/tests/membership/list_my_groups_test.py b/modules/api/src/test/functional/tests/membership/list_my_groups_test.py index cfcf557e8..516cdc76a 100644 --- a/modules/api/src/test/functional/tests/membership/list_my_groups_test.py +++ b/modules/api/src/test/functional/tests/membership/list_my_groups_test.py @@ -13,20 +13,11 @@ def test_list_my_groups_no_parameters(list_my_groups_context): Test that we can get all the groups where a user is a member """ results = list_my_groups_context.client.list_my_groups(status=200) - assert_that(results, has_length(3)) # 3 fields - - # Only count the groups with the group prefix - groups = [x for x in results["groups"] if x["name"].startswith(list_my_groups_context.group_prefix)] - assert_that(groups, has_length(50)) + assert_that(results, has_length(4)) # 4 fields assert_that(results, is_not(has_key("groupNameFilter"))) assert_that(results, is_not(has_key("startFrom"))) - assert_that(results, is_not(has_key("nextId"))) - assert_that(results["maxItems"], is_(200)) - - results["groups"] = sorted(groups, key=lambda x: x["name"]) - - for i in range(0, 50): - assert_that(results["groups"][i]["name"], is_("{0}-{1:0>3}".format(list_my_groups_context.group_prefix, i))) + assert_that(results, is_(has_key("nextId"))) + assert_that(results["maxItems"], is_(100)) def test_get_my_groups_using_old_account_auth(list_my_groups_context): @@ -34,11 +25,11 @@ def test_get_my_groups_using_old_account_auth(list_my_groups_context): Test passing in an account will return an empty set """ results = list_my_groups_context.client.list_my_groups(status=200) - assert_that(results, has_length(3)) + assert_that(results, has_length(4)) assert_that(results, is_not(has_key("groupNameFilter"))) assert_that(results, is_not(has_key("startFrom"))) - assert_that(results, is_not(has_key("nextId"))) - assert_that(results["maxItems"], is_(200)) + assert_that(results, is_(has_key("nextId"))) + assert_that(results["maxItems"], is_(100)) def test_list_my_groups_max_items(list_my_groups_context): @@ -102,7 +93,7 @@ def test_list_my_groups_filter_matches(list_my_groups_context): assert_that(results["groupNameFilter"], is_(f"{list_my_groups_context.group_prefix}-01")) assert_that(results, is_not(has_key("startFrom"))) assert_that(results, is_not(has_key("nextId"))) - assert_that(results["maxItems"], is_(200)) + assert_that(results["maxItems"], is_(100)) results["groups"] = sorted(results["groups"], key=lambda x: x["name"]) @@ -133,28 +124,20 @@ def test_list_my_groups_with_ignore_access_true(list_my_groups_context): Test that we can get all the groups whether a user is a member or not """ results = list_my_groups_context.client.list_my_groups(ignore_access=True, status=200) - - # Only count the groups with the group prefix + assert_that(results, has_length(4)) # 4 fields assert_that(len(results["groups"]), greater_than(50)) - assert_that(results["maxItems"], is_(200)) + assert_that(results["maxItems"], is_(100)) assert_that(results["ignoreAccess"], is_(True)) - my_results = list_my_groups_context.client.list_my_groups(status=200) - my_groups = [x for x in my_results["groups"] if x["name"].startswith(list_my_groups_context.group_prefix)] - sorted_groups = sorted(my_groups, key=lambda x: x["name"]) - - for i in range(0, 50): - assert_that(sorted_groups[i]["name"], is_("{0}-{1:0>3}".format(list_my_groups_context.group_prefix, i))) - def test_list_my_groups_as_support_user(list_my_groups_context): """ Test that we can get all the groups as a support user, even without ignore_access """ results = list_my_groups_context.support_user_client.list_my_groups(status=200) - + assert_that(results, has_length(4)) # 4 fields assert_that(len(results["groups"]), greater_than(50)) - assert_that(results["maxItems"], is_(200)) + assert_that(results["maxItems"], is_(100)) assert_that(results["ignoreAccess"], is_(False)) @@ -163,7 +146,7 @@ def test_list_my_groups_as_support_user_with_ignore_access_true(list_my_groups_c Test that we can get all the groups as a support user """ results = list_my_groups_context.support_user_client.list_my_groups(ignore_access=True, status=200) - + assert_that(results, has_length(4)) # 4 fields assert_that(len(results["groups"]), greater_than(50)) - assert_that(results["maxItems"], is_(200)) + assert_that(results["maxItems"], is_(100)) assert_that(results["ignoreAccess"], is_(True)) diff --git a/modules/api/src/test/functional/vinyldns_python.py b/modules/api/src/test/functional/vinyldns_python.py index c6e29e135..87c7f8866 100644 --- a/modules/api/src/test/functional/vinyldns_python.py +++ b/modules/api/src/test/functional/vinyldns_python.py @@ -220,7 +220,7 @@ class VinylDNSClient(object): return data - def list_my_groups(self, group_name_filter=None, start_from=None, max_items=200, ignore_access=False, **kwargs): + def list_my_groups(self, group_name_filter=None, start_from=None, max_items=100, ignore_access=False, **kwargs): """ Retrieves my groups :param start_from: the start key of the page diff --git a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeConverterSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeConverterSpec.scala index 0baefd576..7087fdb32 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeConverterSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeConverterSpec.scala @@ -37,6 +37,8 @@ import vinyldns.core.domain.record._ import vinyldns.core.domain.zone.Zone class BatchChangeConverterSpec extends AnyWordSpec with Matchers with CatsHelpers { + private val notExistCompletedMessage: String = "This record does not exist." + + "No further action is required." private def makeSingleAddChange( name: String, @@ -160,6 +162,14 @@ class BatchChangeConverterSpec extends AnyWordSpec with Matchers with CatsHelper makeAddChangeForValidation("mxToUpdate", MXData(1, Fqdn("update.com.")), MX) ) + private val singleChangesOneDelete = List( + makeSingleDeleteRRSetChange("DoesNotExistToDelete", A) + ) + + private val changeForValidationOneDelete = List( + makeDeleteRRSetChangeForValidation("DoesNotExistToDelete", A) + ) + private val singleChangesOneBad = List( makeSingleAddChange("one", AData("1.1.1.1")), makeSingleAddChange("two", AData("1.1.1.2")), @@ -535,6 +545,42 @@ class BatchChangeConverterSpec extends AnyWordSpec with Matchers with CatsHelper savedBatch shouldBe Some(returnedBatch) } + "set status to complete when deleting a record that does not exist" in { + val batchWithBadChange = + BatchChange( + okUser.id, + okUser.userName, + None, + DateTime.now, + singleChangesOneDelete, + approvalStatus = BatchChangeApprovalStatus.AutoApproved + ) + val result = rightResultOf( + underTest + .sendBatchForProcessing( + batchWithBadChange, + existingZones, + ChangeForValidationMap(changeForValidationOneDelete.map(_.validNel), existingRecordSets), + None + ) + .value + ) + + val returnedBatch = result.batchChange + + // validate completed status returned + val receivedChange = returnedBatch.changes(0) + receivedChange.status shouldBe SingleChangeStatus.Complete + receivedChange.recordChangeId shouldBe None + receivedChange.systemMessage shouldBe Some(notExistCompletedMessage) + returnedBatch.changes(0) shouldBe singleChangesOneDelete(0).copy(systemMessage = Some(notExistCompletedMessage), status = SingleChangeStatus.Complete) + + // check the update has been made in the DB + val savedBatch: Option[BatchChange] = + await(batchChangeRepo.getBatchChange(batchWithBadChange.id)) + savedBatch shouldBe Some(returnedBatch) + } + "return error if an unsupported record is received" in { val batchChangeUnsupported = BatchChange( 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 d1a227ad8..6ecf64778 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 @@ -1004,7 +1004,7 @@ class BatchChangeValidationsSpec ) } - property("validateChangesWithContext: should fail for update if record does not exist") { + property("validateChangesWithContext: should complete for update if record does not exist") { val deleteRRSet = makeDeleteUpdateDeleteRRSet("deleteRRSet") val deleteRecord = makeDeleteUpdateDeleteRRSet("deleteRecord", Some(AData("1.1.1.1"))) val deleteNonExistentEntry = makeDeleteUpdateDeleteRRSet("ok", Some(AData("1.1.1.1"))) @@ -1026,15 +1026,8 @@ class BatchChangeValidationsSpec ) result(0) shouldBe valid - result(1) should haveInvalid[DomainValidationError]( - RecordDoesNotExist(deleteRRSet.inputChange.inputName) - ) - result(3) should haveInvalid[DomainValidationError]( - RecordDoesNotExist(deleteRecord.inputChange.inputName) - ) - result(3) should haveInvalid[DomainValidationError]( - RecordDoesNotExist(deleteRecord.inputChange.inputName) - ) + result(1) shouldBe valid + result(3) shouldBe valid result(4) shouldBe valid deleteNonExistentEntry.inputChange.record.foreach { record => result(5) should haveInvalid[DomainValidationError]( @@ -1589,7 +1582,7 @@ class BatchChangeValidationsSpec } property( - """validateChangesWithContext: should fail DeleteChangeForValidation with RecordDoesNotExist + """validateChangesWithContext: should complete DeleteChangeForValidation |if record does not exist""".stripMargin ) { val deleteRRSet = makeDeleteUpdateDeleteRRSet("record-does-not-exist") @@ -1606,12 +1599,8 @@ class BatchChangeValidationsSpec None ) - result(0) should haveInvalid[DomainValidationError]( - RecordDoesNotExist(deleteRRSet.inputChange.inputName) - ) - result(1) should haveInvalid[DomainValidationError]( - RecordDoesNotExist(deleteRecord.inputChange.inputName) - ) + result(0) shouldBe valid + result(1) shouldBe valid } property("""validateChangesWithContext: should succeed for DeleteChangeForValidation diff --git a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala index 44d178c18..923f89959 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipServiceSpec.scala @@ -783,6 +783,59 @@ class MembershipServiceSpec } } + "getGroupChange" should { + "return the single group change" in { + val groupChangeRepoResponse = listOfDummyGroupChanges.take(1).head + doReturn(IO.pure(Option(groupChangeRepoResponse))) + .when(mockGroupChangeRepo) + .getGroupChange(anyString) + + doReturn(IO.pure(ListUsersResults(Seq(dummyUser), Some("1")))) + .when(mockUserRepo) + .getUsers(any[Set[String]], any[Option[String]], any[Option[Int]]) + + val userMap = Seq(dummyUser).map(u => (u.id, u.userName)).toMap + val expected: GroupChangeInfo = + listOfDummyGroupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))).take(1).head + + val result: GroupChangeInfo = + rightResultOf(underTest.getGroupChange(dummyGroup.id, dummyAuth).value) + result shouldBe expected + } + + "return the single group change even if the user is not authorized" in { + val groupChangeRepoResponse = listOfDummyGroupChanges.take(1).head + doReturn(IO.pure(Some(groupChangeRepoResponse))) + .when(mockGroupChangeRepo) + .getGroupChange(anyString) + + doReturn(IO.pure(ListUsersResults(Seq(dummyUser), Some("1")))) + .when(mockUserRepo) + .getUsers(any[Set[String]], any[Option[String]], any[Option[Int]]) + + val userMap = Seq(dummyUser).map(u => (u.id, u.userName)).toMap + val expected: GroupChangeInfo = + listOfDummyGroupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))).take(1).head + + val result: GroupChangeInfo = + rightResultOf(underTest.getGroupChange(dummyGroup.id, okAuth).value) + result shouldBe expected + } + + "return a InvalidGroupRequestError if the group change id is not valid" in { + doReturn(IO.pure(None)) + .when(mockGroupChangeRepo) + .getGroupChange(anyString) + + doReturn(IO.pure(ListUsersResults(Seq(dummyUser), Some("1")))) + .when(mockUserRepo) + .getUsers(any[Set[String]], any[Option[String]], any[Option[Int]]) + + val result = leftResultOf(underTest.getGroupChange(dummyGroup.id, okAuth).value) + result shouldBe a[InvalidGroupRequestError] + } + } + "getGroupActivity" should { "return the group activity" in { val groupChangeRepoResponse = ListGroupChangesResults( @@ -793,8 +846,13 @@ class MembershipServiceSpec .when(mockGroupChangeRepo) .getGroupChanges(anyString, any[Option[String]], anyInt) + doReturn(IO.pure(ListUsersResults(Seq(dummyUser), Some("1")))) + .when(mockUserRepo) + .getUsers(any[Set[String]], any[Option[String]], any[Option[Int]]) + + val userMap = Seq(dummyUser).map(u => (u.id, u.userName)).toMap val expected: List[GroupChangeInfo] = - listOfDummyGroupChanges.map(GroupChangeInfo.apply).take(100) + listOfDummyGroupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))).take(100) val result: ListGroupChangesResponse = rightResultOf(underTest.getGroupActivity(dummyGroup.id, None, 100, dummyAuth).value) @@ -813,8 +871,13 @@ class MembershipServiceSpec .when(mockGroupChangeRepo) .getGroupChanges(anyString, any[Option[String]], anyInt) + doReturn(IO.pure(ListUsersResults(Seq(dummyUser), Some("1")))) + .when(mockUserRepo) + .getUsers(any[Set[String]], any[Option[String]], any[Option[Int]]) + + val userMap = Seq(dummyUser).map(u => (u.id, u.userName)).toMap val expected: List[GroupChangeInfo] = - listOfDummyGroupChanges.map(GroupChangeInfo.apply).take(100) + listOfDummyGroupChanges.map(change => GroupChangeInfo.apply(change.copy(userName = userMap.get(change.userId)))).take(100) val result: ListGroupChangesResponse = rightResultOf(underTest.getGroupActivity(dummyGroup.id, None, 100, okAuth).value) @@ -825,6 +888,19 @@ class MembershipServiceSpec } } + "determine group difference" should { + "return difference between two groups" in { + val groupChange = Seq(okGroupChange, dummyGroupChangeUpdate, okGroupChange.copy(changeType = GroupChangeType.Delete)) + val result: Seq[String] = rightResultOf(underTest.determineGroupDifference(groupChange).value) + // Newly created group's change message + result(0) shouldBe "Group Created." + // Updated group's change message + result(1) shouldBe "Group name changed to 'dummy-group'. Group email changed to 'dummy@test.com'. Group description changed to 'dummy group'. Group admin/s with userId/s (12345-abcde-6789,56789-edcba-1234) added. Group admin/s with userId/s (ok) removed. Group member/s with userId/s (12345-abcde-6789,56789-edcba-1234) added. Group member/s with userId/s (ok) removed." + // Deleted group's change message + result(2) shouldBe "Group Deleted." + } + } + "listAdmins" should { "return a list of admins" in { val testGroup = diff --git a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipValidationsSpec.scala index 84e7e842c..c59eb2fa0 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/membership/MembershipValidationsSpec.scala @@ -96,5 +96,16 @@ class MembershipValidationsSpec } } + + "isGroupChangePresent" should { + "return true when there is a group change present for the requested group change id" in { + isGroupChangePresent(Some(okGroupChange)) should be(right) + } + "return an error when there is a group change present for the requested group change id" in { + val error = leftValue(isGroupChangePresent(None)) + error shouldBe an[InvalidGroupRequestError] + } + + } } } diff --git a/modules/api/src/test/scala/vinyldns/api/route/MembershipRoutingSpec.scala b/modules/api/src/test/scala/vinyldns/api/route/MembershipRoutingSpec.scala index 6f6298c75..22a8d15f1 100644 --- a/modules/api/src/test/scala/vinyldns/api/route/MembershipRoutingSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/route/MembershipRoutingSpec.scala @@ -730,6 +730,37 @@ class MembershipRoutingSpec } } + "GET group change" should { + "return a 200 response with the group change when found" in { + val grpChange = GroupChangeInfo(okGroupChange) + doReturn(result(grpChange)).when(membershipService).getGroupChange("ok", okAuth) + Get("/groups/change/ok") ~> Route.seal(membershipRoute) ~> check { + status shouldBe StatusCodes.OK + + val result = responseAs[GroupChangeInfo] + result shouldBe grpChange + } + } + + "return a 400 Bad Request when the group change id is not valid" in { + doReturn(result(InvalidGroupRequestError("Invalid Group Change ID"))) + .when(membershipService) + .getGroupChange("notValid", okAuth) + Get("/groups/change/notValid") ~> Route.seal(membershipRoute) ~> check { + status shouldBe StatusCodes.BadRequest + } + } + + "return a 500 response on failure" in { + doReturn(result(new RuntimeException("fail"))) + .when(membershipService) + .getGroupChange("bad", okAuth) + Get(s"/groups/change/bad") ~> Route.seal(membershipRoute) ~> check { + status shouldBe StatusCodes.InternalServerError + } + } + } + "PUT update user lock status" should { "return a 200 response with the user locked" in { membershipRoute = superUserRoute diff --git a/modules/core/src/main/scala/vinyldns/core/domain/batch/SingleChange.scala b/modules/core/src/main/scala/vinyldns/core/domain/batch/SingleChange.scala index b0c0c95f5..5483820a4 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/batch/SingleChange.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/batch/SingleChange.scala @@ -46,6 +46,13 @@ sealed trait SingleChange { delete.copy(status = SingleChangeStatus.Failed, systemMessage = Some(error)) } + def withDoesNotExistMessage(error: String): SingleChange = this match { + case add: SingleAddChange => + add.copy(status = SingleChangeStatus.Failed, systemMessage = Some(error)) + case delete: SingleDeleteRRSetChange => + delete.copy(status = SingleChangeStatus.Complete, systemMessage = Some(error)) + } + def withProcessingError(message: Option[String], failedRecordChangeId: String): SingleChange = this match { case add: SingleAddChange => diff --git a/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChange.scala b/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChange.scala index b48ad6cc0..61e7c715a 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChange.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChange.scala @@ -34,7 +34,9 @@ case class GroupChange( userId: String, oldGroup: Option[Group] = None, id: String = UUID.randomUUID().toString, - created: DateTime = DateTime.now + created: DateTime = DateTime.now, + userName: Option[String] = None, + groupChangeMessage: Option[String] = None ) object GroupChange { diff --git a/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChangeRepository.scala b/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChangeRepository.scala index 846522b78..6a99302e6 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChangeRepository.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/membership/GroupChangeRepository.scala @@ -24,7 +24,8 @@ import vinyldns.core.repository.Repository trait GroupChangeRepository extends Repository { def save(db: DB, groupChange: GroupChange): IO[GroupChange] - def getGroupChange(groupChangeId: String): IO[Option[GroupChange]] // For testing + def getGroupChange(groupChangeId: String): IO[Option[GroupChange]] + def getGroupChanges( groupId: String, startFrom: Option[String], diff --git a/modules/core/src/test/scala/vinyldns/core/TestMembershipData.scala b/modules/core/src/test/scala/vinyldns/core/TestMembershipData.scala index 36ba54ec0..4f9480ed5 100644 --- a/modules/core/src/test/scala/vinyldns/core/TestMembershipData.scala +++ b/modules/core/src/test/scala/vinyldns/core/TestMembershipData.scala @@ -157,4 +157,13 @@ object TestMembershipData { id = s"$i" ) } + val dummyGroupChangeUpdate: GroupChange = GroupChange( + okGroup.copy(name = "dummy-group", email = "dummy@test.com", description = Some("dummy group"), + memberIds = Set(dummyUser.copy(id="12345-abcde-6789").id, superUser.copy(id="56789-edcba-1234").id), + adminUserIds = Set(dummyUser.copy(id="12345-abcde-6789").id, superUser.copy(id="56789-edcba-1234").id)), + GroupChangeType.Update, + okUser.id, + Some(okGroup), + created = DateTime.now.secondOfDay().roundFloorCopy() + ) } diff --git a/modules/portal/app/controllers/VinylDNS.scala b/modules/portal/app/controllers/VinylDNS.scala index 8e8908252..f6d3852e9 100644 --- a/modules/portal/app/controllers/VinylDNS.scala +++ b/modules/portal/app/controllers/VinylDNS.scala @@ -214,6 +214,32 @@ class VinylDNS @Inject() ( }) } + def getGroupChange(gcid: String): Action[AnyContent] = userAction.async { implicit request => + val vinyldnsRequest = VinylDNSRequest("GET", s"$vinyldnsServiceBackend", s"groups/change/$gcid") + executeRequest(vinyldnsRequest, request.user).map(response => { + logger.info(s"group change [$gcid] retrieved with status [${response.status}]") + Status(response.status)(response.body) + .withHeaders(cacheHeaders: _*) + }) + } + + def listGroupChanges(id: String): Action[AnyContent] = userAction.async { implicit request => + val queryParameters = new HashMap[String, java.util.List[String]]() + for { + (name, values) <- request.queryString + } queryParameters.put(name, values.asJava) + val vinyldnsRequest = new VinylDNSRequest( + "GET", + s"$vinyldnsServiceBackend", + s"groups/$id/activity", + parameters = queryParameters + ) + executeRequest(vinyldnsRequest, request.user).map(response => { + Status(response.status)(response.body) + .withHeaders(cacheHeaders: _*) + }) + } + def getUser(id: String): Action[AnyContent] = userAction.async { implicit request => val vinyldnsRequest = VinylDNSRequest("GET", s"$vinyldnsServiceBackend", s"users/$id") executeRequest(vinyldnsRequest, request.user).map(response => { @@ -431,6 +457,23 @@ class VinylDNS @Inject() ( }) } + def getZoneChange(id: String): Action[AnyContent] = userAction.async { implicit request => + val queryParameters = new HashMap[String, java.util.List[String]]() + for { + (name, values) <- request.queryString + } queryParameters.put(name, values.asJava) + val vinyldnsRequest = + new VinylDNSRequest( + "GET", + s"$vinyldnsServiceBackend", + s"zones/$id/changes", + parameters = queryParameters) + executeRequest(vinyldnsRequest, request.user).map(response => { + Status(response.status)(response.body) + .withHeaders(cacheHeaders: _*) + }) + } + def syncZone(id: String): Action[AnyContent] = userAction.async { implicit request => // $COVERAGE-OFF$ val vinyldnsRequest = diff --git a/modules/portal/app/views/groups/groupDetail.scala.html b/modules/portal/app/views/groups/groupDetail.scala.html index e7c3862d8..62717c8d2 100644 --- a/modules/portal/app/views/groups/groupDetail.scala.html +++ b/modules/portal/app/views/groups/groupDetail.scala.html @@ -2,110 +2,281 @@ @content = { - -
Description: {{membership.group.description}}
-Group Email: {{membership.group.email}}
- -