feat: Implement target selectors for policies.#3704
Conversation
Signed-off-by: Lior Okman <lior.okman@sap.com>
Signed-off-by: Lior Okman <lior.okman@sap.com>
Signed-off-by: Lior Okman <lior.okman@sap.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3704 +/- ##
==========================================
+ Coverage 68.81% 69.05% +0.24%
==========================================
Files 175 176 +1
Lines 21525 21720 +195
==========================================
+ Hits 14812 14999 +187
- Misses 5636 5639 +3
- Partials 1077 1082 +5 ☔ View full report in Codecov by Sentry. |
internal/gatewayapi/helpers.go
Outdated
| } | ||
|
|
||
| func getPolicyTargetRefs[T client.Object](policy egv1a1.PolicyTargetReferences, potentialTargets []T) []gwapiv1a2.LocalPolicyTargetReferenceWithSectionName { | ||
| dedup := map[gwapiv1a2.LocalPolicyTargetReferenceWithSectionName]byte{} |
internal/gatewayapi/helpers.go
Outdated
| } | ||
|
|
||
| ret := []gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{} | ||
| for key := range dedup { |
There was a problem hiding this comment.
can we garantee stability of list order with maps or sets ?
There was a problem hiding this comment.
also is there a way to make this stateless ? we are actively trying to reduce mem usage #3698
There was a problem hiding this comment.
can we garantee stability of list order with maps or sets ?
No. Sets are the same as the builtin maps, since they are implemented in the same way, and they do not guarantee the list order. When transforming the map to a list, I can sort the list. What would be the correct key in which the list should be ordered?
is there a way to make this stateless ? we are actively trying to reduce mem usage
Using a map to deduplicate the set of relevant targets means that the code never allocates more memory than it would need to keep track of the targets of the policy. I don't see a way to collect the targets of the selectors and not keep some state. Note that the memory is only allocated for the duration of the time it takes to handle the policy - it's not some cache that is kept and never released.
There was a problem hiding this comment.
lets sort based on creation timestamp
// Sort based on timestamp
sort.Slice(ret, func(i, j int) bool {
return ret[i].CreationTimestamp.Before(&(ret[j].CreationTimestamp))
})
```
returned refs are consistent. Signed-off-by: Lior Okman <lior.okman@sap.com>
|
/retest |
|
@arkodg, @liorokman, I installed |
What this PR does / why we need it:
This PR implements target selectors for policies
Which issue(s) this PR fixes:
Fixes #3607