fix(target): Correct accounting of updated rows when deleting address (#2799)

If an update call was made to a target that removed an address, for a
target that already did not have an address, the incorrect number of
rows updated was being reported. During the update the target's version
was correctly being updated, resulting in one row being updated.
However then the delete call would result in zero rows being deleted,
since there was no address associated with the target, and this would
replace the value for the returned rowsUpdated. This resulted in the
target service thinking that no update happened, and it would report an
error.

internal/target/repository.go

@@ -615,13 +615,17 @@ func (r *Repository) UpdateTarget(ctx context.Context, target Target, version ui
 				var err error
 				address = allocTargetAddress()
 				address.TargetAddress.TargetId = t.GetPublicId()
-				rowsUpdated, err = w.Delete(ctx, address, db.WithOplog(oplogWrapper, address.oplog(oplog.OpType_OP_TYPE_DELETE)))
+				rowsDeleted, err := w.Delete(ctx, address, db.WithOplog(oplogWrapper, address.oplog(oplog.OpType_OP_TYPE_DELETE)))
 				if err != nil {
 					return errors.Wrap(ctx, err, op, errors.WithMsg("unable to delete target address"))
 				}
-				if rowsUpdated > 1 {
+				if rowsDeleted > 1 {
 					return errors.New(ctx, errors.MultipleRecords, op, "more than 1 target resource would have been deleted")
 				}
+				// If the only update was deleting an address, consider this as one "row" being updated.
+				if rowsUpdated == 0 && rowsDeleted == 1 {
+					rowsUpdated = 1
+				}
 			default:
 				address, err = fetchAddress(ctx, read, t.GetPublicId())
 				if err != nil && !errors.IsNotFoundError(err) {

internal/tests/api/targets/target_test.go

@@ -546,6 +546,45 @@ func TestCreateTarget_DirectlyAttachedAddress(t *testing.T) {
 	}
 }
 
+func TestUpdateTarget_DeleteAddress(t *testing.T) {
+	tc := controller.NewTestController(t, nil)
+	t.Cleanup(tc.Shutdown)
+
+	client := tc.Client()
+	at := tc.Token()
+	client.SetToken(at.Token)
+	_, proj := iam.TestScopes(t, tc.IamRepo(), iam.WithUserId(at.UserId))
+	tClient := targets.NewClient(client)
+
+	assert, require := assert.New(t), require.New(t)
+
+	// Create Target with address
+	addr := "127.0.0.1"
+	createResp, err := tClient.Create(tc.Context(), "tcp", proj.PublicId,
+		targets.WithName("test_target_addr_delete"), targets.WithAddress(addr), targets.WithTcpTargetDefaultPort(22))
+	require.NoError(err)
+	require.NotNil(createResp)
+	assert.Equal(addr, createResp.GetItem().Address)
+
+	// Update to delete address (set to null)
+	updateResp, err := tClient.Update(tc.Context(), createResp.Item.Id, createResp.Item.Version, targets.DefaultAddress())
+	require.NoError(err)
+	require.NotNil(updateResp)
+	assert.Empty(updateResp.GetItem().Address)
+
+	// Do it again with same version, should error
+	_, err = tClient.Update(tc.Context(), createResp.Item.Id, createResp.Item.Version, targets.DefaultAddress())
+	require.Error(err)
+
+	// Do it again with the correct version, ensure this is not an error.
+	secondUpdateResp, err := tClient.Update(tc.Context(), createResp.Item.Id, updateResp.Item.Version, targets.DefaultAddress())
+	require.NoError(err)
+	require.NotNil(secondUpdateResp)
+	assert.Empty(secondUpdateResp.GetItem().Address)
+
+	assert.NotEqual(updateResp.Item.Version, secondUpdateResp.Item.Version)
+}
+
 func comparableSlice(in []*targets.Target) []targets.Target {
 	var filtered []targets.Target
 	for _, i := range in {