auth: store cached permission information in a form of interval tree

[mitake]

This commit change the type of cached permission information from the
home made thing to interval tree. It improves computational complexity
of permission checking from O(n) to O(lg n).

/cc @heyitsanthony This is a preparation for #7468 , could you take a look? We can support the unlimited range by configuring a permission like ["a", "\xff") in a simple manner thanks to interval tree.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@049ca87). Click here to learn what that means.
The diff coverage is 55.76%.

@@            Coverage Diff            @@
##             master    #7569   +/-   ##
=========================================
  Coverage          ?   70.15%           
=========================================
  Files             ?      320           
  Lines             ?    26165           
  Branches          ?        0           
=========================================
  Hits              ?    18355           
  Misses            ?     6355           
  Partials          ?     1455

Impacted Files	Coverage Δ
auth/store.go	83.46% <100%> (ø)
auth/range_perm_cache.go	52.5% <54.9%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 049ca87...dca7fce. Read the comment docs.

[heyitsanthony]

Good to see patches that remove code. Looks OK in general, but would benefit from some tests (at least for the [a, z) vs ([a, c) U [x,z)) case)

[heyitsanthony]

var ivl adt.Interval
if len(perm.RangeEnd) != 0 {
    ivl = adt.NewStringAffineInterval(string(perm.Key), string(perm.RangeEnd))
} else {
    ivl = adt.NewStringAffinePoint(string(perm.Key))
}
switch perm.PermType {
...
}

?

[mitake]

Ah, yes, the type of the return value of NewStringAffinePoint() is also Interval, so it can be simplified as you pointed. I'll fix it.

[heyitsanthony]

If I check for permission on [a, z) and I have permission for ranges [a, c) and [x, z), this will say there's permission for all of [a, z)?

[mitake]

I missed the case... thanks! Possibly

1. visit every interval that overlaps with the required range
2. check all the intervals are continuous
3. if they are not continuous, permission denied
4. return minBegin.Compare(adt.StringComparable(key)) < 0 && maxEnd.Compare(adt.StringComparable(maxEnd)) < 0

I'll also add some test cases. At first I thought that the implementation of interval tree has its own tests, so adding new tests would be redundant. But clearly it was wrong...

[heyitsanthony]

They're not necessarily continuous for cases like [a, f), [c, d), [f, z). If you're feeling adventurous, this can probably be written abstractly using only adt.Comparable.

[mitake]

The word "continuous" wasn't good. I just tried to mean "no hole in the granted permissions". The check would be performed like this:

	tocheck.Visit(ivl, func(n *adt.IntervalValue) bool {
		visited = append(visited, n)

		return false
	})
	for i := 0; i < len(visited)-1; i++ {
		if visited[i].Ivl.End.Compare(visited[i+1].Ivl.Begin) == -1 {
			// there is a hole, permission denied
			return false
		}
	}

The loop can handle the case you showed?

[heyitsanthony]

I think it's still possible to get something like [a, d), [a, b), [c, f).

isContiguous := true
var maxEnd, minBegin adt.Comparable
tocheck.Visit(ivl, func(n *adt.IntervalValue) bool { 
    if minBegin == nil { minBegin = n.Ivl.Begin; maxEnd = n.Ivl.End; return true}
    if maxEnd.Compare(n.ivl.Begin) < 0 { isContiguous = false; return false }
    if maxEnd.Compare(n.ivl.End) > 0 { maxEnd = n.ivl.End }
    return true
})
contains := isContiguous && maxEnd.Compare(ivl.End) >= 0 && minBegin.Compare(ivl.Begin) <= 0

?

[mitake]

Your strategy seems to be good, thanks!

[heyitsanthony]

I think I got the bools right-- false should stop evaluating. On the other hand, it appears that the interval tree is ignoring the return value entirely. haha oops. I'll fix it in that sorted visit PR

[mitake]

Yes, it was my misunderstanding... I noticed it and updated my comment because I didn't receive your reply at that time ;)

[mitake]

@heyitsanthony updated based on your comments. The main changed points:

• now the code doesn't use StringComparable instead of StringAffineInterval because "" shouldn't be larger than other strings
• changed if maxEnd.Compare(n.ivl.End) > 0 { maxEnd = n.ivl.End } in the callback to n.Ivl.End.Compare(maxEnd) > 0

But this PR depends on your PR #7581 so it cannot be merged now.

[mitake]

@heyitsanthony rebased on the master (that includes your changes of interval tree), so this PR would be able to work well. Could you take a look?

[mitake]

@heyitsanthony two test fails can be found but they don't seem to be related with this change because they are related to membership management: #7594 and #7595

[mitake]

@heyitsanthony kindly ping? Do you have any opinions about the failed tests?

[heyitsanthony]

@mitake oops sorry for losing this in the shuffle. I kicked CI and it's passing.

The interval tree adt could stand to have contiguity checking and BytesAffineInterval to save allocs from the string casts, but that's not a blocker.

lgtm, thanks!

[mitake]

@heyitsanthony then I'm merging it, thanks! I also add the changes of interval tree in other PRs.

BTW the failed test seems to be same with #7595