access rule apply races allow duplicates

[Tom Barron]

While investigating another bug [1] I made a simple shell script [2] that runs concurrent access-allow commands for the same share with conflicting rules like applying IP 128.101.101.101 and 128.101.101.101/32.

It doesn't take long running iterations of this script to see duplicate rules getting applied, indicating that there are races in the API w.r.t. application of the rule to the share in the DB so that conflicts are not getting detected as intended. For example:

inspecting access list for share S1
+--------------------------------------+-------------+----------------+--------------+--------+------------+----------------------------+------------+
| id | access_type | access_to | access_level | state | access_key | created_at | updated_at |
+--------------------------------------+-------------+----------------+--------------+--------+------------+----------------------------+------------+
| 01a4c958-c889-46ce-9703-fcc896b7f091 | ip | 90.20.30.40/32 | rw | active | None | 2019-03-04T21:10:15.000000 | None |
| 6667166e-cd45-49bd-93c7-16112768bc83 | ip | 20.20.30.40 | rw | active | None | 2019-03-04T21:10:16.000000 | None |
| 85368c36-9e41-4d96-b978-44953c24792b | ip | 10.20.30.40/32 | rw | active | None | 2019-03-04T21:10:15.000000 | None |
| aae4d989-bcb4-45b6-9edc-2bc3493b45db | ip | 90.20.30.40 | rw | active | None | 2019-03-04T21:10:15.000000 | None |
+--------------------------------------+-------------+----------------+--------------+--------+------------+----------------------------+------------+

Here 90.20.30.40/32 and 90.20.30.40 should have conflicted and only one rule should have been applied.

[1] https://bugs.launchpad.net/manila/+bug/1808127

[2] Script:

#!/bin/bash

echo
echo 'creating share S1'
manila create nfs 1 --name S1

echo
echo 'inspecting access list for share S1'
manila access-list S1

sleep 5
echo
echo 'sending conflicting access-allows for S1'
sleep 2

manila access-allow S1 ip 10.20.30.40 & manila access-allow S1 ip 20.20.30.40 & \
manila access-allow S1 ip 10.20.30.40/32 & manila access-allow S1 ip 20.20.30.40/32 &\
manila access-allow S1 ip 10.20.30.40 & \
manila access-allow S1 ip 10.20.30.40/32 & \
manila access-allow S1 ip 90.20.30.40 & \
manila access-allow S1 ip 90.20.30.40/32 & \
manila access-allow S1 ip 90.20.30.40 & \
manila access-allow S1 ip 90.20.30.40/32 & \
#manila access-allow S1 ip 200.20.30.40 & \
#manila access-allow S1 ip 200.20.30.40/32 & \
#manila access-allow S1 ip 200.20.30.40 & \
#manila access-allow S1 ip 200.20.30.40/32 &

sleep 20

echo
echo 'inspecting access list for share S1'
manila access-list S1

sleep 5
echo
echo 'look above to see if there are any duplicates'

sleep 2

echo
echo
echo "Cleaning Up"
echo 'removing access rules from share S1'

IDS=$(manila access-list S1 | grep -Ev '^\+| id ' | awk '{print $2}')
for id in $IDS
do
   manila access-deny S1 $id
done

sleep 3

echo 'inspecting access list for share S1'
manila access-list S1

echo 'removing share S1'
manila delete S1

[Goutham Pacha Ravi (gouthamr)]

Good debugging on this issue, wonder if this happens without the cidr notation (/32)? I added the deduplication for cidr notations /32 and /128 here [1]; I didn't expect to hit races down in the database layer for that rather simple lookup.

[1] https://review.openstack.org/#/c/568364

[Jason Grosso (jgrosso)]

Will be discussed as part of PTG

[Vida Haririan (vhariria)]

Additional comments https://meetings.opendev.org/meetings/manila/2021/manila.2021-06-17-15.00.log.html