Some minor rewording of the priority conflict #303
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bowei The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Deploy Preview for kubernetes-sigs-network-policy-api ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
size/M
|
/assign @tssurya |
| // | ||
| // If two (or more) policies with equal priority match the same | ||
| // traffic, then any one of the policies will be applied to the | ||
| // traffic. There is no way for the user to reliably determine the |
There was a problem hiding this comment.
So this unreliability is exactly how it was previously and we purposefully changed it to also include:
then the implementation can apply any of the matching policies to the connection part to suggest to users - so they know its the implementation's algorithm not governed by the API, so should we keep this sentence?
There was a problem hiding this comment.
I actually added something to make it clear that only one policy is applied? We should probably discuss this to make sure we are OK with that.
Regarding talking about the implementation, IMO it's clearer to focus on the behavior for the end user. In the end, the implementation has to meet the contract -- so saying the "implementation can do X" doesn't add anything towards describing how it works.
- Assert that if multiple policies match, even though it is indeterminate which policy matches, only a single one will be applied. - This makes it more clear that admins should make different priorties for conflicts.
|
Ping on this -- I think I address your comment @tssurya? Let me know. |
| // values. All AdminNetworkPolicy rules have higher precedence | ||
| // than NetworkPolicy or BaselineAdminNetworkPolicy rules. | ||
| // | ||
| // If two (or more) policies with equal priority match the |
There was a problem hiding this comment.
connection v/s traffic we should discuss more - #305
I'm pretty sure some implementations do it connection based and that's how initially network policies were also defined.
|
LGTM on the priority wordings. @bowei I think we should keep it as connections instead of traffic for now in v1alpha1 and then for alpha2 we should revisit it. - at least the scope of this PR was to reword priorities and we can then merged this and we can revisit the packets v/s connection topic later |
|
I'm ok with which ever give this is all changing significantly. Please close if this is no longer relevant. |
This makes it more clear that admins should make different priorties for conflicts.